Re: [Xen-devel] [PATCH v2] xen: arm: Support <32MB frametables

["Chris (Christopher) Brand" <chris.brand@xxxxxxxxxxxx>]

Hi Julien,

Thanks for the review.

> >  /* Create Xen's mappings of memory.
> > - * Base and virt must be 32MB aligned and size a multiple of 32MB.
> > + * Mapping_size must be either 2MB or 32MB.
> 
> I would have generalize the function to support any mapping size. But I'm
> also fine with the current solution.

The code would have to be (somewhat) more complex to do that. I think
I prefer to keep it simple for now.

> > +    const unsigned long granularity = mapping_size >> PAGE_SHIFT;
> 
> This variable is only used in the ASSERT. On non-debug build the compiler will
> throw an error because the variable will be unused.

Good point. Will fix.

> > +    if ( mapping_size == MB(32) )
> > +        pte.pt.contig = 1;  /* These maps are in 16-entry contiguous
> > + chunks. */
> 
> mfn_to_xen_entry never set the contig bit to 0 (or anything else). So the
> value will be undefined for MB(2) mapping.
> 
> It looks like to me that mfn_to_xen_entry should always set the contig bit to
> 0. I'm not sure why we didn't see any issue until now. Can you please send a
> patch for this?

Will do.

Chris


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