Re: [Xen-devel] [PATCH-for-4.13] libxl: choose an appropriate default for passthrough...

[Paul Durrant <pdurrant@xxxxxxxxx>]

Ping? Can I get a response on this (w.r.t. 'enabled' vs. 'unknown')
before doing a v2? This issue is currently blocking a push, I believe.

On Tue, 1 Oct 2019 at 11:48, Paul Durrant <Paul.Durrant@xxxxxxxxxx> wrote:
>
> > -----Original Message-----
> > From: Ian Jackson <ian.jackson@xxxxxxxxxx>
> > Sent: 01 October 2019 11:39
> > To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>
> > Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; Wei Liu <wl@xxxxxxx>; Anthony Perard 
> > <anthony.perard@xxxxxxxxxx>;
> > Juergen Gross <jgross@xxxxxxxx>
> > Subject: Re: [PATCH-for-4.13] libxl: choose an appropriate default for 
> > passthrough...
> >
> > Paul Durrant writes ("[PATCH-for-4.13] libxl: choose an appropriate default 
> > for passthrough..."):
> > > ...if there is no IOMMU or it is globally disabled.
> > >
> > > Without this patch, the following assertion may be hit:
> > >
> > > xl: libxl_create.c:589: libxl__domain_make: Assertion `info->passthrough 
> > > !=
> > LIBXL_PASSTHROUGH_ENABLED' failed.
> > >
> > > This is because libxl__domain_create_info_setdefault() currently only sets
> > > an appropriate value for 'passthrough' in the case that 'cap_hvm_directio'
> > > is true, which is not the case unless an IOMMU is present and enabled in
> > > the system. This is normally masked by xl choosing a default value, but
> > > that will not happen if xl is not used (e.g. when using libvirt) or when
> > > a stub domain is being created.
> >
> > It's weird that after this patch "enabled" can mean DISABLED. Surely
> > if you say `passthrough="enabled"' and the host has no PT support (eg
> > it's disabled in the bios) it should fail ?
>
> Indeed, and xl will do exactly that.
>
> >
> > Normally libxl config options have an "unknown" or "default" option.
> >
> > Also it is anomalous that xl is doing the complex work of choosing a
> > default.  I think all the complex code
> >
> > +    switch (c_info->passthrough) {
> > +    case LIBXL_PASSTHROUGH_ENABLED:
> >
> > in xl_parse.c should be in libxl.  (Some of it is there already.)
> >
> > I'm sorry that I wasn't didn't review babde47a3fed...
> >
>
> So, would you advocate an 'unknown' value then? That's essentially just a 
> rename operation on 'enabled'.
>
> The code in xl_parse.c is there for a reason though; the appropriate amount 
> of extra memory for the IOMMU page tables has to be determined there because 
> the 'build' parts of libxl seem to be largely firewalled from the 'create' 
> parts and thus the relevant information is not available to decide the 
> appropriate overhead.
>
>   Paul
>
> > Ian.
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxxx
> https://lists.xenproject.org/mailman/listinfo/xen-devel

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