Nouveau - May 2023 - [PATCH 2/4] x86: always initialize xen-swiotlb when xen-pcifront is enabling

On Thu, May 18, 2023 at 08:18:39PM +0200, Marek Marczykowski-G?recki
wrote:
> On Thu, May 18, 2023 at 03:42:51PM +0200, Christoph Hellwig wrote:
> > Remove the dangerous late initialization of xen-swiotlb in
> > pci_xen_swiotlb_init_late and instead just always initialize
> > xen-swiotlb in the boot code if CONFIG_XEN_PCIDEV_FRONTEND is enabled.
> > 
> > Signed-off-by: Christoph Hellwig <hch at lst.de>
> 
> Doesn't it mean all the PV guests will basically waste 64MB of RAM
> by default each if they don't really have PCI devices?
If CONFIG_XEN_PCIDEV_FRONTEND is enabled, and the kernel's isn't booted
with swiotlb=noforce, yes.

On Fri, May 19, 2023 at 06:04:05AM +0200, Christoph Hellwig
wrote:
> On Thu, May 18, 2023 at 08:18:39PM +0200, Marek Marczykowski-G?recki wrote:
> > On Thu, May 18, 2023 at 03:42:51PM +0200, Christoph Hellwig wrote:
> > > Remove the dangerous late initialization of xen-swiotlb in
> > > pci_xen_swiotlb_init_late and instead just always initialize
> > > xen-swiotlb in the boot code if CONFIG_XEN_PCIDEV_FRONTEND is
enabled.
> > > 
> > > Signed-off-by: Christoph Hellwig <hch at lst.de>
> > 
> > Doesn't it mean all the PV guests will basically waste 64MB of RAM
> > by default each if they don't really have PCI devices?
> 
> If CONFIG_XEN_PCIDEV_FRONTEND is enabled, and the kernel's isn't
booted
> with swiotlb=noforce, yes.
That's "a bit" unfortunate, since that might be significant part
of the
VM memory, or if you have a lot of VMs, a significant part of the host
memory - it quickly adds up.
While I would say PCI passthrough is not very common for PV guests, can
the decision about xen-swiotlb be delayed until you can enumerate
xenstore to check if there are any PCI devices connected (and not
allocate xen-swiotlb by default if there are none)? This would
still not cover the hotplug case (in which case, you'd need to force it
with a cmdline), but at least you wouldn't loose much memory just
because one of your VMs may use PCI passthrough (so, you have it enabled
in your kernel).
Please remember that guest kernel is not always under full control of
the host admin, so making guests loose 64MB of RAM always, in default
setup isn't good for customers of such VMs...

-- 
Best Regards,
Marek Marczykowski-G?recki
Invisible Things Lab
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL:
<https://lists.freedesktop.org/archives/nouveau/attachments/20230519/5fdd14e7/attachment-0001.sig>

On Fri, May 19, 2023 at 12:10:26PM +0200, Marek Marczykowski-G?recki
wrote:
> While I would say PCI passthrough is not very common for PV guests, can
> the decision about xen-swiotlb be delayed until you can enumerate
> xenstore to check if there are any PCI devices connected (and not
> allocate xen-swiotlb by default if there are none)? This would
> still not cover the hotplug case (in which case, you'd need to force it
> with a cmdline), but at least you wouldn't loose much memory just
> because one of your VMs may use PCI passthrough (so, you have it enabled
> in your kernel).
How early can we query xenstore?  We'd need to do this before setting
up DMA for any device.

The alternative would be to finally merge swiotlb-xen into swiotlb, in
which case we might be able to do this later.  Let me see what I can
do there.

On 19.05.23 12:10, Marek Marczykowski-G?recki wrote:
> On Fri, May 19, 2023 at 06:04:05AM +0200, Christoph Hellwig wrote:
>> On Thu, May 18, 2023 at 08:18:39PM +0200, Marek Marczykowski-G?recki
wrote:
>>> On Thu, May 18, 2023 at 03:42:51PM +0200, Christoph Hellwig wrote:
>>>> Remove the dangerous late initialization of xen-swiotlb in
>>>> pci_xen_swiotlb_init_late and instead just always initialize
>>>> xen-swiotlb in the boot code if CONFIG_XEN_PCIDEV_FRONTEND is
enabled.
>>>>
>>>> Signed-off-by: Christoph Hellwig <hch at lst.de>
>>>
>>> Doesn't it mean all the PV guests will basically waste 64MB of
RAM
>>> by default each if they don't really have PCI devices?
>>
>> If CONFIG_XEN_PCIDEV_FRONTEND is enabled, and the kernel's
isn't booted
>> with swiotlb=noforce, yes.
> 
> That's "a bit" unfortunate, since that might be significant
part of the
> VM memory, or if you have a lot of VMs, a significant part of the host
> memory - it quickly adds up.
> While I would say PCI passthrough is not very common for PV guests, can
> the decision about xen-swiotlb be delayed until you can enumerate
> xenstore to check if there are any PCI devices connected (and not
> allocate xen-swiotlb by default if there are none)? This would
> still not cover the hotplug case (in which case, you'd need to force it
> with a cmdline), but at least you wouldn't loose much memory just
> because one of your VMs may use PCI passthrough (so, you have it enabled
> in your kernel).
> Please remember that guest kernel is not always under full control of
> the host admin, so making guests loose 64MB of RAM always, in default
> setup isn't good for customers of such VMs...
> 
In normal cases PCI passthrough in PV guests requires to start the guest
with e820_host=1. So it should be rather easy to limit allocating the
64MB in PV guests to the cases where the memory map has non-RAM regions
especially in the first 1MB of the memory.

This will cover even hotplug cases. The only case not covered would be a
guest started with e820_host=1 even if no PCI passthrough was planned.
But this should be rather rare (at least I hope so).


Juergen
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_0xB0DE9DD628BF132F.asc
Type: application/pgp-keys
Size: 3098 bytes
Desc: OpenPGP public key
URL:
<https://lists.freedesktop.org/archives/nouveau/attachments/20230522/29e95421/attachment.key>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 495 bytes
Desc: OpenPGP digital signature
URL:
<https://lists.freedesktop.org/archives/nouveau/attachments/20230522/29e95421/attachment.sig>