Re: [Xen-devel] [PATCH v8 4/6] remove late (on-demand) construction of IOMMU page tables

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Julien Grall <julien.grall@xxxxxxx>
> Sent: 05 September 2019 21:21
> To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: Alexandru Isaila <aisaila@xxxxxxxxxxxxxxx>; Razvan Cojocaru 
> <rcojocaru@xxxxxxxxxxxxxxx>; Jan
> Beulich <jbeulich@xxxxxxxx>; Stefano Stabellini <sstabellini@xxxxxxxxxx>; 
> Volodymyr Babchuk
> <Volodymyr_Babchuk@xxxxxxxx>; Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>; 
> George Dunlap
> <George.Dunlap@xxxxxxxxxx>; Ian Jackson <Ian.Jackson@xxxxxxxxxx>; Konrad 
> Rzeszutek Wilk
> <konrad.wilk@xxxxxxxxxx>; Tim (Xen.org) <tim@xxxxxxx>; Wei Liu <wl@xxxxxxx>; 
> Roger Pau Monne
> <roger.pau@xxxxxxxxxx>; Tamas K Lengyel <tamas@xxxxxxxxxxxxx>; Petre Pircalabu
> <ppircalabu@xxxxxxxxxxxxxxx>
> Subject: Re: [PATCH v8 4/6] remove late (on-demand) construction of IOMMU 
> page tables
> 
> Hi,
> 
> On 9/2/19 3:50 PM, Paul Durrant wrote:
> > diff --git a/tools/libxl/libxl_mem.c b/tools/libxl/libxl_mem.c
> > index 448a2af8fd..fd6f33312e 100644
> > --- a/tools/libxl/libxl_mem.c
> > +++ b/tools/libxl/libxl_mem.c
> > @@ -461,15 +461,17 @@ int libxl_domain_need_memory(libxl_ctx *ctx,
> >       if (rc) goto out;
> >
> >       *need_memkb = b_info->target_memkb;
> > +    *need_memkb += b_info->shadow_memkb + b_info->iommu_memkb;
> 
> AFAICT, iommu_memkb will be non-0 even when the IOMMU share the
> page-table with the CPUs. If so, why is this required for that case?

The toostack can't know about shared EPT as there's no mechanism to tell it. 
Once patch #6 goes in though, the toolstack will be able to select shared and 
forego the overhead. However, I've just realized that of course this means that 
the domain may fail due to lack of resources on a host which doesn't support 
shared EPT so I think I'm going to have to add add extra info (following on 
from Roger's recent patch) so the toolstack can know whether shared EPT is 
available.

  Paul

> 
> > +
> >       switch (b_info->type) {
> >       case LIBXL_DOMAIN_TYPE_PVH:
> >       case LIBXL_DOMAIN_TYPE_HVM:
> > -        *need_memkb += b_info->shadow_memkb + LIBXL_HVM_EXTRA_MEMORY;
> > +        *need_memkb += LIBXL_HVM_EXTRA_MEMORY;
> >           if (libxl_defbool_val(b_info->device_model_stubdomain))
> >               *need_memkb += 32 * 1024;
> >           break;
> >       case LIBXL_DOMAIN_TYPE_PV:
> > -        *need_memkb += b_info->shadow_memkb + LIBXL_PV_EXTRA_MEMORY;
> > +        *need_memkb += LIBXL_PV_EXTRA_MEMORY;
> >           break;
> >       default:
> >           rc = ERROR_INVAL;
> > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> > index b61399ce36..d94b7453cb 100644
> > --- a/tools/libxl/libxl_types.idl
> > +++ b/tools/libxl/libxl_types.idl
> > @@ -486,6 +486,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
> >       ("target_memkb",    MemKB),
> >       ("video_memkb",     MemKB),
> >       ("shadow_memkb",    MemKB),
> > +    ("iommu_memkb",     MemKB),
> 
> I think you want a corresponding LIBXL_HAVE in libxl.h to tell external
> toolstack whether the field exist.
> 
> >       ("rtc_timeoffset",  uint32),
> >       ("exec_ssidref",    uint32),
> >       ("exec_ssid_label", string),
> 
> Cheers,
> 
> --
> Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel