Re: [Xen-devel] [PATCH v2 12/18] xen: setup Xen specific data for PVH

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

On Mon, Oct 29, 2018 at 03:19:34PM +0100, Juergen Gross wrote:
> On 29/10/2018 13:57, Roger Pau Monné wrote:
> > On Fri, Oct 19, 2018 at 06:39:50PM +0200, Juergen Gross wrote:
> >> On 19/10/2018 18:10, Roger Pau Monné wrote:
> >>> On Tue, Oct 09, 2018 at 01:03:11PM +0200, Juergen Gross wrote:
> >>>> Initialize the needed Xen specific data. This is:
> >>>>
> >>>> - the Xen start of day page containing the console and Xenstore ring
> >>>>   page PFN and event channel
> >>>> - the grant table
> >>>> - the shared info page
> >>>>
> >>>> Set the RSDP address for the guest from the start_info page passed
> >>>> as boot parameter.
> >>>>
> >>>> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
> >>>> ---
> >>>>  grub-core/kern/i386/xen/pvh.c | 107 
> >>>> ++++++++++++++++++++++++++++++++++++++++++
> >>>>  1 file changed, 107 insertions(+)
> >>>>
> >>>> diff --git a/grub-core/kern/i386/xen/pvh.c 
> >>>> b/grub-core/kern/i386/xen/pvh.c
> >>>> index b4933b454..93ed68245 100644
> >>>> --- a/grub-core/kern/i386/xen/pvh.c
> >>>> +++ b/grub-core/kern/i386/xen/pvh.c
> >>>> @@ -24,6 +24,7 @@
> >>>>  #include <grub/xen.h>
> >>>>  #include <grub/i386/linux.h>
> >>>>  #include <grub/machine/kernel.h>
> >>>> +#include <xen/hvm/params.h>
> >>>>  #include <xen/memory.h>
> >>>>  
> >>>>  struct xen_machine_mmap_entry
> >>>> @@ -39,6 +40,7 @@ static struct { char _entry[32]; } hypercall_page[128]
> >>>>    __attribute__ ((aligned (GRUB_XEN_PAGE_SIZE)));
> >>>>  
> >>>>  static grub_uint32_t xen_cpuid_base;
> >>>> +static struct start_info grub_xen_start_page;
> >>>>  static struct xen_machine_mmap_entry map[128];
> >>>>  static unsigned int nr_map_entries;
> >>>>  
> >>>> @@ -104,6 +106,36 @@ grub_xen_hypercall (grub_uint32_t callno, 
> >>>> grub_uint32_t a0,
> >>>>    return __res;
> >>>>  }
> >>>>  
> >>>> +static grub_uint32_t
> >>>> +grub_xen_get_param (int idx)
> >>>> +{
> >>>> +  struct xen_hvm_param xhv;
> >>>> +  int r;
> >>>> +
> >>>> +  xhv.domid = DOMID_SELF;
> >>>> +  xhv.index = idx;
> >>>> +  r = grub_xen_hypercall (__HYPERVISOR_hvm_op, HVMOP_get_param,
> >>>> +                          (grub_uint32_t) (&xhv), 0, 0, 0, 0);
> >>>> +  if (r < 0)
> >>>> +    grub_xen_early_halt ();
> >>>> +  return xhv.value;
> >>>> +}
> >>>> +
> >>>> +static void *
> >>>> +grub_xen_add_physmap (unsigned int space, void *addr)
> >>>> +{
> >>>> +  struct xen_add_to_physmap xatp;
> >>>> +
> >>>> +  xatp.domid = DOMID_SELF;
> >>>> +  xatp.idx = 0;
> >>>> +  xatp.space = space;
> >>>> +  xatp.gpfn = (grub_addr_t) addr >> GRUB_XEN_LOG_PAGE_SIZE;
> >>>> +  if (grub_xen_hypercall (__HYPERVISOR_memory_op, XENMEM_add_to_physmap,
> >>>> +                          (grub_uint32_t) (&xatp), 0, 0, 0, 0))
> >>>> +    grub_xen_early_halt ();
> >>>> +  return addr;
> >>>> +}
> >>>> +
> >>>>  static void
> >>>>  grub_xen_sort_mmap (void)
> >>>>  {
> >>>> @@ -190,12 +222,87 @@ grub_xen_get_mmap (void)
> >>>>    grub_xen_sort_mmap ();
> >>>>  }
> >>>>  
> >>>> +static grub_uint64_t
> >>>> +grub_xen_find_page (grub_uint64_t start)
> >>>> +{
> >>>> +  unsigned int i, j;
> >>>> +  grub_uint64_t last = start;
> >>>> +
> >>>> +  /* Try to find a e820 map hole below 4G. */
> >>>
> >>> Doing this is kind of dangerous, what if you end up placing something
> >>> on top of an MMIO region (either emulated or from a real passthrough
> >>> device)?
> >>
> >> Shouldn't those be marked as "Reserved" in the memory map?
> > 
> > I don't think BARs are guaranteed to be in areas marked as reserved in
> > the memory map. Unless you also scan for PCI devices and make sure
> > there's no device with a BAR in the area you are attempting to
> > populate I think the above is not safe.
> 
> So either I can add scanning the PCI bus (which wouldn't be too
> hard IMO), or we require Xen tools to add memory map entries with
> "Reserved" attribute for passed-through PCI device's MMIO-areas
> (we can still do that as PCI pass-through for PVH isn't possible
> yet AFAIK).

Ideally (and that's kind of far away from what we are now), I would
like to have an interface to request Xen for a range of gfns available
to map stuff in them (grants/foreign mappings/shared page...). That
interface could be a simple hypercall that would return such range, or
an hypercall that could be used to fetch something akin to an
'extended memory map' with specific Xen information (like such
regions).

In both cases this requires Xen having a clearer picture of the p2m,
because any of the above solutions cannot rely on scanning the p2m
table in order to figure out what's where.

So the only change I would request is that if you use a RAM page you
update the memory map stored in Xen to match the new layout, by using
the XENMEM_set_memory_map hypercall.

Thanks, Roger.

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