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

[Julien Grall <julien.grall@xxxxxxx>]

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@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel