Re: [PATCH v11 11/12] vduse: Introduce VDUSE - vDPA Device in Userspace

2021-08-24 Thread Yongji Xie
On Wed, Aug 25, 2021 at 2:10 AM Michael S. Tsirkin  wrote:
>
> On Wed, Aug 18, 2021 at 08:06:41PM +0800, Xie Yongji wrote:
> > This VDUSE driver enables implementing software-emulated vDPA
> > devices in userspace. The vDPA device is created by
> > ioctl(VDUSE_CREATE_DEV) on /dev/vduse/control. Then a char device
> > interface (/dev/vduse/$NAME) is exported to userspace for device
> > emulation.
> >
> > In order to make the device emulation more secure, the device's
> > control path is handled in kernel. A message mechnism is introduced
> > to forward some dataplane related control messages to userspace.
> >
> > And in the data path, the DMA buffer will be mapped into userspace
> > address space through different ways depending on the vDPA bus to
> > which the vDPA device is attached. In virtio-vdpa case, the MMU-based
> > software IOTLB is used to achieve that. And in vhost-vdpa case, the
> > DMA buffer is reside in a userspace memory region which can be shared
> > to the VDUSE userspace processs via transferring the shmfd.
> >
> > For more details on VDUSE design and usage, please see the follow-on
> > Documentation commit.
> >
> > Signed-off-by: Xie Yongji 
>
> Build bot seems unhappy with this patch.
>

Yes, this is because the series relies on the unmerged patch:

https://lore.kernel.org/lkml/20210705071910.31965-1-jasow...@redhat.com/

Do I need to remove this dependency in the next version?

Thanks,
Yongji
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v3 00/14] x86: Trenchboot secure dynamic launch Linux kernel support

2021-08-24 Thread Daniel P. Smith
On 8/10/21 12:23 PM, Jarkko Sakkinen wrote:
> On Mon, Aug 09, 2021 at 12:38:42PM -0400, Ross Philipson wrote:
>> The focus of Trechboot project (https://github.com/TrenchBoot) is to
>> enhance the boot security and integrity. This requires the linux kernel
>  ~
>  Linux
> 
> How does it enhance it? The following sentence explains the requirements
> for the Linux kernel, i.e. it's a question without answer. And if there
> is no answer, there is no need to merge this.

We have added a documentation patch that provides background
information, an overview of the capability, and details about the
implementation. We can reword the cover letter, adding reference to this
documentation. And ack on fixing the incorrect case on Linux.

V/r
dps
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v11 11/12] vduse: Introduce VDUSE - vDPA Device in Userspace

2021-08-24 Thread Michael S. Tsirkin
On Wed, Aug 18, 2021 at 08:06:41PM +0800, Xie Yongji wrote:
> This VDUSE driver enables implementing software-emulated vDPA
> devices in userspace. The vDPA device is created by
> ioctl(VDUSE_CREATE_DEV) on /dev/vduse/control. Then a char device
> interface (/dev/vduse/$NAME) is exported to userspace for device
> emulation.
> 
> In order to make the device emulation more secure, the device's
> control path is handled in kernel. A message mechnism is introduced
> to forward some dataplane related control messages to userspace.
> 
> And in the data path, the DMA buffer will be mapped into userspace
> address space through different ways depending on the vDPA bus to
> which the vDPA device is attached. In virtio-vdpa case, the MMU-based
> software IOTLB is used to achieve that. And in vhost-vdpa case, the
> DMA buffer is reside in a userspace memory region which can be shared
> to the VDUSE userspace processs via transferring the shmfd.
> 
> For more details on VDUSE design and usage, please see the follow-on
> Documentation commit.
> 
> Signed-off-by: Xie Yongji 

Build bot seems unhappy with this patch.

> ---
>  Documentation/userspace-api/ioctl/ioctl-number.rst |1 +
>  drivers/vdpa/Kconfig   |   10 +
>  drivers/vdpa/Makefile  |1 +
>  drivers/vdpa/vdpa_user/Makefile|5 +
>  drivers/vdpa/vdpa_user/vduse_dev.c | 1611 
> 
>  include/uapi/linux/vduse.h |  304 
>  6 files changed, 1932 insertions(+)
>  create mode 100644 drivers/vdpa/vdpa_user/Makefile
>  create mode 100644 drivers/vdpa/vdpa_user/vduse_dev.c
>  create mode 100644 include/uapi/linux/vduse.h
> 
> diff --git a/Documentation/userspace-api/ioctl/ioctl-number.rst 
> b/Documentation/userspace-api/ioctl/ioctl-number.rst
> index 1409e40e6345..293ca3aef358 100644
> --- a/Documentation/userspace-api/ioctl/ioctl-number.rst
> +++ b/Documentation/userspace-api/ioctl/ioctl-number.rst
> @@ -300,6 +300,7 @@ Code  Seq#Include File
>Comments
>  'z'   10-4F  drivers/s390/crypto/zcrypt_api.h
> conflict!
>  '|'   00-7F  linux/media.h
>  0x80  00-1F  linux/fb.h
> +0x81  00-1F  linux/vduse.h
>  0x89  00-06  arch/x86/include/asm/sockios.h
>  0x89  0B-DF  linux/sockios.h
>  0x89  E0-EF  linux/sockios.h 
> SIOCPROTOPRIVATE range
> diff --git a/drivers/vdpa/Kconfig b/drivers/vdpa/Kconfig
> index a503c1b2bfd9..6e23bce6433a 100644
> --- a/drivers/vdpa/Kconfig
> +++ b/drivers/vdpa/Kconfig
> @@ -33,6 +33,16 @@ config VDPA_SIM_BLOCK
> vDPA block device simulator which terminates IO request in a
> memory buffer.
>  
> +config VDPA_USER
> + tristate "VDUSE (vDPA Device in Userspace) support"
> + depends on EVENTFD && MMU && HAS_DMA
> + select DMA_OPS
> + select VHOST_IOTLB
> + select IOMMU_IOVA
> + help
> +   With VDUSE it is possible to emulate a vDPA Device
> +   in a userspace program.
> +
>  config IFCVF
>   tristate "Intel IFC VF vDPA driver"
>   depends on PCI_MSI
> diff --git a/drivers/vdpa/Makefile b/drivers/vdpa/Makefile
> index 67fe7f3d6943..f02ebed33f19 100644
> --- a/drivers/vdpa/Makefile
> +++ b/drivers/vdpa/Makefile
> @@ -1,6 +1,7 @@
>  # SPDX-License-Identifier: GPL-2.0
>  obj-$(CONFIG_VDPA) += vdpa.o
>  obj-$(CONFIG_VDPA_SIM) += vdpa_sim/
> +obj-$(CONFIG_VDPA_USER) += vdpa_user/
>  obj-$(CONFIG_IFCVF)+= ifcvf/
>  obj-$(CONFIG_MLX5_VDPA) += mlx5/
>  obj-$(CONFIG_VP_VDPA)+= virtio_pci/
> diff --git a/drivers/vdpa/vdpa_user/Makefile b/drivers/vdpa/vdpa_user/Makefile
> new file mode 100644
> index ..260e0b26af99
> --- /dev/null
> +++ b/drivers/vdpa/vdpa_user/Makefile
> @@ -0,0 +1,5 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +vduse-y := vduse_dev.o iova_domain.o
> +
> +obj-$(CONFIG_VDPA_USER) += vduse.o
> diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c 
> b/drivers/vdpa/vdpa_user/vduse_dev.c
> new file mode 100644
> index ..ce081b7895d5
> --- /dev/null
> +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> @@ -0,0 +1,1611 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * VDUSE: vDPA Device in Userspace
> + *
> + * Copyright (C) 2020-2021 Bytedance Inc. and/or its affiliates. All rights 
> reserved.
> + *
> + * Author: Xie Yongji 
> + *
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "iova_domain.h"
> +
> +#define DRV_AUTHOR   "Yongji Xie "
> +#define DRV_DESC "vDPA Device in Userspace"
> +#define DRV_LICENSE  "GPL v2"
> +
> +#define VDUSE_DEV_MAX (1U << MINORBITS)
> +#define VDUSE_BOUNCE_SIZE (64 * 1024 * 1024)
> +#define VDUSE_IOVA_SIZE (128 * 1024 * 1024)
> +#define VDUSE_MSG_DEFAULT_TIMEOUT 

Re: [PATCH v11 01/12] iova: Export alloc_iova_fast() and free_iova_fast()

2021-08-24 Thread Michael S. Tsirkin
On Wed, Aug 18, 2021 at 08:06:31PM +0800, Xie Yongji wrote:
> Export alloc_iova_fast() and free_iova_fast() so that
> some modules can make use of the per-CPU cache to get
> rid of rbtree spinlock in alloc_iova() and free_iova()
> during IOVA allocation.
> 
> Signed-off-by: Xie Yongji 


This needs ack from iommu maintainers. Guys?

> ---
>  drivers/iommu/iova.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c
> index b6cf5f16123b..3941ed6bb99b 100644
> --- a/drivers/iommu/iova.c
> +++ b/drivers/iommu/iova.c
> @@ -521,6 +521,7 @@ alloc_iova_fast(struct iova_domain *iovad, unsigned long 
> size,
>  
>   return new_iova->pfn_lo;
>  }
> +EXPORT_SYMBOL_GPL(alloc_iova_fast);
>  
>  /**
>   * free_iova_fast - free iova pfn range into rcache
> @@ -538,6 +539,7 @@ free_iova_fast(struct iova_domain *iovad, unsigned long 
> pfn, unsigned long size)
>  
>   free_iova(iovad, pfn);
>  }
> +EXPORT_SYMBOL_GPL(free_iova_fast);
>  
>  #define fq_ring_for_each(i, fq) \
>   for ((i) = (fq)->head; (i) != (fq)->tail; (i) = ((i) + 1) % 
> IOVA_FQ_SIZE)
> -- 
> 2.11.0

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH 1/2] dt-bindings: arm-smmu: Add compatible for SM6350 SoC

2021-08-24 Thread Rob Herring
On Fri, 20 Aug 2021 22:29:04 +0200, Konrad Dybcio wrote:
> Add the SoC specific compatible for SM6350 implementing
> arm,mmu-500.
> 
> Signed-off-by: Konrad Dybcio 
> ---
>  Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 

Acked-by: Rob Herring 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH] iommu/io-pgtable: Abstract iommu_iotlb_gather access

2021-08-24 Thread Robin Murphy

On 2021-08-24 14:55, Geert Uytterhoeven wrote:

Hi Robin,

On Fri, Aug 20, 2021 at 3:22 PM Robin Murphy  wrote:

Previously io-pgtable merely passed the iommu_iotlb_gather pointer
through to helpers, but now it has grown its own direct dereference.
This turns out to break the build for !IOMMU_API configs where the
structure only has a dummy definition. It will probably also crash
drivers who don't use the gather mechanism and simply pass in NULL.

Wrap this dereference in a suitable helper which can both be stubbed
out for !IOMMU_API and encapsulate a NULL check otherwise.

Fixes: 7a7c5badf858 ("iommu: Indicate queued flushes via gather data")


Is this the right Fixes tag?


Conceptually, yes - that's where the new member was introduced, so 
that's where its accessor should have been introduced as well, had I not 
managed to overlook the structure being conditionally defined and assume 
it didn't need anything special doing. Of course it's not going to make 
much difference in practice since they are immediately adjacent commits 
anyway, but it felt right to point at where I made the fundamental 
mistake rather than where the symptom appeared :)



The build issue was introduced by:
Fixes: a8e5f04458c4e496 ("iommu/io-pgtable: Remove non-strict quirk")


Reported-by: kernel test robot 
Signed-off-by: Robin Murphy 


Thanks, this fixes the build issues I was seeing.

Tested-by: Geert Uytterhoeven 


Thanks for confirming!

Robin.



Gr{oetje,eeting}s,

 Geert


___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v15 10/12] swiotlb: Add restricted DMA pool initialization

2021-08-24 Thread Guenter Roeck
Hi Claire,

On Thu, Jun 24, 2021 at 11:55:24PM +0800, Claire Chang wrote:
> Add the initialization function to create restricted DMA pools from
> matching reserved-memory nodes.
> 
> Regardless of swiotlb setting, the restricted DMA pool is preferred if
> available.
> 
> The restricted DMA pools provide a basic level of protection against the
> DMA overwriting buffer contents at unexpected times. However, to protect
> against general data leakage and system memory corruption, the system
> needs to provide a way to lock down the memory access, e.g., MPU.
> 
> Signed-off-by: Claire Chang 
> Reviewed-by: Christoph Hellwig 
> Tested-by: Stefano Stabellini 
> Tested-by: Will Deacon 
> ---
>  include/linux/swiotlb.h |  3 +-
>  kernel/dma/Kconfig  | 14 
>  kernel/dma/swiotlb.c| 76 +
>  3 files changed, 92 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
> index 3b9454d1e498..39284ff2a6cd 100644
> --- a/include/linux/swiotlb.h
> +++ b/include/linux/swiotlb.h
> @@ -73,7 +73,8 @@ extern enum swiotlb_force swiotlb_force;
>   *   range check to see if the memory was in fact allocated by this
>   *   API.
>   * @nslabs:  The number of IO TLB blocks (in groups of 64) between @start and
> - *   @end. This is command line adjustable via setup_io_tlb_npages.
> + *   @end. For default swiotlb, this is command line adjustable via
> + *   setup_io_tlb_npages.
>   * @used:The number of used IO TLB block.
>   * @list:The free list describing the number of free entries available
>   *   from each index.
> diff --git a/kernel/dma/Kconfig b/kernel/dma/Kconfig
> index 77b405508743..3e961dc39634 100644
> --- a/kernel/dma/Kconfig
> +++ b/kernel/dma/Kconfig
> @@ -80,6 +80,20 @@ config SWIOTLB
>   bool
>   select NEED_DMA_MAP_STATE
>  
> +config DMA_RESTRICTED_POOL
> + bool "DMA Restricted Pool"
> + depends on OF && OF_RESERVED_MEM
> + select SWIOTLB

This makes SWIOTLB user configurable, which in turn results in

mips64-linux-ld: arch/mips/kernel/setup.o: in function `arch_mem_init':
setup.c:(.init.text+0x19c8): undefined reference to `plat_swiotlb_setup'
make[1]: *** [Makefile:1280: vmlinux] Error 1

when building mips:allmodconfig.

Should this possibly be "depends on SWIOTLB" ?

Thanks,
Guenter
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH] iommu/io-pgtable: Abstract iommu_iotlb_gather access

2021-08-24 Thread Geert Uytterhoeven
Hi Robin,

On Fri, Aug 20, 2021 at 3:22 PM Robin Murphy  wrote:
> Previously io-pgtable merely passed the iommu_iotlb_gather pointer
> through to helpers, but now it has grown its own direct dereference.
> This turns out to break the build for !IOMMU_API configs where the
> structure only has a dummy definition. It will probably also crash
> drivers who don't use the gather mechanism and simply pass in NULL.
>
> Wrap this dereference in a suitable helper which can both be stubbed
> out for !IOMMU_API and encapsulate a NULL check otherwise.
>
> Fixes: 7a7c5badf858 ("iommu: Indicate queued flushes via gather data")

Is this the right Fixes tag?

The build issue was introduced by:
Fixes: a8e5f04458c4e496 ("iommu/io-pgtable: Remove non-strict quirk")

> Reported-by: kernel test robot 
> Signed-off-by: Robin Murphy 

Thanks, this fixes the build issues I was seeing.

Tested-by: Geert Uytterhoeven 

Gr{oetje,eeting}s,

Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v4 15/24] iommu/io-pgtable: Remove non-strict quirk

2021-08-24 Thread Robin Murphy

Hi Geert,

On 2021-08-24 14:25, Geert Uytterhoeven wrote:

Hi Robin,

On Wed, Aug 11, 2021 at 2:24 PM Robin Murphy  wrote:

IO_PGTABLE_QUIRK_NON_STRICT was never a very comfortable fit, since it's
not a quirk of the pagetable format itself. Now that we have a more
appropriate way to convey non-strict unmaps, though, this last of the
non-quirk quirks can also go, and with the flush queue code also now
enforcing its own ordering we can have a lovely cleanup all round.

Signed-off-by: Robin Murphy 


Thanks for your patch, which is now commit a8e5f04458c4e496
("iommu/io-pgtable: Remove non-strict quirk") in iommu/next.


--- a/drivers/iommu/io-pgtable-arm-v7s.c
+++ b/drivers/iommu/io-pgtable-arm-v7s.c
@@ -700,14 +700,7 @@ static size_t __arm_v7s_unmap(struct arm_v7s_io_pgtable 
*data,
 ARM_V7S_BLOCK_SIZE(lvl + 1));
 ptep = iopte_deref(pte[i], lvl, data);
 __arm_v7s_free_table(ptep, lvl + 1, data);
-   } else if (iop->cfg.quirks & 
IO_PGTABLE_QUIRK_NON_STRICT) {
-   /*
-* Order the PTE update against queueing the 
IOVA, to
-* guarantee that a flush callback from a 
different CPU
-* has observed it before the TLBIALL can be 
issued.
-*/
-   smp_wmb();
-   } else {
+   } else if (!gather->queued) {


If CONFIG_IOMMU_API=n:

error: ‘struct iommu_iotlb_gather’ has no member named ‘queued’

This can be reproduced using e.g. shmobile_defconfig with
 CONFIG_IOMMU_SUPPORT=y
 CONFIG_IOMMU_IO_PGTABLE_ARMV7S=y



 io_pgtable_tlb_add_page(iop, gather, iova, 
blk_size);
 }
 iova += blk_size;



--- a/drivers/iommu/io-pgtable-arm.c
+++ b/drivers/iommu/io-pgtable-arm.c
@@ -638,14 +638,7 @@ static size_t __arm_lpae_unmap(struct arm_lpae_io_pgtable 
*data,
 io_pgtable_tlb_flush_walk(iop, iova + i * 
size, size,
   
ARM_LPAE_GRANULE(data));
 __arm_lpae_free_pgtable(data, lvl + 1, 
iopte_deref(pte, data));
-   } else if (iop->cfg.quirks & 
IO_PGTABLE_QUIRK_NON_STRICT) {
-   /*
-* Order the PTE update against queueing the 
IOVA, to
-* guarantee that a flush callback from a 
different CPU
-* has observed it before the TLBIALL can be 
issued.
-*/
-   smp_wmb();
-   } else {
+   } else if (!gather->queued) {


If CONFIG_IOMMU_API=n:

error: ‘struct iommu_iotlb_gather’ has no member named ‘queued’

This can be reproduced using e.g. shmobile_defconfig with
 CONFIG_IOMMU_SUPPORT=y
 CONFIG_IOMMU_IO_PGTABLE_LPAE=y


 io_pgtable_tlb_add_page(iop, gather, iova + i 
* size, size);
 }



Perhaps "select IOMMU_API" should be added (moved from individual
drivers) to both IOMMU_IO_PGTABLE_ARMV7S and IOMMU_IO_PGTABLE_LPAE?
Or iommu_iotlb_gather.queued should not be accessed here, or the
access wrapped into a static inline helper function with a dummy for
the CONFIG_IOMMU_API=n case?


Those (and worse) should be fixed by this patch:

https://lore.kernel.org/linux-iommu/83672ee76f6405c82845a55c148fa836f56fbbc1.1629465282.git.robin.mur...@arm.com/

which apparently hasn't made it to -next yet.

Thanks,
Robin.



Gr{oetje,eeting}s,

 Geert


___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v4 15/24] iommu/io-pgtable: Remove non-strict quirk

2021-08-24 Thread Geert Uytterhoeven
Hi Robin,

On Wed, Aug 11, 2021 at 2:24 PM Robin Murphy  wrote:
> IO_PGTABLE_QUIRK_NON_STRICT was never a very comfortable fit, since it's
> not a quirk of the pagetable format itself. Now that we have a more
> appropriate way to convey non-strict unmaps, though, this last of the
> non-quirk quirks can also go, and with the flush queue code also now
> enforcing its own ordering we can have a lovely cleanup all round.
>
> Signed-off-by: Robin Murphy 

Thanks for your patch, which is now commit a8e5f04458c4e496
("iommu/io-pgtable: Remove non-strict quirk") in iommu/next.

> --- a/drivers/iommu/io-pgtable-arm-v7s.c
> +++ b/drivers/iommu/io-pgtable-arm-v7s.c
> @@ -700,14 +700,7 @@ static size_t __arm_v7s_unmap(struct arm_v7s_io_pgtable 
> *data,
> ARM_V7S_BLOCK_SIZE(lvl + 1));
> ptep = iopte_deref(pte[i], lvl, data);
> __arm_v7s_free_table(ptep, lvl + 1, data);
> -   } else if (iop->cfg.quirks & 
> IO_PGTABLE_QUIRK_NON_STRICT) {
> -   /*
> -* Order the PTE update against queueing the 
> IOVA, to
> -* guarantee that a flush callback from a 
> different CPU
> -* has observed it before the TLBIALL can be 
> issued.
> -*/
> -   smp_wmb();
> -   } else {
> +   } else if (!gather->queued) {

If CONFIG_IOMMU_API=n:

error: ‘struct iommu_iotlb_gather’ has no member named ‘queued’

This can be reproduced using e.g. shmobile_defconfig with
CONFIG_IOMMU_SUPPORT=y
CONFIG_IOMMU_IO_PGTABLE_ARMV7S=y


> io_pgtable_tlb_add_page(iop, gather, iova, 
> blk_size);
> }
> iova += blk_size;

> --- a/drivers/iommu/io-pgtable-arm.c
> +++ b/drivers/iommu/io-pgtable-arm.c
> @@ -638,14 +638,7 @@ static size_t __arm_lpae_unmap(struct 
> arm_lpae_io_pgtable *data,
> io_pgtable_tlb_flush_walk(iop, iova + i * 
> size, size,
>   
> ARM_LPAE_GRANULE(data));
> __arm_lpae_free_pgtable(data, lvl + 1, 
> iopte_deref(pte, data));
> -   } else if (iop->cfg.quirks & 
> IO_PGTABLE_QUIRK_NON_STRICT) {
> -   /*
> -* Order the PTE update against queueing the 
> IOVA, to
> -* guarantee that a flush callback from a 
> different CPU
> -* has observed it before the TLBIALL can be 
> issued.
> -*/
> -   smp_wmb();
> -   } else {
> +   } else if (!gather->queued) {

If CONFIG_IOMMU_API=n:

error: ‘struct iommu_iotlb_gather’ has no member named ‘queued’

This can be reproduced using e.g. shmobile_defconfig with
CONFIG_IOMMU_SUPPORT=y
CONFIG_IOMMU_IO_PGTABLE_LPAE=y

> io_pgtable_tlb_add_page(iop, gather, iova + i 
> * size, size);
> }
>

Perhaps "select IOMMU_API" should be added (moved from individual
drivers) to both IOMMU_IO_PGTABLE_ARMV7S and IOMMU_IO_PGTABLE_LPAE?
Or iommu_iotlb_gather.queued should not be accessed here, or the
access wrapped into a static inline helper function with a dummy for
the CONFIG_IOMMU_API=n case?

Gr{oetje,eeting}s,

Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v11 05/12] vhost-vdpa: Handle the failure of vdpa_reset()

2021-08-24 Thread Stefano Garzarella

On Wed, Aug 18, 2021 at 08:06:35PM +0800, Xie Yongji wrote:

The vdpa_reset() may fail now. This adds check to its return
value and fail the vhost_vdpa_open().

Signed-off-by: Xie Yongji 
---
drivers/vhost/vdpa.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)


Reviewed-by: Stefano Garzarella 

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v11 03/12] vdpa: Fix some coding style issues

2021-08-24 Thread Stefano Garzarella

On Wed, Aug 18, 2021 at 08:06:33PM +0800, Xie Yongji wrote:

Fix some code indent issues and following checkpatch warning:

WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
371: FILE: include/linux/vdpa.h:371:
+static inline void vdpa_get_config(struct vdpa_device *vdev, unsigned offset,

Signed-off-by: Xie Yongji 
---
include/linux/vdpa.h | 34 +-
1 file changed, 17 insertions(+), 17 deletions(-)


Reviewed-by: Stefano Garzarella 



diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
index 954b340f6c2f..8a645f8f4476 100644
--- a/include/linux/vdpa.h
+++ b/include/linux/vdpa.h
@@ -43,17 +43,17 @@ struct vdpa_vq_state_split {
 * @last_used_idx: used index
 */
struct vdpa_vq_state_packed {
-u16last_avail_counter:1;
-u16last_avail_idx:15;
-u16last_used_counter:1;
-u16last_used_idx:15;
+   u16 last_avail_counter:1;
+   u16 last_avail_idx:15;
+   u16 last_used_counter:1;
+   u16 last_used_idx:15;
};

struct vdpa_vq_state {
- union {
-  struct vdpa_vq_state_split split;
-  struct vdpa_vq_state_packed packed;
- };
+   union {
+   struct vdpa_vq_state_split split;
+   struct vdpa_vq_state_packed packed;
+   };
};

struct vdpa_mgmt_dev;
@@ -131,7 +131,7 @@ struct vdpa_iova_range {
 *  @vdev: vdpa device
 *  @idx: virtqueue index
 *  @state: pointer to returned state 
(last_avail_idx)
- * @get_vq_notification:   Get the notification area for a virtqueue
+ * @get_vq_notification:   Get the notification area for a virtqueue
 *  @vdev: vdpa device
 *  @idx: virtqueue index
 *  Returns the notifcation area
@@ -353,25 +353,25 @@ static inline struct device *vdpa_get_dma_dev(struct 
vdpa_device *vdev)

static inline void vdpa_reset(struct vdpa_device *vdev)
{
-const struct vdpa_config_ops *ops = vdev->config;
+   const struct vdpa_config_ops *ops = vdev->config;

vdev->features_valid = false;
-ops->set_status(vdev, 0);
+   ops->set_status(vdev, 0);
}

static inline int vdpa_set_features(struct vdpa_device *vdev, u64 features)
{
-const struct vdpa_config_ops *ops = vdev->config;
+   const struct vdpa_config_ops *ops = vdev->config;

vdev->features_valid = true;
-return ops->set_features(vdev, features);
+   return ops->set_features(vdev, features);
}

-
-static inline void vdpa_get_config(struct vdpa_device *vdev, unsigned offset,
-  void *buf, unsigned int len)
+static inline void vdpa_get_config(struct vdpa_device *vdev,
+  unsigned int offset, void *buf,
+  unsigned int len)
{
-const struct vdpa_config_ops *ops = vdev->config;
+   const struct vdpa_config_ops *ops = vdev->config;

/*
 * Config accesses aren't supposed to trigger before features are set.
--
2.11.0



___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


min_align_mask Re: [PATCH V3 13/13] HV/Storvsc: Add Isolation VM support for storvsc driver

2021-08-24 Thread h...@lst.de
On Fri, Aug 20, 2021 at 03:40:08PM +, Michael Kelley wrote:
> I see that the swiotlb code gets and uses the min_align_mask field.  But
> the NVME driver is the only driver that ever sets it, so the value is zero
> in all other cases.  Does swiotlb just use PAGE_SIZE in that that case?  I
> couldn't tell from a quick glance at the swiotlb code.

The encoding isn't all that common.  I only know it for the RDMA memory
registration format, and RDMA in general does not interact very well
with swiotlb (although the in-kernel drivers should work fine, it is
userspace RDMA that is the problem).  It seems recently a new driver
using the format (mpi3mr) also showed up.  All these should probably set
the min_align_mask.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH V3 13/13] HV/Storvsc: Add Isolation VM support for storvsc driver

2021-08-24 Thread h...@lst.de
On Sat, Aug 21, 2021 at 02:04:11AM +0800, Tianyu Lan wrote:
> After dma_map_sg(), we still need to go through scatter list again to 
> populate payload->rrange.pfn_array. We may just go through the scatter list 
> just once if dma_map_sg() accepts a callback and run it during go
> through scatter list.

Iterating a cache hot array is way faster than doing lots of indirect
calls.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH V3 05/13] HV: Add Write/Read MSR registers via ghcb page

2021-08-24 Thread Christoph Hellwig
This patch fails to compile when CONFIG_AMD_MEM_ENCRYPT is not enabled,
in which case there is sev_es_ghcb_hv_call_simple is not defined.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v2 16/29] iommu/mediatek: Adjust device link when it is sub-common

2021-08-24 Thread Hsin-Yi Wang
On Fri, Aug 13, 2021 at 3:03 PM Yong Wu  wrote:
>
> For MM IOMMU, We always add device link between smi-common and IOMMU HW.
> In mt8195, we add smi-sub-common. Thus, if the node is sub-common, we still
> need find again to get smi-common, then do device link.
>
> Signed-off-by: Yong Wu 
> ---
>  drivers/iommu/mtk_iommu.c | 11 +++
>  1 file changed, 11 insertions(+)
>
> diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
> index a4479916ad33..a72241724adb 100644
> --- a/drivers/iommu/mtk_iommu.c
> +++ b/drivers/iommu/mtk_iommu.c
> @@ -845,6 +845,17 @@ static int mtk_iommu_mm_dts_parse(struct device *dev,
> if (!smicomm_node)
> return -EINVAL;
>
> +   /* Find smi-common again if this is smi-sub-common */
> +   if (of_property_read_bool(smicomm_node, "mediatek,smi_sub_common")) {
> +   of_node_put(smicomm_node); /* put the sub common */
> +
> +   smicomm_node = of_parse_phandle(smicomm_node, "mediatek,smi", 
> 0);

This only checks 1 level here, and does not check if the mediatek,smi
of a sub-common node is not another sub-common node.
So maybe add a check that the updated node here doesn't have
mediatek,smi_sub_common property.

> +   if (!smicomm_node) {
> +   dev_err(dev, "sub-comm has no common.\n");
> +   return -EINVAL;
> +   }
> +   }
> +
> plarbdev = of_find_device_by_node(smicomm_node);
> of_node_put(smicomm_node);
> data->smicomm_dev = >dev;
> --
> 2.18.0
> ___
> Linux-mediatek mailing list
> linux-media...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v2 03/12] x86/sev: Add an x86 version of prot_guest_has()

2021-08-24 Thread Christoph Hellwig
On Thu, Aug 19, 2021 at 01:33:09PM -0500, Tom Lendacky wrote:
> I did it as inline originally because the presence of the function will be
> decided based on the ARCH_HAS_PROTECTED_GUEST config. For now, that is
> only selected by the AMD memory encryption support, so if I went out of
> line I could put in mem_encrypt.c. But with TDX wanting to also use it, it
> would have to be in an always built file with some #ifdefs or in its own
> file that is conditionally built based on the ARCH_HAS_PROTECTED_GUEST
> setting (they've already tried building with ARCH_HAS_PROTECTED_GUEST=y
> and AMD_MEM_ENCRYPT not set).
> 
> To take it out of line, I'm leaning towards the latter, creating a new
> file that is built based on the ARCH_HAS_PROTECTED_GUEST setting.

Yes.  In general everytime architectures have to provide the prototype
and not just the implementation of something we end up with a giant mess
sooner or later.  In a few cases that is still warranted due to
performance concerns, but i don't think that is the case here.

> 
> > 
> >> +/* 0x800 - 0x8ff reserved for AMD */
> >> +#define PATTR_SME 0x800
> >> +#define PATTR_SEV 0x801
> >> +#define PATTR_SEV_ES  0x802
> > 
> > Why do we need reservations for a purely in-kernel namespace?
> > 
> > And why are you overoading a brand new generic API with weird details
> > of a specific implementation like this?
> 
> There was some talk about this on the mailing list where TDX and SEV may
> need to be differentiated, so we wanted to reserve a range of values per
> technology. I guess I can remove them until they are actually needed.

In that case add a flag for the differing behavior.  And only add them
when actually needed.  And either way there is absolutely no need to
reserve ranges.

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v2 11/29] iommu/mediatek: Always pm_runtime_get while tlb flush

2021-08-24 Thread Hsin-Yi Wang
On Fri, Aug 13, 2021 at 2:57 PM Yong Wu  wrote:
>
> Prepare for 2 HWs that sharing pgtable in different power-domains.
>
> The previous SoC don't have PM. Only mt8192 has power-domain,
> and it is display's power-domain which nearly always is enabled.
>
> When there are 2 M4U HWs, it may has problem.
> In this function, we get the pm_status via the m4u dev, but it don't
> reflect the real power-domain status of the HW since there may be other
> HW also use that power-domain.
>
> Currently we could not get the real power-domain status, thus always
> pm_runtime_get here.
>
> Prepare for mt8195, thus, no need fix tags here.
>
> This patch may drop the performance, we expect the user could
> pm_runtime_get_sync before dma_alloc_attrs which need tlb ops.

Can you check if there are existing users that need to add this change?


>
> Signed-off-by: Yong Wu 
> ---

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu