e820_host default value and libxl (not xl)

e820_host default value and libxl (not xl)

[Marek Marczykowski-Górecki]

[-- Attachment #1.1: Type: text/plain, Size: 738 bytes --]

Hi,

According to xl.cfg(5) " This option defaults to true (1) if any PCI
passthrough devices are configured and false (0) otherwise."
And indeed this behaviour is implemented in xl. But not in libxl, which
means other libxl based toolstacks (libvirt) will not take advantage of
this directly.

What would be the best approach here? Duplicate that behaviour in
libvirt (currently libvirt knows nothing about this option), or move
that default handling to libxl? I think the later makes more sense, but
maybe there is some reason against it?

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

Re: e820_host default value and libxl (not xl)

[Dario Faggioli]

On Sat, May 21, 2016 at 4:42 AM, Marek Marczykowski-Górecki
<marmarek@invisiblethingslab.com> wrote:
> Hi,
>
> According to xl.cfg(5) " This option defaults to true (1) if any PCI
> passthrough devices are configured and false (0) otherwise."
> And indeed this behaviour is implemented in xl. But not in libxl, which
> means other libxl based toolstacks (libvirt) will not take advantage of
> this directly.
>
> What would be the best approach here? Duplicate that behaviour in
> libvirt (currently libvirt knows nothing about this option), or move
> that default handling to libxl? I think the later makes more sense, but
> maybe there is some reason against it?
>
I'm no tools maintainers (and in fact I'm Cc-ing them), but I'd say
that moving this to libxl is what looks to me to make more sense...

Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
---------------------------------------------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

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

Re: e820_host default value and libxl (not xl)

[Wei Liu]

On Sat, May 21, 2016 at 04:42:11AM +0200, Marek Marczykowski-Górecki wrote:
> Hi,
> 
> According to xl.cfg(5) " This option defaults to true (1) if any PCI
> passthrough devices are configured and false (0) otherwise."
> And indeed this behaviour is implemented in xl. But not in libxl, which
> means other libxl based toolstacks (libvirt) will not take advantage of
> this directly.
> 
> What would be the best approach here? Duplicate that behaviour in
> libvirt (currently libvirt knows nothing about this option), or move
> that default handling to libxl? I think the later makes more sense, but
> maybe there is some reason against it?
> 

The latter.

I wouldn't be surprised if the boundary between xl and libxl was
overlooked when implementing this flag. I've done similar things to push
xsm label handling logic from xl to libxl.

Wei.

> -- 
> Best Regards,
> Marek Marczykowski-Górecki
> Invisible Things Lab
> A: Because it messes up the order in which people normally read text.
> Q: Why is top-posting such a bad thing?



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


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

Re: e820_host default value and libxl (not xl)

[Andrew Cooper]

On 23/05/16 11:47, Wei Liu wrote:
> On Sat, May 21, 2016 at 04:42:11AM +0200, Marek Marczykowski-Górecki wrote:
>> Hi,
>>
>> According to xl.cfg(5) " This option defaults to true (1) if any PCI
>> passthrough devices are configured and false (0) otherwise."
>> And indeed this behaviour is implemented in xl. But not in libxl, which
>> means other libxl based toolstacks (libvirt) will not take advantage of
>> this directly.
>>
>> What would be the best approach here? Duplicate that behaviour in
>> libvirt (currently libvirt knows nothing about this option), or move
>> that default handling to libxl? I think the later makes more sense, but
>> maybe there is some reason against it?
>>
> The latter.
>
> I wouldn't be surprised if the boundary between xl and libxl was
> overlooked when implementing this flag. I've done similar things to push
> xsm label handling logic from xl to libxl.

Please don't propage this bandaid any further than it currently is.  It
is not appropriate for libxl to set this by default.

The reason it is currently used is because libxl/libxc doesn't know how
to lay out a guests physmap.  This is something I am working on
resolving for some ballooning issues we are having in XenServer.

xl can get away defaulting this on, because xl is inherently a single
host toolstack.  However, using host_e820 is wrong for any multi-host
setup where the VM might plausibly migrate (which includes the
passthrough case here).

~Andrew

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

Re: e820_host default value and libxl (not xl)

[Marek Marczykowski-Górecki]

[-- Attachment #1.1: Type: text/plain, Size: 2334 bytes --]

On Mon, May 23, 2016 at 11:59:10AM +0100, Andrew Cooper wrote:
> On 23/05/16 11:47, Wei Liu wrote:
> > On Sat, May 21, 2016 at 04:42:11AM +0200, Marek Marczykowski-Górecki wrote:
> >> Hi,
> >>
> >> According to xl.cfg(5) " This option defaults to true (1) if any PCI
> >> passthrough devices are configured and false (0) otherwise."
> >> And indeed this behaviour is implemented in xl. But not in libxl, which
> >> means other libxl based toolstacks (libvirt) will not take advantage of
> >> this directly.
> >>
> >> What would be the best approach here? Duplicate that behaviour in
> >> libvirt (currently libvirt knows nothing about this option), or move
> >> that default handling to libxl? I think the later makes more sense, but
> >> maybe there is some reason against it?
> >>
> > The latter.
> >
> > I wouldn't be surprised if the boundary between xl and libxl was
> > overlooked when implementing this flag. I've done similar things to push
> > xsm label handling logic from xl to libxl.
> 
> Please don't propage this bandaid any further than it currently is.  It
> is not appropriate for libxl to set this by default.
> 
> The reason it is currently used is because libxl/libxc doesn't know how
> to lay out a guests physmap.  This is something I am working on
> resolving for some ballooning issues we are having in XenServer.

Does it mean e820_host wouldn't be needed anymore?

> xl can get away defaulting this on, because xl is inherently a single
> host toolstack.  However, using host_e820 is wrong for any multi-host
> setup where the VM might plausibly migrate (which includes the
> passthrough case here).

But *currently* having host_e820 disabled makes it impossible to
passthrough some devices, even when no migration is involved. Libvirt
does not support e820_host. So, I'm looking for a solution for the
problem, which makes it impossible to use some devices at all when using
libvirt.

If e820_host will not be needed anymore in the near future, I can wait,
or simply carry a local patch for this. But otherwise I think it would
be good to fix this in either libxl or libvirt.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

Re: e820_host default value and libxl (not xl)

[Konrad Rzeszutek Wilk]

On Mon, May 23, 2016 at 11:47:48AM +0100, Wei Liu wrote:
> On Sat, May 21, 2016 at 04:42:11AM +0200, Marek Marczykowski-Górecki wrote:
> > Hi,
> > 
> > According to xl.cfg(5) " This option defaults to true (1) if any PCI
> > passthrough devices are configured and false (0) otherwise."
> > And indeed this behaviour is implemented in xl. But not in libxl, which
> > means other libxl based toolstacks (libvirt) will not take advantage of
> > this directly.
> > 
> > What would be the best approach here? Duplicate that behaviour in
> > libvirt (currently libvirt knows nothing about this option), or move
> > that default handling to libxl? I think the later makes more sense, but
> > maybe there is some reason against it?
> > 
> 
> The latter.
> 
> I wouldn't be surprised if the boundary between xl and libxl was
> overlooked when implementing this flag. I've done similar things to push

It never occurred to me when I wrote that code sadly :-(

> xsm label handling logic from xl to libxl.
> 
> Wei.
> 
> > -- 
> > Best Regards,
> > Marek Marczykowski-Górecki
> > Invisible Things Lab
> > A: Because it messes up the order in which people normally read text.
> > Q: Why is top-posting such a bad thing?
> 
> 
> 
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xen.org
> > http://lists.xen.org/xen-devel
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

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

Re: e820_host default value and libxl (not xl)

[Konrad Rzeszutek Wilk]

On Mon, May 23, 2016 at 01:27:31PM +0200, Marek Marczykowski-Górecki wrote:
> On Mon, May 23, 2016 at 11:59:10AM +0100, Andrew Cooper wrote:
> > On 23/05/16 11:47, Wei Liu wrote:
> > > On Sat, May 21, 2016 at 04:42:11AM +0200, Marek Marczykowski-Górecki wrote:
> > >> Hi,
> > >>
> > >> According to xl.cfg(5) " This option defaults to true (1) if any PCI
> > >> passthrough devices are configured and false (0) otherwise."
> > >> And indeed this behaviour is implemented in xl. But not in libxl, which
> > >> means other libxl based toolstacks (libvirt) will not take advantage of
> > >> this directly.
> > >>
> > >> What would be the best approach here? Duplicate that behaviour in
> > >> libvirt (currently libvirt knows nothing about this option), or move
> > >> that default handling to libxl? I think the later makes more sense, but
> > >> maybe there is some reason against it?
> > >>
> > > The latter.
> > >
> > > I wouldn't be surprised if the boundary between xl and libxl was
> > > overlooked when implementing this flag. I've done similar things to push
> > > xsm label handling logic from xl to libxl.
> > 
> > Please don't propage this bandaid any further than it currently is.  It
> > is not appropriate for libxl to set this by default.
> > 
> > The reason it is currently used is because libxl/libxc doesn't know how
> > to lay out a guests physmap.  This is something I am working on
> > resolving for some ballooning issues we are having in XenServer.
> 
> Does it mean e820_host wouldn't be needed anymore?

Presumarily also the other flags we have: mmio_hole ?
> 
> > xl can get away defaulting this on, because xl is inherently a single
> > host toolstack.  However, using host_e820 is wrong for any multi-host
> > setup where the VM might plausibly migrate (which includes the
> > passthrough case here).

It would be better if you could give the libxl/xl a e820 array record.

As in - one could jam an e820 that would be the same across various
servers.

> 
> But *currently* having host_e820 disabled makes it impossible to
> passthrough some devices, even when no migration is involved. Libvirt
> does not support e820_host. So, I'm looking for a solution for the
> problem, which makes it impossible to use some devices at all when using
> libvirt.
> 
> If e820_host will not be needed anymore in the near future, I can wait,
> or simply carry a local patch for this. But otherwise I think it would
> be good to fix this in either libxl or libvirt.
> 
> -- 
> Best Regards,
> Marek Marczykowski-Górecki
> Invisible Things Lab
> A: Because it messes up the order in which people normally read text.
> Q: Why is top-posting such a bad thing?



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


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

Re: e820_host default value and libxl (not xl)

[Wei Liu]

On Mon, May 23, 2016 at 11:59:10AM +0100, Andrew Cooper wrote:
> On 23/05/16 11:47, Wei Liu wrote:
> > On Sat, May 21, 2016 at 04:42:11AM +0200, Marek Marczykowski-Górecki wrote:
> >> Hi,
> >>
> >> According to xl.cfg(5) " This option defaults to true (1) if any PCI
> >> passthrough devices are configured and false (0) otherwise."
> >> And indeed this behaviour is implemented in xl. But not in libxl, which
> >> means other libxl based toolstacks (libvirt) will not take advantage of
> >> this directly.
> >>
> >> What would be the best approach here? Duplicate that behaviour in
> >> libvirt (currently libvirt knows nothing about this option), or move
> >> that default handling to libxl? I think the later makes more sense, but
> >> maybe there is some reason against it?
> >>
> > The latter.
> >
> > I wouldn't be surprised if the boundary between xl and libxl was
> > overlooked when implementing this flag. I've done similar things to push
> > xsm label handling logic from xl to libxl.
> 
> Please don't propage this bandaid any further than it currently is.  It
> is not appropriate for libxl to set this by default.
> 
> The reason it is currently used is because libxl/libxc doesn't know how
> to lay out a guests physmap.  This is something I am working on
> resolving for some ballooning issues we are having in XenServer.
> 

This is not entirely true. It knows a version of it, but not the final
version, which is even worse than not knowing at all. :-)

> xl can get away defaulting this on, because xl is inherently a single
> host toolstack.  However, using host_e820 is wrong for any multi-host
> setup where the VM might plausibly migrate (which includes the
> passthrough case here).
> 

Libxl has to determine the value one way or another:

1. get that value from upper layer
2. derive from internal logic

#1 takes precedence. Currently the "internal logic" is "always off".
This makes life harder for toolstack other than xl, so making that
internal logic better looks acceptable to me.

On the other hand I agree this is only bandaid to bigger issue. The
objection is also quite convincing.

So living it as is is probably the safest option.

Wei.

> ~Andrew

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