Re: [PATCH 13/14] xen/mm: Convert {s, g}et_gpfn_from_mfn() to use typesafe MFN

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 10/05/2019 15:08, Julien Grall wrote:
>
>
> On 10/05/2019 15:05, Andrew Cooper wrote:
>> On 10/05/2019 15:02, Jan Beulich wrote:
>>>>>> On 10.05.19 at 15:46, <julien.grall@arm.com> wrote:
>>>> On 10/05/2019 14:41, Jan Beulich wrote:
>>>>>>>> On 10.05.19 at 15:34, <julien.grall@arm.com> wrote:
>>>>>> On 10/05/2019 14:21, Jan Beulich wrote:
>>>>>>>>>> On 07.05.19 at 17:14, <julien.grall@arm.com> wrote:
>>>>>>>> @@ -1030,19 +1031,19 @@ long p2m_pt_audit_p2m(struct p2m_domain
>>>>>>>> *p2m)
>>>>>>>> @@ -2795,54 +2795,54 @@ void audit_p2m(struct domain *d,
>>>>>>>>         spin_lock(&d->page_alloc_lock);
>>>>>>>>         page_list_for_each ( page, &d->page_list )
>>>>>>>>         {
>>>>>>>> -        mfn = mfn_x(page_to_mfn(page));
>>>>>>>> +        mfn = page_to_mfn(page);
>>>>>>>>     -        P2M_PRINTK("auditing guest page, mfn=%#lx\n", mfn);
>>>>>>>> +        P2M_PRINTK("auditing guest page, mfn=%"PRI_mfn"\n",
>>>>>>>> mfn_x(mfn));
>>>>>>>>                 od = page_get_owner(page);
>>>>>>>>                 if ( od != d )
>>>>>>>>             {
>>>>>>>> -            P2M_PRINTK("wrong owner %#lx -> %p(%u) != %p(%u)\n",
>>>>>>>> -                       mfn, od, (od?od->domain_id:-1), d,
>>>>>>>> d->domain_id);
>>>>>>>> +            P2M_PRINTK("wrong owner %"PRI_mfn" -> %p(%u) !=
>>>>>>>> %p(%u)\n",
>>>>>>>> +                       mfn_x(mfn), od, (od?od->domain_id:-1),
>>>>>>>> d, d->domain_id);
>>>>>>> Please be careful not to drop 0x prefixes from the resulting output
>>>>>>> (which are an effect of the # flag that you delete), at least when
>>>>>>> log messages contain a mix of hex and dec numbers. (I am, btw,
>>>>>>> not convinced that switching to PRI_mfn here is helpful.)
>>>>>> Last time I keeped %# for MFN, I have been asked to remove the #.
>>>>>> I prefer
>>>>>> having 0x for all the hex, and I am happy to be keep as is. But I
>>>>>> would like a
>>>>>> bit of consistency on the way we print MFN...
>>>>> Well, "%#"PRI_mfn is bogus (because of the combination with the
>>>>> minimum width specification), so it ought to be "%#lx" or
>>>>> "0x%"PRI_mfn.
>>>>> Have you really been asked for something else? If so, and if it
>>>>> was me,
>>>>> then I apologize.
>>>> I am not sure why this is bogus. The thing is using different
>>>> format for the MFN
>>>> makes it difficult to read a message without looking format string.
>>> We look to be in agreement that there should be a 0x prefix here.
>>> What I'm asking for is to avoid the value logged to de-generate into
>>> a 3-digit one (0x???) when a five digit one is meant (see PRI_mfn).
>>> Not to speak of the further inconsistent string that would be logged
>>> for MFN 0.
>>
>> The overwhelming majority way of printing mfns is via:
>>
>> mfn %"PRI_mfn"
>>
>> which is almost fully consistent across the x86 code.
>
> If I got it right, the format here would be "wrong owner mfn
> %"PRI_mfn". Am I correct?

IMO, yes, but see my specific email for an even better alternative.

~Andrew


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel