Re: [Xen-devel] [PATCH 08/15] xen/x86: p2m: Use typesafe gfn for the P2M callbacks get_entry and set_entry

[Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>]

On 09/13/2017 09:27 PM, Julien Grall wrote:
> Hi Andrew,
> 
> On 09/13/2017 07:22 PM, Andrew Cooper wrote:
>> On 13/09/2017 18:59, Julien Grall wrote:
>>> diff --git a/xen/arch/x86/mm/p2m-pt.c b/xen/arch/x86/mm/p2m-pt.c
>>> index 0e63d6ed11..57878b1886 100644
>>> --- a/xen/arch/x86/mm/p2m-pt.c
>>> +++ b/xen/arch/x86/mm/p2m-pt.c
>>> @@ -479,12 +479,13 @@ int p2m_pt_handle_deferred_changes(uint64_t gpa)
>>>     /* Returns: 0 for success, -errno for failure */
>>>   static int
>>> -p2m_pt_set_entry(struct p2m_domain *p2m, unsigned long gfn, mfn_t mfn,
>>> +p2m_pt_set_entry(struct p2m_domain *p2m, gfn_t gfn_t, mfn_t mfn,
>>>                    unsigned int page_order, p2m_type_t p2mt,
>>> p2m_access_t p2ma,
>>>                    int sve)
>>>   {
>>>       /* XXX -- this might be able to be faster iff current->domain
>>> == d */
>>>       void *table;
>>> +    unsigned long gfn = gfn_x(gfn_t);
>>>       unsigned long i, gfn_remainder = gfn;
>>>       l1_pgentry_t *p2m_entry, entry_content;
>>>       /* Intermediate table to free if we're replacing it with a
>>> superpage. */
>>> @@ -731,11 +732,12 @@ p2m_pt_set_entry(struct p2m_domain *p2m,
>>> unsigned long gfn, mfn_t mfn,
>>>   }
>>>     static mfn_t
>>> -p2m_pt_get_entry(struct p2m_domain *p2m, unsigned long gfn,
>>> +p2m_pt_get_entry(struct p2m_domain *p2m, gfn_t gfn_t,
>>>                    p2m_type_t *t, p2m_access_t *a, p2m_query_t q,
>>>                    unsigned int *page_order, bool_t *sve)
>>>   {
>>>       mfn_t mfn;
>>> +    unsigned long gfn = gfn_x(gfn_t);
>>
>> These two are rather risky, because you shadow the gfn_t type with a
>> variable named gfn_t.  I know its ugly, but how about just gfn_ ?
> 
> I can do that. Hopefully in the future both functions will be fully
> typesafe, so gfn_ will completely disappear.

Ah, I had proposed gfn_l for unsigned long gfns - it seems to be the
unspoken convention in the rest of the code, so I think it fits nicely.


Thanks,
Razvan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel