* unexport swiotlb_active
@ 2023-05-18 13:42 Christoph Hellwig
2023-05-18 13:42 ` [PATCH 1/4] x86: move a check out of pci_xen_swiotlb_init Christoph Hellwig
` (3 more replies)
0 siblings, 4 replies; 22+ messages in thread
From: Christoph Hellwig @ 2023-05-18 13:42 UTC (permalink / raw)
To: Juergen Gross, Stefano Stabellini, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Ben Skeggs,
Karol Herbst, Lyude Paul
Cc: xen-devel, iommu, linux-kernel, nouveau
Hi all,
this little series removes the last swiotlb API exposed to modules.
Diffstat:
arch/x86/include/asm/xen/swiotlb-xen.h | 6 ------
arch/x86/kernel/pci-dma.c | 28 ++++------------------------
drivers/gpu/drm/nouveau/nouveau_ttm.c | 10 +++-------
drivers/pci/xen-pcifront.c | 6 ------
kernel/dma/swiotlb.c | 1 -
5 files changed, 7 insertions(+), 44 deletions(-)
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 1/4] x86: move a check out of pci_xen_swiotlb_init
2023-05-18 13:42 unexport swiotlb_active Christoph Hellwig
@ 2023-05-18 13:42 ` Christoph Hellwig
2023-05-18 13:42 ` [PATCH 2/4] x86: always initialize xen-swiotlb when xen-pcifront is enabling Christoph Hellwig
` (2 subsequent siblings)
3 siblings, 0 replies; 22+ messages in thread
From: Christoph Hellwig @ 2023-05-18 13:42 UTC (permalink / raw)
To: Juergen Gross, Stefano Stabellini, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Ben Skeggs,
Karol Herbst, Lyude Paul
Cc: xen-devel, iommu, linux-kernel, nouveau
Move the exact checks when to initialize the Xen swiotlb code out
of pci_xen_swiotlb_init and into the caller so that is uses readable
positive checks, rather than negative ones that will get even more
confusing with another addition.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
arch/x86/kernel/pci-dma.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kernel/pci-dma.c b/arch/x86/kernel/pci-dma.c
index de6be0a3965ee4..f887b08ac5ffe4 100644
--- a/arch/x86/kernel/pci-dma.c
+++ b/arch/x86/kernel/pci-dma.c
@@ -74,8 +74,6 @@ static inline void __init pci_swiotlb_detect(void)
#ifdef CONFIG_SWIOTLB_XEN
static void __init pci_xen_swiotlb_init(void)
{
- if (!xen_initial_domain() && !x86_swiotlb_enable)
- return;
x86_swiotlb_enable = true;
x86_swiotlb_flags |= SWIOTLB_ANY;
swiotlb_init_remap(true, x86_swiotlb_flags, xen_swiotlb_fixup);
@@ -113,7 +111,8 @@ static inline void __init pci_xen_swiotlb_init(void)
void __init pci_iommu_alloc(void)
{
if (xen_pv_domain()) {
- pci_xen_swiotlb_init();
+ if (xen_initial_domain() || x86_swiotlb_enable)
+ pci_xen_swiotlb_init();
return;
}
pci_swiotlb_detect();
--
2.39.2
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 2/4] x86: always initialize xen-swiotlb when xen-pcifront is enabling
2023-05-18 13:42 unexport swiotlb_active Christoph Hellwig
2023-05-18 13:42 ` [PATCH 1/4] x86: move a check out of pci_xen_swiotlb_init Christoph Hellwig
@ 2023-05-18 13:42 ` Christoph Hellwig
2023-05-18 18:18 ` Marek Marczykowski-Górecki
2023-05-18 13:42 ` [PATCH 3/4] drm/nouveau: stop using is_swiotlb_active Christoph Hellwig
2023-05-18 13:42 ` [PATCH 4/4] swiotlb: unexport is_swiotlb_active Christoph Hellwig
3 siblings, 1 reply; 22+ messages in thread
From: Christoph Hellwig @ 2023-05-18 13:42 UTC (permalink / raw)
To: Juergen Gross, Stefano Stabellini, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Ben Skeggs,
Karol Herbst, Lyude Paul
Cc: xen-devel, iommu, linux-kernel, nouveau
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@lst.de>
---
arch/x86/include/asm/xen/swiotlb-xen.h | 6 ------
arch/x86/kernel/pci-dma.c | 25 +++----------------------
drivers/pci/xen-pcifront.c | 6 ------
3 files changed, 3 insertions(+), 34 deletions(-)
diff --git a/arch/x86/include/asm/xen/swiotlb-xen.h b/arch/x86/include/asm/xen/swiotlb-xen.h
index 77a2d19cc9909e..abde0f44df57dc 100644
--- a/arch/x86/include/asm/xen/swiotlb-xen.h
+++ b/arch/x86/include/asm/xen/swiotlb-xen.h
@@ -2,12 +2,6 @@
#ifndef _ASM_X86_SWIOTLB_XEN_H
#define _ASM_X86_SWIOTLB_XEN_H
-#ifdef CONFIG_SWIOTLB_XEN
-extern int pci_xen_swiotlb_init_late(void);
-#else
-static inline int pci_xen_swiotlb_init_late(void) { return -ENXIO; }
-#endif
-
int xen_swiotlb_fixup(void *buf, unsigned long nslabs);
int xen_create_contiguous_region(phys_addr_t pstart, unsigned int order,
unsigned int address_bits,
diff --git a/arch/x86/kernel/pci-dma.c b/arch/x86/kernel/pci-dma.c
index f887b08ac5ffe4..c4a7ead9eb674e 100644
--- a/arch/x86/kernel/pci-dma.c
+++ b/arch/x86/kernel/pci-dma.c
@@ -81,27 +81,6 @@ static void __init pci_xen_swiotlb_init(void)
if (IS_ENABLED(CONFIG_PCI))
pci_request_acs();
}
-
-int pci_xen_swiotlb_init_late(void)
-{
- if (dma_ops == &xen_swiotlb_dma_ops)
- return 0;
-
- /* we can work with the default swiotlb */
- if (!io_tlb_default_mem.nslabs) {
- int rc = swiotlb_init_late(swiotlb_size_or_default(),
- GFP_KERNEL, xen_swiotlb_fixup);
- if (rc < 0)
- return rc;
- }
-
- /* XXX: this switches the dma ops under live devices! */
- dma_ops = &xen_swiotlb_dma_ops;
- if (IS_ENABLED(CONFIG_PCI))
- pci_request_acs();
- return 0;
-}
-EXPORT_SYMBOL_GPL(pci_xen_swiotlb_init_late);
#else
static inline void __init pci_xen_swiotlb_init(void)
{
@@ -111,7 +90,9 @@ static inline void __init pci_xen_swiotlb_init(void)
void __init pci_iommu_alloc(void)
{
if (xen_pv_domain()) {
- if (xen_initial_domain() || x86_swiotlb_enable)
+ if (xen_initial_domain() ||
+ IS_ENABLED(CONFIG_XEN_PCIDEV_FRONTEND) ||
+ x86_swiotlb_enable)
pci_xen_swiotlb_init();
return;
}
diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
index 83c0ab50676dff..11636634ae512f 100644
--- a/drivers/pci/xen-pcifront.c
+++ b/drivers/pci/xen-pcifront.c
@@ -22,7 +22,6 @@
#include <linux/bitops.h>
#include <linux/time.h>
#include <linux/ktime.h>
-#include <linux/swiotlb.h>
#include <xen/platform_pci.h>
#include <asm/xen/swiotlb-xen.h>
@@ -669,11 +668,6 @@ static int pcifront_connect_and_init_dma(struct pcifront_device *pdev)
spin_unlock(&pcifront_dev_lock);
- if (!err && !is_swiotlb_active(&pdev->xdev->dev)) {
- err = pci_xen_swiotlb_init_late();
- if (err)
- dev_err(&pdev->xdev->dev, "Could not setup SWIOTLB!\n");
- }
return err;
}
--
2.39.2
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 3/4] drm/nouveau: stop using is_swiotlb_active
2023-05-18 13:42 unexport swiotlb_active Christoph Hellwig
2023-05-18 13:42 ` [PATCH 1/4] x86: move a check out of pci_xen_swiotlb_init Christoph Hellwig
2023-05-18 13:42 ` [PATCH 2/4] x86: always initialize xen-swiotlb when xen-pcifront is enabling Christoph Hellwig
@ 2023-05-18 13:42 ` Christoph Hellwig
2023-05-18 20:30 ` Lyude Paul
2023-05-18 13:42 ` [PATCH 4/4] swiotlb: unexport is_swiotlb_active Christoph Hellwig
3 siblings, 1 reply; 22+ messages in thread
From: Christoph Hellwig @ 2023-05-18 13:42 UTC (permalink / raw)
To: Juergen Gross, Stefano Stabellini, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Ben Skeggs,
Karol Herbst, Lyude Paul
Cc: xen-devel, iommu, linux-kernel, nouveau
Drivers have no business looking into dma-mapping internals and check
what backend is used. Unfortunstely the DRM core is still broken and
tries to do plain page allocations instead of using DMA API allocators
by default and uses various bandaids on when to use dma_alloc_coherent.
Switch nouveau to use the same (broken) scheme as amdgpu and radeon
to remove the last driver user of is_swiotlb_active.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/gpu/drm/nouveau/nouveau_ttm.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_ttm.c b/drivers/gpu/drm/nouveau/nouveau_ttm.c
index 1469a88910e45d..486f39f31a38df 100644
--- a/drivers/gpu/drm/nouveau/nouveau_ttm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_ttm.c
@@ -24,9 +24,9 @@
*/
#include <linux/limits.h>
-#include <linux/swiotlb.h>
#include <drm/ttm/ttm_range_manager.h>
+#include <drm/drm_cache.h>
#include "nouveau_drv.h"
#include "nouveau_gem.h"
@@ -265,7 +265,6 @@ nouveau_ttm_init(struct nouveau_drm *drm)
struct nvkm_pci *pci = device->pci;
struct nvif_mmu *mmu = &drm->client.mmu;
struct drm_device *dev = drm->dev;
- bool need_swiotlb = false;
int typei, ret;
ret = nouveau_ttm_init_host(drm, 0);
@@ -300,13 +299,10 @@ nouveau_ttm_init(struct nouveau_drm *drm)
drm->agp.cma = pci->agp.cma;
}
-#if IS_ENABLED(CONFIG_SWIOTLB) && IS_ENABLED(CONFIG_X86)
- need_swiotlb = is_swiotlb_active(dev->dev);
-#endif
-
ret = ttm_device_init(&drm->ttm.bdev, &nouveau_bo_driver, drm->dev->dev,
dev->anon_inode->i_mapping,
- dev->vma_offset_manager, need_swiotlb,
+ dev->vma_offset_manager,
+ drm_need_swiotlb(drm->client.mmu.dmabits),
drm->client.mmu.dmabits <= 32);
if (ret) {
NV_ERROR(drm, "error initialising bo driver, %d\n", ret);
--
2.39.2
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 4/4] swiotlb: unexport is_swiotlb_active
2023-05-18 13:42 unexport swiotlb_active Christoph Hellwig
` (2 preceding siblings ...)
2023-05-18 13:42 ` [PATCH 3/4] drm/nouveau: stop using is_swiotlb_active Christoph Hellwig
@ 2023-05-18 13:42 ` Christoph Hellwig
3 siblings, 0 replies; 22+ messages in thread
From: Christoph Hellwig @ 2023-05-18 13:42 UTC (permalink / raw)
To: Juergen Gross, Stefano Stabellini, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Ben Skeggs,
Karol Herbst, Lyude Paul
Cc: xen-devel, iommu, linux-kernel, nouveau
Drivers have no business looking at dma-mapping or swiotlb internals.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
kernel/dma/swiotlb.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index af2e304c672c43..9f1fd28264a067 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -921,7 +921,6 @@ bool is_swiotlb_active(struct device *dev)
return mem && mem->nslabs;
}
-EXPORT_SYMBOL_GPL(is_swiotlb_active);
#ifdef CONFIG_DEBUG_FS
--
2.39.2
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH 2/4] x86: always initialize xen-swiotlb when xen-pcifront is enabling
2023-05-18 13:42 ` [PATCH 2/4] x86: always initialize xen-swiotlb when xen-pcifront is enabling Christoph Hellwig
@ 2023-05-18 18:18 ` Marek Marczykowski-Górecki
2023-05-19 4:04 ` Christoph Hellwig
0 siblings, 1 reply; 22+ messages in thread
From: Marek Marczykowski-Górecki @ 2023-05-18 18:18 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Juergen Gross, Stefano Stabellini, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Ben Skeggs,
Karol Herbst, Lyude Paul, xen-devel, iommu, linux-kernel, nouveau
[-- Attachment #1: Type: text/plain, Size: 3582 bytes --]
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@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?
> ---
> arch/x86/include/asm/xen/swiotlb-xen.h | 6 ------
> arch/x86/kernel/pci-dma.c | 25 +++----------------------
> drivers/pci/xen-pcifront.c | 6 ------
> 3 files changed, 3 insertions(+), 34 deletions(-)
>
> diff --git a/arch/x86/include/asm/xen/swiotlb-xen.h b/arch/x86/include/asm/xen/swiotlb-xen.h
> index 77a2d19cc9909e..abde0f44df57dc 100644
> --- a/arch/x86/include/asm/xen/swiotlb-xen.h
> +++ b/arch/x86/include/asm/xen/swiotlb-xen.h
> @@ -2,12 +2,6 @@
> #ifndef _ASM_X86_SWIOTLB_XEN_H
> #define _ASM_X86_SWIOTLB_XEN_H
>
> -#ifdef CONFIG_SWIOTLB_XEN
> -extern int pci_xen_swiotlb_init_late(void);
> -#else
> -static inline int pci_xen_swiotlb_init_late(void) { return -ENXIO; }
> -#endif
> -
> int xen_swiotlb_fixup(void *buf, unsigned long nslabs);
> int xen_create_contiguous_region(phys_addr_t pstart, unsigned int order,
> unsigned int address_bits,
> diff --git a/arch/x86/kernel/pci-dma.c b/arch/x86/kernel/pci-dma.c
> index f887b08ac5ffe4..c4a7ead9eb674e 100644
> --- a/arch/x86/kernel/pci-dma.c
> +++ b/arch/x86/kernel/pci-dma.c
> @@ -81,27 +81,6 @@ static void __init pci_xen_swiotlb_init(void)
> if (IS_ENABLED(CONFIG_PCI))
> pci_request_acs();
> }
> -
> -int pci_xen_swiotlb_init_late(void)
> -{
> - if (dma_ops == &xen_swiotlb_dma_ops)
> - return 0;
> -
> - /* we can work with the default swiotlb */
> - if (!io_tlb_default_mem.nslabs) {
> - int rc = swiotlb_init_late(swiotlb_size_or_default(),
> - GFP_KERNEL, xen_swiotlb_fixup);
> - if (rc < 0)
> - return rc;
> - }
> -
> - /* XXX: this switches the dma ops under live devices! */
> - dma_ops = &xen_swiotlb_dma_ops;
> - if (IS_ENABLED(CONFIG_PCI))
> - pci_request_acs();
> - return 0;
> -}
> -EXPORT_SYMBOL_GPL(pci_xen_swiotlb_init_late);
> #else
> static inline void __init pci_xen_swiotlb_init(void)
> {
> @@ -111,7 +90,9 @@ static inline void __init pci_xen_swiotlb_init(void)
> void __init pci_iommu_alloc(void)
> {
> if (xen_pv_domain()) {
> - if (xen_initial_domain() || x86_swiotlb_enable)
> + if (xen_initial_domain() ||
> + IS_ENABLED(CONFIG_XEN_PCIDEV_FRONTEND) ||
> + x86_swiotlb_enable)
> pci_xen_swiotlb_init();
> return;
> }
> diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
> index 83c0ab50676dff..11636634ae512f 100644
> --- a/drivers/pci/xen-pcifront.c
> +++ b/drivers/pci/xen-pcifront.c
> @@ -22,7 +22,6 @@
> #include <linux/bitops.h>
> #include <linux/time.h>
> #include <linux/ktime.h>
> -#include <linux/swiotlb.h>
> #include <xen/platform_pci.h>
>
> #include <asm/xen/swiotlb-xen.h>
> @@ -669,11 +668,6 @@ static int pcifront_connect_and_init_dma(struct pcifront_device *pdev)
>
> spin_unlock(&pcifront_dev_lock);
>
> - if (!err && !is_swiotlb_active(&pdev->xdev->dev)) {
> - err = pci_xen_swiotlb_init_late();
> - if (err)
> - dev_err(&pdev->xdev->dev, "Could not setup SWIOTLB!\n");
> - }
> return err;
> }
>
> --
> 2.39.2
>
>
--
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/4] drm/nouveau: stop using is_swiotlb_active
2023-05-18 13:42 ` [PATCH 3/4] drm/nouveau: stop using is_swiotlb_active Christoph Hellwig
@ 2023-05-18 20:30 ` Lyude Paul
2023-06-07 13:11 ` Christoph Hellwig
0 siblings, 1 reply; 22+ messages in thread
From: Lyude Paul @ 2023-05-18 20:30 UTC (permalink / raw)
To: Christoph Hellwig, Juergen Gross, Stefano Stabellini,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Ben Skeggs, Karol Herbst
Cc: xen-devel, iommu, linux-kernel, nouveau
Reviewed-by: Lyude Paul <lyude@redhat.com>
Thanks for getting to this!
On Thu, 2023-05-18 at 15:42 +0200, Christoph Hellwig wrote:
> Drivers have no business looking into dma-mapping internals and check
> what backend is used. Unfortunstely the DRM core is still broken and
> tries to do plain page allocations instead of using DMA API allocators
> by default and uses various bandaids on when to use dma_alloc_coherent.
>
> Switch nouveau to use the same (broken) scheme as amdgpu and radeon
> to remove the last driver user of is_swiotlb_active.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> drivers/gpu/drm/nouveau/nouveau_ttm.c | 10 +++-------
> 1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_ttm.c b/drivers/gpu/drm/nouveau/nouveau_ttm.c
> index 1469a88910e45d..486f39f31a38df 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_ttm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_ttm.c
> @@ -24,9 +24,9 @@
> */
>
> #include <linux/limits.h>
> -#include <linux/swiotlb.h>
>
> #include <drm/ttm/ttm_range_manager.h>
> +#include <drm/drm_cache.h>
>
> #include "nouveau_drv.h"
> #include "nouveau_gem.h"
> @@ -265,7 +265,6 @@ nouveau_ttm_init(struct nouveau_drm *drm)
> struct nvkm_pci *pci = device->pci;
> struct nvif_mmu *mmu = &drm->client.mmu;
> struct drm_device *dev = drm->dev;
> - bool need_swiotlb = false;
> int typei, ret;
>
> ret = nouveau_ttm_init_host(drm, 0);
> @@ -300,13 +299,10 @@ nouveau_ttm_init(struct nouveau_drm *drm)
> drm->agp.cma = pci->agp.cma;
> }
>
> -#if IS_ENABLED(CONFIG_SWIOTLB) && IS_ENABLED(CONFIG_X86)
> - need_swiotlb = is_swiotlb_active(dev->dev);
> -#endif
> -
> ret = ttm_device_init(&drm->ttm.bdev, &nouveau_bo_driver, drm->dev->dev,
> dev->anon_inode->i_mapping,
> - dev->vma_offset_manager, need_swiotlb,
> + dev->vma_offset_manager,
> + drm_need_swiotlb(drm->client.mmu.dmabits),
> drm->client.mmu.dmabits <= 32);
> if (ret) {
> NV_ERROR(drm, "error initialising bo driver, %d\n", ret);
--
Cheers,
Lyude Paul (she/her)
Software Engineer at Red Hat
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/4] x86: always initialize xen-swiotlb when xen-pcifront is enabling
2023-05-18 18:18 ` Marek Marczykowski-Górecki
@ 2023-05-19 4:04 ` Christoph Hellwig
2023-05-19 10:10 ` Marek Marczykowski-Górecki
0 siblings, 1 reply; 22+ messages in thread
From: Christoph Hellwig @ 2023-05-19 4:04 UTC (permalink / raw)
To: Marek Marczykowski-Górecki
Cc: Christoph Hellwig, Juergen Gross, Stefano Stabellini,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Ben Skeggs, Karol Herbst, Lyude Paul, xen-devel,
iommu, linux-kernel, nouveau
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@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.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/4] x86: always initialize xen-swiotlb when xen-pcifront is enabling
2023-05-19 4:04 ` Christoph Hellwig
@ 2023-05-19 10:10 ` Marek Marczykowski-Górecki
2023-05-19 12:41 ` Christoph Hellwig
` (2 more replies)
0 siblings, 3 replies; 22+ messages in thread
From: Marek Marczykowski-Górecki @ 2023-05-19 10:10 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Juergen Gross, Stefano Stabellini, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Ben Skeggs,
Karol Herbst, Lyude Paul, xen-devel, iommu, linux-kernel, nouveau
[-- Attachment #1: Type: text/plain, Size: 1682 bytes --]
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@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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/4] x86: always initialize xen-swiotlb when xen-pcifront is enabling
2023-05-19 10:10 ` Marek Marczykowski-Górecki
@ 2023-05-19 12:41 ` Christoph Hellwig
2023-05-19 12:49 ` Andrew Cooper
2023-05-22 7:54 ` Petr Tesařík
2023-05-22 8:37 ` Juergen Gross
2 siblings, 1 reply; 22+ messages in thread
From: Christoph Hellwig @ 2023-05-19 12:41 UTC (permalink / raw)
To: Marek Marczykowski-Górecki
Cc: Christoph Hellwig, Juergen Gross, Stefano Stabellini,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Ben Skeggs, Karol Herbst, Lyude Paul, xen-devel,
iommu, linux-kernel, nouveau
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.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/4] x86: always initialize xen-swiotlb when xen-pcifront is enabling
2023-05-19 12:41 ` Christoph Hellwig
@ 2023-05-19 12:49 ` Andrew Cooper
2023-05-19 12:58 ` Christoph Hellwig
0 siblings, 1 reply; 22+ messages in thread
From: Andrew Cooper @ 2023-05-19 12:49 UTC (permalink / raw)
To: Christoph Hellwig, Marek Marczykowski-Górecki
Cc: Juergen Gross, Stefano Stabellini, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Ben Skeggs,
Karol Herbst, Lyude Paul, xen-devel, iommu, linux-kernel, nouveau
On 19/05/2023 1:41 pm, Christoph Hellwig wrote:
> 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.
Not that early. One supported configuration has xenstore not starting
for an indefinite period of time after boot.
> 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.
If that is an option, it would be great to reduce the special-cashing.
~Andrew
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/4] x86: always initialize xen-swiotlb when xen-pcifront is enabling
2023-05-19 12:49 ` Andrew Cooper
@ 2023-05-19 12:58 ` Christoph Hellwig
2023-05-20 6:21 ` Christoph Hellwig
0 siblings, 1 reply; 22+ messages in thread
From: Christoph Hellwig @ 2023-05-19 12:58 UTC (permalink / raw)
To: Andrew Cooper
Cc: Christoph Hellwig, Marek Marczykowski-Górecki, Juergen Gross,
Stefano Stabellini, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, H. Peter Anvin, Ben Skeggs, Karol Herbst,
Lyude Paul, xen-devel, iommu, linux-kernel, nouveau
On Fri, May 19, 2023 at 01:49:46PM +0100, Andrew Cooper wrote:
> > 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.
>
> If that is an option, it would be great to reduce the special-cashing.
I think it's doable, and I've been wanting it for a while. I just
need motivated testers, but it seems like I just found at least two :)
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/4] x86: always initialize xen-swiotlb when xen-pcifront is enabling
2023-05-19 12:58 ` Christoph Hellwig
@ 2023-05-20 6:21 ` Christoph Hellwig
2023-05-22 7:52 ` Petr Tesařík
0 siblings, 1 reply; 22+ messages in thread
From: Christoph Hellwig @ 2023-05-20 6:21 UTC (permalink / raw)
To: Andrew Cooper
Cc: Christoph Hellwig, Marek Marczykowski-Górecki, Juergen Gross,
Stefano Stabellini, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, H. Peter Anvin, Ben Skeggs, Karol Herbst,
Lyude Paul, xen-devel, iommu, linux-kernel, nouveau
On Fri, May 19, 2023 at 02:58:57PM +0200, Christoph Hellwig wrote:
> On Fri, May 19, 2023 at 01:49:46PM +0100, Andrew Cooper wrote:
> > > 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.
> >
> > If that is an option, it would be great to reduce the special-cashing.
>
> I think it's doable, and I've been wanting it for a while. I just
> need motivated testers, but it seems like I just found at least two :)
So looking at swiotlb-xen it does these off things where it takes a value
generated originally be xen_phys_to_dma, then only does a dma_to_phys
to go back and call pfn_valid on the result. Does this make sense, or
is it wrong and just works by accident? I.e. is the patch below correct?
diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
index 67aa74d201627d..3396c5766f0dd8 100644
--- a/drivers/xen/swiotlb-xen.c
+++ b/drivers/xen/swiotlb-xen.c
@@ -90,9 +90,7 @@ static inline int range_straddles_page_boundary(phys_addr_t p, size_t size)
static int is_xen_swiotlb_buffer(struct device *dev, dma_addr_t dma_addr)
{
- unsigned long bfn = XEN_PFN_DOWN(dma_to_phys(dev, dma_addr));
- unsigned long xen_pfn = bfn_to_local_pfn(bfn);
- phys_addr_t paddr = (phys_addr_t)xen_pfn << XEN_PAGE_SHIFT;
+ phys_addr_t paddr = xen_dma_to_phys(dev, dma_addr);
/* If the address is outside our domain, it CAN
* have the same virtual address as another address
@@ -234,7 +232,7 @@ static dma_addr_t xen_swiotlb_map_page(struct device *dev, struct page *page,
done:
if (!dev_is_dma_coherent(dev) && !(attrs & DMA_ATTR_SKIP_CPU_SYNC)) {
- if (pfn_valid(PFN_DOWN(dma_to_phys(dev, dev_addr))))
+ if (pfn_valid(PFN_DOWN(phys)))
arch_sync_dma_for_device(phys, size, dir);
else
xen_dma_sync_for_device(dev, dev_addr, size, dir);
@@ -258,7 +256,7 @@ static void xen_swiotlb_unmap_page(struct device *hwdev, dma_addr_t dev_addr,
BUG_ON(dir == DMA_NONE);
if (!dev_is_dma_coherent(hwdev) && !(attrs & DMA_ATTR_SKIP_CPU_SYNC)) {
- if (pfn_valid(PFN_DOWN(dma_to_phys(hwdev, dev_addr))))
+ if (pfn_valid(PFN_DOWN(paddr)))
arch_sync_dma_for_cpu(paddr, size, dir);
else
xen_dma_sync_for_cpu(hwdev, dev_addr, size, dir);
@@ -276,7 +274,7 @@ xen_swiotlb_sync_single_for_cpu(struct device *dev, dma_addr_t dma_addr,
phys_addr_t paddr = xen_dma_to_phys(dev, dma_addr);
if (!dev_is_dma_coherent(dev)) {
- if (pfn_valid(PFN_DOWN(dma_to_phys(dev, dma_addr))))
+ if (pfn_valid(PFN_DOWN(paddr)))
arch_sync_dma_for_cpu(paddr, size, dir);
else
xen_dma_sync_for_cpu(dev, dma_addr, size, dir);
@@ -296,7 +294,7 @@ xen_swiotlb_sync_single_for_device(struct device *dev, dma_addr_t dma_addr,
swiotlb_sync_single_for_device(dev, paddr, size, dir);
if (!dev_is_dma_coherent(dev)) {
- if (pfn_valid(PFN_DOWN(dma_to_phys(dev, dma_addr))))
+ if (pfn_valid(PFN_DOWN(paddr)))
arch_sync_dma_for_device(paddr, size, dir);
else
xen_dma_sync_for_device(dev, dma_addr, size, dir);
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH 2/4] x86: always initialize xen-swiotlb when xen-pcifront is enabling
2023-05-20 6:21 ` Christoph Hellwig
@ 2023-05-22 7:52 ` Petr Tesařík
0 siblings, 0 replies; 22+ messages in thread
From: Petr Tesařík @ 2023-05-22 7:52 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Andrew Cooper, Marek Marczykowski-Górecki, Juergen Gross,
Stefano Stabellini, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, H. Peter Anvin, Ben Skeggs, Karol Herbst,
Lyude Paul, xen-devel, iommu, linux-kernel, nouveau
Hi Christoph,
On Sat, 20 May 2023 08:21:03 +0200
Christoph Hellwig <hch@lst.de> wrote:
> On Fri, May 19, 2023 at 02:58:57PM +0200, Christoph Hellwig wrote:
> > On Fri, May 19, 2023 at 01:49:46PM +0100, Andrew Cooper wrote:
> > > > 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.
> > >
> > > If that is an option, it would be great to reduce the special-cashing.
> >
> > I think it's doable, and I've been wanting it for a while. I just
> > need motivated testers, but it seems like I just found at least two :)
>
> So looking at swiotlb-xen it does these off things where it takes a value
> generated originally be xen_phys_to_dma, then only does a dma_to_phys
> to go back and call pfn_valid on the result. Does this make sense, or
> is it wrong and just works by accident? I.e. is the patch below correct?
>
>
> diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
> index 67aa74d201627d..3396c5766f0dd8 100644
> --- a/drivers/xen/swiotlb-xen.c
> +++ b/drivers/xen/swiotlb-xen.c
> @@ -90,9 +90,7 @@ static inline int range_straddles_page_boundary(phys_addr_t p, size_t size)
>
> static int is_xen_swiotlb_buffer(struct device *dev, dma_addr_t dma_addr)
> {
> - unsigned long bfn = XEN_PFN_DOWN(dma_to_phys(dev, dma_addr));
> - unsigned long xen_pfn = bfn_to_local_pfn(bfn);
> - phys_addr_t paddr = (phys_addr_t)xen_pfn << XEN_PAGE_SHIFT;
> + phys_addr_t paddr = xen_dma_to_phys(dev, dma_addr);
I'm no big Xen expert, but I think this is wrong. Let's go through it
line by line:
- bfn = XEN_PFN_DOWN(dma_to_phys(dev, dma_addr));
Take the DMA address (as seen by devices on the bus), convert it to a
physical address (as seen by the CPU on the bus) and shift it right
by XEN_PAGE_SHIFT. The result is a Xen machine PFN.
- xen_pfn = bfn_to_local_pfn(bfn);
Take the machine PFN and converts it to a physical PFN.
- paddr = (phys_addr_t)xen_pfn << XEN_PAGE_SHIFT;
Convert the physical PFN to a physical address.
The important thing here is that Xen PV does not have auto-translated
physical addresses, so physical address != machine address. Physical
addresses in Xen PV domains are "artificial", used by the kernel to
index the mem_map array, so a PFN can be easily converted to a struct
page pointer and back. However, these addresses are never used by
hardware, not even by CPU. The addresses used by the CPU are called
machine addresses. There is no address translation between VCPUs and
CPUs, because a PV domain runs directly on the CPU. After all, that's
why it is called _para_virtualized.
HTH
Petr T
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/4] x86: always initialize xen-swiotlb when xen-pcifront is enabling
2023-05-19 10:10 ` Marek Marczykowski-Górecki
2023-05-19 12:41 ` Christoph Hellwig
@ 2023-05-22 7:54 ` Petr Tesařík
2023-05-22 8:37 ` Juergen Gross
2 siblings, 0 replies; 22+ messages in thread
From: Petr Tesařík @ 2023-05-22 7:54 UTC (permalink / raw)
To: Marek Marczykowski-Górecki
Cc: Christoph Hellwig, Juergen Gross, Stefano Stabellini,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Ben Skeggs, Karol Herbst, Lyude Paul, xen-devel,
iommu, linux-kernel, nouveau
[-- Attachment #1: Type: text/plain, Size: 1152 bytes --]
On Fri, 19 May 2023 12:10:26 +0200
Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> 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@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.
I wonder if dynamic swiotlb allocation might also help with this...
Petr T
[-- Attachment #2: Digitální podpis OpenPGP --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/4] x86: always initialize xen-swiotlb when xen-pcifront is enabling
2023-05-19 10:10 ` Marek Marczykowski-Górecki
2023-05-19 12:41 ` Christoph Hellwig
2023-05-22 7:54 ` Petr Tesařík
@ 2023-05-22 8:37 ` Juergen Gross
2023-06-07 13:12 ` Christoph Hellwig
2 siblings, 1 reply; 22+ messages in thread
From: Juergen Gross @ 2023-05-22 8:37 UTC (permalink / raw)
To: Marek Marczykowski-Górecki, Christoph Hellwig
Cc: Stefano Stabellini, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, H. Peter Anvin, Ben Skeggs, Karol Herbst,
Lyude Paul, xen-devel, iommu, linux-kernel, nouveau
[-- Attachment #1.1.1: Type: text/plain, Size: 2175 bytes --]
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@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
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3149 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/4] drm/nouveau: stop using is_swiotlb_active
2023-05-18 20:30 ` Lyude Paul
@ 2023-06-07 13:11 ` Christoph Hellwig
0 siblings, 0 replies; 22+ messages in thread
From: Christoph Hellwig @ 2023-06-07 13:11 UTC (permalink / raw)
To: Lyude Paul
Cc: Christoph Hellwig, Juergen Gross, Stefano Stabellini,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Ben Skeggs, Karol Herbst, xen-devel, iommu,
linux-kernel, nouveau
On Thu, May 18, 2023 at 04:30:49PM -0400, Lyude Paul wrote:
> Reviewed-by: Lyude Paul <lyude@redhat.com>
>
> Thanks for getting to this!
I've tentantively queued this up in the dma-mapping for-next tree.
Let me know if you'd prefer it to go through the nouveau tree.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/4] x86: always initialize xen-swiotlb when xen-pcifront is enabling
2023-05-22 8:37 ` Juergen Gross
@ 2023-06-07 13:12 ` Christoph Hellwig
2023-06-09 15:38 ` Juergen Gross
0 siblings, 1 reply; 22+ messages in thread
From: Christoph Hellwig @ 2023-06-07 13:12 UTC (permalink / raw)
To: Juergen Gross
Cc: Marek Marczykowski-Górecki, Christoph Hellwig,
Stefano Stabellini, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, H. Peter Anvin, Ben Skeggs, Karol Herbst,
Lyude Paul, xen-devel, iommu, linux-kernel, nouveau
On Mon, May 22, 2023 at 10:37:09AM +0200, Juergen Gross wrote:
> 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).
So is this an ACK for the patch and can we go ahead with it?
(I'd still like to merge swiotlb-xen into swiotlb eventually, but it's
probably not going to happen this merge window)
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/4] x86: always initialize xen-swiotlb when xen-pcifront is enabling
2023-06-07 13:12 ` Christoph Hellwig
@ 2023-06-09 15:38 ` Juergen Gross
2023-06-12 6:47 ` Christoph Hellwig
2023-06-12 8:08 ` Juergen Gross
0 siblings, 2 replies; 22+ messages in thread
From: Juergen Gross @ 2023-06-09 15:38 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Marek Marczykowski-Górecki, Stefano Stabellini,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Ben Skeggs, Karol Herbst, Lyude Paul, xen-devel,
iommu, linux-kernel, nouveau
[-- Attachment #1.1.1: Type: text/plain, Size: 967 bytes --]
On 07.06.23 15:12, Christoph Hellwig wrote:
> On Mon, May 22, 2023 at 10:37:09AM +0200, Juergen Gross wrote:
>> 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).
>
> So is this an ACK for the patch and can we go ahead with it?
As long as above mentioned check of the E820 map is done, yes.
If you want I can send a diff to be folded into your patch on Monday.
>
> (I'd still like to merge swiotlb-xen into swiotlb eventually, but it's
> probably not going to happen this merge window)
Would be nice.
Juergen
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3149 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/4] x86: always initialize xen-swiotlb when xen-pcifront is enabling
2023-06-09 15:38 ` Juergen Gross
@ 2023-06-12 6:47 ` Christoph Hellwig
2023-06-12 8:08 ` Juergen Gross
1 sibling, 0 replies; 22+ messages in thread
From: Christoph Hellwig @ 2023-06-12 6:47 UTC (permalink / raw)
To: Juergen Gross
Cc: Christoph Hellwig, Marek Marczykowski-Górecki,
Stefano Stabellini, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, H. Peter Anvin, Ben Skeggs, Karol Herbst,
Lyude Paul, xen-devel, iommu, linux-kernel, nouveau
On Fri, Jun 09, 2023 at 05:38:28PM +0200, Juergen Gross wrote:
>>> guest started with e820_host=1 even if no PCI passthrough was planned.
>>> But this should be rather rare (at least I hope so).
>>
>> So is this an ACK for the patch and can we go ahead with it?
>
> As long as above mentioned check of the E820 map is done, yes.
>
> If you want I can send a diff to be folded into your patch on Monday.
Yes, that would be great!
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/4] x86: always initialize xen-swiotlb when xen-pcifront is enabling
2023-06-09 15:38 ` Juergen Gross
2023-06-12 6:47 ` Christoph Hellwig
@ 2023-06-12 8:08 ` Juergen Gross
2023-06-12 8:23 ` Christoph Hellwig
1 sibling, 1 reply; 22+ messages in thread
From: Juergen Gross @ 2023-06-12 8:08 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Marek Marczykowski-Górecki, Stefano Stabellini,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Ben Skeggs, Karol Herbst, Lyude Paul, xen-devel,
iommu, linux-kernel, nouveau
[-- Attachment #1.1.1: Type: text/plain, Size: 1455 bytes --]
On 09.06.23 17:38, Juergen Gross wrote:
> On 07.06.23 15:12, Christoph Hellwig wrote:
>> On Mon, May 22, 2023 at 10:37:09AM +0200, Juergen Gross wrote:
>>> 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).
>>
>> So is this an ACK for the patch and can we go ahead with it?
>
> As long as above mentioned check of the E820 map is done, yes.
>
> If you want I can send a diff to be folded into your patch on Monday.
Attached is a patch for adding a new flag xen_pv_pci_possible. You can
either add the patch to your series or merge it into your patch 2.
You need to modify your patch like this:
@@ -111,7 +90,10 @@ static inline void __init pci_xen_swiotlb_init(void)
void __init pci_iommu_alloc(void)
{
if (xen_pv_domain()) {
- if (xen_initial_domain() || x86_swiotlb_enable)
+ if (xen_initial_domain() ||
+ (IS_ENABLED(CONFIG_XEN_PCIDEV_FRONTEND) &&
+ xen_pv_pci_possible) ||
+ x86_swiotlb_enable)
pci_xen_swiotlb_init();
return;
}
Juergen
[-- Attachment #1.1.2: 0001-xen-pci-add-flag-for-PCI-passthrough-being-possible.patch --]
[-- Type: text/x-patch, Size: 1903 bytes --]
From 7e84a88243b57bc90d1ef6bd42661f499886e659 Mon Sep 17 00:00:00 2001
From: Juergen Gross <jgross@suse.com>
Date: Mon, 12 Jun 2023 09:57:18 +0200
Subject: [PATCH] xen/pci: add flag for PCI passthrough being possible
When running as a Xen PV guests passed through PCI devices only have a
chance to work if the Xen supplied memory map has some PCI space
reserved.
Add a flag xen_pv_pci_possible which will be set in early boot in case
the memory map has at least one area with the type E820_TYPE_RESERVED.
Signed-off-by: Juergen Gross <jgross@suse.com>
---
arch/x86/xen/setup.c | 6 ++++++
include/xen/xen.h | 6 ++++++
2 files changed, 12 insertions(+)
diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c
index c2be3efb2ba0..716f76c41416 100644
--- a/arch/x86/xen/setup.c
+++ b/arch/x86/xen/setup.c
@@ -43,6 +43,9 @@ struct xen_memory_region xen_extra_mem[XEN_EXTRA_MEM_MAX_REGIONS] __initdata;
/* Number of pages released from the initial allocation. */
unsigned long xen_released_pages;
+/* Memory map would allow PCI passthrough. */
+bool xen_pv_pci_possible;
+
/* E820 map used during setting up memory. */
static struct e820_table xen_e820_table __initdata;
@@ -804,6 +807,9 @@ char * __init xen_memory_setup(void)
chunk_size = size;
type = xen_e820_table.entries[i].type;
+ if (type == E820_TYPE_RESERVED)
+ xen_pv_pci_possible = true;
+
if (type == E820_TYPE_RAM) {
if (addr < mem_end) {
chunk_size = min(size, mem_end - addr);
diff --git a/include/xen/xen.h b/include/xen/xen.h
index 0efeb652f9b8..5eb0a974a11e 100644
--- a/include/xen/xen.h
+++ b/include/xen/xen.h
@@ -29,6 +29,12 @@ extern bool xen_pvh;
extern uint32_t xen_start_flags;
+#ifdef CONFIG_XEN_PV
+extern bool xen_pv_pci_possible;
+#else
+#define xen_pv_pci_possible 0
+#endif
+
#include <xen/interface/hvm/start_info.h>
extern struct hvm_start_info pvh_start_info;
--
2.35.3
[-- Attachment #1.1.3: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3149 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH 2/4] x86: always initialize xen-swiotlb when xen-pcifront is enabling
2023-06-12 8:08 ` Juergen Gross
@ 2023-06-12 8:23 ` Christoph Hellwig
0 siblings, 0 replies; 22+ messages in thread
From: Christoph Hellwig @ 2023-06-12 8:23 UTC (permalink / raw)
To: Juergen Gross
Cc: Christoph Hellwig, Marek Marczykowski-Górecki,
Stefano Stabellini, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, H. Peter Anvin, Ben Skeggs, Karol Herbst,
Lyude Paul, xen-devel, iommu, linux-kernel, nouveau
Thank you. I'll queue it up as a separate patch.
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2023-06-12 8:23 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-18 13:42 unexport swiotlb_active Christoph Hellwig
2023-05-18 13:42 ` [PATCH 1/4] x86: move a check out of pci_xen_swiotlb_init Christoph Hellwig
2023-05-18 13:42 ` [PATCH 2/4] x86: always initialize xen-swiotlb when xen-pcifront is enabling Christoph Hellwig
2023-05-18 18:18 ` Marek Marczykowski-Górecki
2023-05-19 4:04 ` Christoph Hellwig
2023-05-19 10:10 ` Marek Marczykowski-Górecki
2023-05-19 12:41 ` Christoph Hellwig
2023-05-19 12:49 ` Andrew Cooper
2023-05-19 12:58 ` Christoph Hellwig
2023-05-20 6:21 ` Christoph Hellwig
2023-05-22 7:52 ` Petr Tesařík
2023-05-22 7:54 ` Petr Tesařík
2023-05-22 8:37 ` Juergen Gross
2023-06-07 13:12 ` Christoph Hellwig
2023-06-09 15:38 ` Juergen Gross
2023-06-12 6:47 ` Christoph Hellwig
2023-06-12 8:08 ` Juergen Gross
2023-06-12 8:23 ` Christoph Hellwig
2023-05-18 13:42 ` [PATCH 3/4] drm/nouveau: stop using is_swiotlb_active Christoph Hellwig
2023-05-18 20:30 ` Lyude Paul
2023-06-07 13:11 ` Christoph Hellwig
2023-05-18 13:42 ` [PATCH 4/4] swiotlb: unexport is_swiotlb_active Christoph Hellwig
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).