Re: [Xen-devel] [PATCH v6 05/11] x86/hyperv: setup hypercall page

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Mon, Feb 03, 2020 at 03:42:17PM +0000, Wei Liu wrote:
> On Mon, Feb 03, 2020 at 04:32:52PM +0100, Jan Beulich wrote:
> > On 03.02.2020 16:21, Roger Pau Monné wrote:
> > > On Mon, Feb 03, 2020 at 03:07:24PM +0000, Wei Liu wrote:
> > >> On Mon, Feb 03, 2020 at 04:01:54PM +0100, Roger Pau Monné wrote:
> > >>> On Fri, Jan 31, 2020 at 05:49:24PM +0000, Wei Liu wrote:
> > >>>> Hyper-V uses a technique called overlay page for its hypercall page. It
> > >>>> will insert a backing page to the guest when the hypercall 
> > >>>> functionality
> > >>>> is enabled. That means we can use a page that is not backed by real
> > >>>> memory for hypercall page.
> > >>>>
> > >>>> Use the top-most addressable page for that purpose. Adjust e820 map
> > >>>> accordingly.
> > >>>
> > >>> Can you add this is done to avoid page shattering and to make sure
> > >>> Xen isn't overwriting any MMIO area which might be present at lower
> > >>> addresses?
> > >>
> > >> NP.
> > >>
> > >>>
> > >>>> +
> > >>>> +static void __init e820_fixup(struct e820map *e820)
> > >>>> +{
> > >>>> +    uint64_t s = HV_HCALL_MFN << PAGE_SHIFT;
> > >>>> +
> > >>>> +    if ( !e820_add_range(e820, s, s + PAGE_SIZE, E820_RESERVED) )
> > >>>
> > >>> I think end should be s + PAGE_SIZE - 1, or else it expands across two
> > >>> pages?
> > >>
> > >> No, it shouldn't.
> > >>
> > >> E820 entry records the size of the region, which is calculated as
> > >> end-start. The one usage in pv/shim.c follows the same pattern here.
> > > 
> > > Hm, I see. I'm not sure this is correct, I think the e820 entry
> > > should look like:
> > > 
> > > addr = s;
> > > size = PAGE_SIZE - 1;
> > > 
> > > As ranges on the e820 are inclusive, so if size ends up being
> > > PAGE_SIZE then the entry would expand across two pages.
> > 
> > Ranges can sensibly be inclusive only when specified as [start,end]
> > tuples. (start,size) pairs make no sense for representing
> > [start,start+size], they only make sense for [start,start+size).
> > Otherwise, as in your example above, size taken on its own is off
> > by one (i.e. is rather "last byte" than "size").
> > 
> > Modern Linux, when logging the memory map, indeed subtracts 1 from
> > the sum of addr and size, to show an inclusive range.
> 
> We should perhaps do the same then.
> 
> If people agree this is the way to go, I can write a patch.

Oh, sorry. I got messed up by the way we print the ranges.

I think it would be helpful to -1 when printing the ranges, but
there's no need to do it in this series.

Thanks, Roger

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