Re: [Xen-devel] [PATCH MM-PART3 v2 06/12] xen/arm: mm: Sanity check any update of Xen page tables

[Julien Grall <julien.grall@arm.com>]

Hi,

On 12/06/2019 16:54, Stefano Stabellini wrote:
> On Wed, 12 Jun 2019, Julien Grall wrote:
>> On 12/06/2019 01:10, Stefano Stabellini wrote:
>>> On Tue, 14 May 2019, Julien Grall wrote:
>>> I understand we could skip the valid check on REMOVE, but should we skip
>>> it on MODIFY too? Is that also going to be helpful in future changes?
>>
>> Hmmm, I can't exactly remember why I didn't check the valid bit here.
>>
>> I did it for REMOVE as for the early FDT mapping it is more convenient to
>> remove the full possible range over keeping track of the exact start/size.
>>
>> I would assume the same would hold for MODIFY, but I don't have a concrete
>> example here. I am happy to add the valid check and defer the decision to
>> remove it if it is deem to be useful in the future.
> 
> Yes, it would be better

I will update it in the next version.

[...]

>>>>    static int xen_pt_update_entry(enum xenmap_operation op, unsigned long
>>>> addr,
>>>>                                   mfn_t mfn, unsigned int flags)
>>>>    {
>>>>        lpae_t pte, *entry;
>>>>        lpae_t *third = NULL;
>>>>    +    /* _PAGE_POPULATE and _PAGE_PRESENT should never be set together.
>>>> */
>>>> +    ASSERT((flags & (_PAGE_POPULATE|_PAGE_PRESENT)) !=
>>>> (_PAGE_POPULATE|_PAGE_PRESENT));
>>>
>>> over 80 chars?
>>
>> It is 87 chars, I was hoping you didn't notice it :). The line splitting
>> result to nasty code. Alternatively, I could introduce a define for
>> _PAGE_POPULATE|_PAGE_PRESENT, maybe EXCLUSIVE_FLAGS?
>>
>> Any preference?
> 
> I don't care so much about 80 chars limit.
> Anything but another macro :-)

Ok I will keep the 80 lines then!

Cheers,

-- 
Julien Grall

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