Re: [PATCH 1/1] iommu/arm-smmu-v3: replace writel with writel_relaxed in queue_inc_prod

[Leizhen (ThunderTown)]

On 2017/6/20 19:35, Robin Murphy wrote:
> On 20/06/17 12:04, Zhen Lei wrote:
>> This function is protected by spinlock, and the latter will do memory
>> barrier implicitly. So that we can safely use writel_relaxed. In fact, the
>> dmb operation will lengthen the time protected by lock, which indirectly
>> increase the locking confliction in the stress scene.
> 
> If you remove the DSB between writing the commands (to Normal memory)
> and writing the pointer (to Device memory), how can you guarantee that
> the complete command is visible to the SMMU and it isn't going to try to
> consume stale memory contents? The spinlock is irrelevant since it's
> taken *before* the command is written.
OK, I see, thanks. Let's me see if there are any other methods. And I think
that this may should be done well by hardware.

> 
> Robin.
> 
>> Signed-off-by: Zhen Lei 
>> ---
>>  drivers/iommu/arm-smmu-v3.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
>> index 380969a..d2fbee3 100644
>> --- a/drivers/iommu/arm-smmu-v3.c
>> +++ b/drivers/iommu/arm-smmu-v3.c
>> @@ -728,7 +728,7 @@ static void queue_inc_prod(struct arm_smmu_queue *q)
>>  u32 prod = (Q_WRP(q, q->prod) | Q_IDX(q, q->prod)) + 1;
>>
>>  q->prod = Q_OVF(q, q->prod) | Q_WRP(q, prod) | Q_IDX(q, prod);
>> -writel(q->prod, q->prod_reg);
>> +writel_relaxed(q->prod, q->prod_reg);
>>  }
>>
>>  /*
>> --
>> 2.5.0
>>
>>
> 
> 
> .
> 

-- 
Thanks!
BestRegards

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

Re: [PATCH v7 07/36] x86/mm: Don't use phys_to_virt in ioremap() if SME is active

[Thomas Gleixner]

On Fri, 16 Jun 2017, Tom Lendacky wrote:

> Currently there is a check if the address being mapped is in the ISA
> range (is_ISA_range()), and if it is then phys_to_virt() is used to
> perform the mapping.  When SME is active, however, this will result
> in the mapping having the encryption bit set when it is expected that
> an ioremap() should not have the encryption bit set. So only use the
> phys_to_virt() function if SME is not active

This does not make sense to me. What the heck has phys_to_virt() to do with
the encryption bit. Especially why would the encryption bit be set on that
mapping in the first place?

I'm probably missing something, but this want's some coherent explanation
understandable by mere mortals both in the changelog and the code comment.

Thanks,

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

Re: [PATCH v7 06/36] x86/mm: Add Secure Memory Encryption (SME) support

[Thomas Gleixner]

On Fri, 16 Jun 2017, Tom Lendacky wrote:
>  
> +config ARCH_HAS_MEM_ENCRYPT
> + def_bool y
> + depends on X86

That one is silly. The config switch is in the x86 KConfig file, so X86 is
on. If you intended to move this to some generic place outside of
x86/Kconfig then this should be

config ARCH_HAS_MEM_ENCRYPT
bool

and x86/Kconfig should have

select ARCH_HAS_MEM_ENCRYPT

and that should be selected by AMD_MEM_ENCRYPT

> +config AMD_MEM_ENCRYPT
> + bool "AMD Secure Memory Encryption (SME) support"
> + depends on X86_64 && CPU_SUP_AMD
> + ---help---
> +   Say yes to enable support for the encryption of system memory.
> +   This requires an AMD processor that supports Secure Memory
> +   Encryption (SME).

Thanks,

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

Re: [PATCH v7 19/36] x86/mm: Add support to access boot related data in the clear

[Borislav Petkov]

On Fri, Jun 16, 2017 at 01:53:26PM -0500, Tom Lendacky wrote:
> Boot data (such as EFI related data) is not encrypted when the system is
> booted because UEFI/BIOS does not run with SME active. In order to access
> this data properly it needs to be mapped decrypted.
> 
> Update early_memremap() to provide an arch specific routine to modify the
> pagetable protection attributes before they are applied to the new
> mapping. This is used to remove the encryption mask for boot related data.
> 
> Update memremap() to provide an arch specific routine to determine if RAM
> remapping is allowed.  RAM remapping will cause an encrypted mapping to be
> generated. By preventing RAM remapping, ioremap_cache() will be used
> instead, which will provide a decrypted mapping of the boot related data.
> 
> Signed-off-by: Tom Lendacky 
> ---
>  arch/x86/include/asm/io.h |5 +
>  arch/x86/mm/ioremap.c |  179 
> +
>  include/linux/io.h|2 +
>  kernel/memremap.c |   20 -
>  mm/early_ioremap.c|   18 -
>  5 files changed, 217 insertions(+), 7 deletions(-)

Reviewed-by: Borislav Petkov 

-- 
Regards/Gruss,
Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v8 1/3] ACPI/IORT: Fixup SMMUv3 resource size for Cavium ThunderX2 SMMUv3 model

[Lorenzo Pieralisi]

On Tue, Jun 20, 2017 at 07:47:37PM +0530, Geetha sowjanya wrote:
> From: Linu Cherian 
> 
> Cavium ThunderX2 implementation doesn't support second page in SMMU
> register space. Hence, resource size is set as 64k for this model.
> 
> Signed-off-by: Linu Cherian 
> Signed-off-by: Geetha Sowjanya 
> ---
>  drivers/acpi/arm64/iort.c |   15 ++-
>  1 files changed, 14 insertions(+), 1 deletions(-)

Acked-by: Lorenzo Pieralisi 

> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> index c5fecf9..c166f3e 100644
> --- a/drivers/acpi/arm64/iort.c
> +++ b/drivers/acpi/arm64/iort.c
> @@ -828,6 +828,18 @@ static int __init arm_smmu_v3_count_resources(struct 
> acpi_iort_node *node)
>   return num_res;
>  }
>  
> +static unsigned long arm_smmu_v3_resource_size(struct acpi_iort_smmu_v3 
> *smmu)
> +{
> + /*
> +  * Override the size, for Cavium ThunderX2 implementation
> +  * which doesn't support the page 1 SMMU register space.
> +  */
> + if (smmu->model == ACPI_IORT_SMMU_V3_CAVIUM_CN99XX)
> + return SZ_64K;
> +
> + return SZ_128K;
> +}
> +
>  static void __init arm_smmu_v3_init_resources(struct resource *res,
> struct acpi_iort_node *node)
>  {
> @@ -838,7 +850,8 @@ static void __init arm_smmu_v3_init_resources(struct 
> resource *res,
>   smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
>  
>   res[num_res].start = smmu->base_address;
> - res[num_res].end = smmu->base_address + SZ_128K - 1;
> + res[num_res].end = smmu->base_address +
> + arm_smmu_v3_resource_size(smmu) - 1;
>   res[num_res].flags = IORESOURCE_MEM;
>  
>   num_res++;
> -- 
> 1.7.1
> 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v8 2/3] iommu/arm-smmu-v3: Add workaround for Cavium ThunderX2 erratum #74

[Will Deacon]

On Tue, Jun 20, 2017 at 07:47:38PM +0530, Geetha sowjanya wrote:
> From: Linu Cherian 
> 
> Cavium ThunderX2 SMMU implementation doesn't support page 1 register space
> and PAGE0_REGS_ONLY option is enabled as an errata workaround.
> This option when turned on, replaces all page 1 offsets used for
> EVTQ_PROD/CONS, PRIQ_PROD/CONS register access with page 0 offsets.
> 
> SMMU resource size checks are now based on SMMU option PAGE0_REGS_ONLY,
> since resource size can be either 64k/128k.
> For this, arm_smmu_device_dt_probe/acpi_probe has been moved before
> platform_get_resource call, so that SMMU options are set beforehand.
> 
> Signed-off-by: Linu Cherian 
> Signed-off-by: Geetha Sowjanya 
> ---
>  Documentation/arm64/silicon-errata.txt |1 +
>  .../devicetree/bindings/iommu/arm,smmu-v3.txt  |6 ++
>  drivers/iommu/arm-smmu-v3.c|   68 ++-
>  3 files changed, 57 insertions(+), 18 deletions(-)
> 
> diff --git a/Documentation/arm64/silicon-errata.txt 
> b/Documentation/arm64/silicon-errata.txt
> index 10f2ddd..4693a32 100644
> --- a/Documentation/arm64/silicon-errata.txt
> +++ b/Documentation/arm64/silicon-errata.txt
> @@ -62,6 +62,7 @@ stable kernels.
>  | Cavium | ThunderX GICv3  | #23154  | CAVIUM_ERRATUM_23154  
>   |
>  | Cavium | ThunderX Core   | #27456  | CAVIUM_ERRATUM_27456  
>   |
>  | Cavium | ThunderX SMMUv2 | #27704  | N/A   
>   |
> +| Cavium | ThunderX2 SMMUv3| #74 | N/A   
>   |
>  || | |   
>   |
>  | Freescale/NXP  | LS2080A/LS1043A | A-008585| FSL_ERRATUM_A008585   
>   |
>  || | |   
>   |
> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt 
> b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
> index be57550..6ecc48c 100644
> --- a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
> @@ -49,6 +49,12 @@ the PCIe specification.
>  - hisilicon,broken-prefetch-cmd
>  : Avoid sending CMD_PREFETCH_* commands to the SMMU.
>  
> +- cavium,cn9900-broken-page1-regspace
> +: Replaces all page 1 offsets used for EVTQ_PROD/CONS,
> +   PRIQ_PROD/CONS register access with page 0 offsets.
> +   Set for Caviun ThunderX2 silicon that doesn't support

s/Caviun/Cavium/

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

Re: [PATCH v8 1/3] ACPI/IORT: Fixup SMMUv3 resource size for Cavium ThunderX2 SMMUv3 model

[Will Deacon]

On Tue, Jun 20, 2017 at 07:47:37PM +0530, Geetha sowjanya wrote:
> From: Linu Cherian 
> 
> Cavium ThunderX2 implementation doesn't support second page in SMMU
> register space. Hence, resource size is set as 64k for this model.
> 
> Signed-off-by: Linu Cherian 
> Signed-off-by: Geetha Sowjanya 
> ---
>  drivers/acpi/arm64/iort.c |   15 ++-
>  1 files changed, 14 insertions(+), 1 deletions(-)

Looks fine to me, but I need Lorenzo's ack on this.

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

Re: [PATCH v8 3/3] iommu/arm-smmu-v3: Add workaround for Cavium ThunderX2 erratum #126

[Will Deacon]

On Tue, Jun 20, 2017 at 07:47:39PM +0530, Geetha sowjanya wrote:
> From: Geetha Sowjanya 
> 
> Cavium ThunderX2 SMMU doesn't support MSI and also doesn't have unique irq
> lines for gerror, eventq and cmdq-sync.
> 
> SHARED_IRQ option is set as a errata workaround, which allows to share the irq
> line by register single irq handler for all the interrupts.
> 
> Signed-off-by: Geetha sowjanya 
> ---
>  .../devicetree/bindings/iommu/arm,smmu-v3.txt  |5 ++
>  drivers/iommu/arm-smmu-v3.c|   73 
> 
>  2 files changed, 64 insertions(+), 14 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt 
> b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
> index 6ecc48c..44b40e0 100644
> --- a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
> @@ -55,6 +55,11 @@ the PCIe specification.
> Set for Caviun ThunderX2 silicon that doesn't support
> SMMU page1 register space.
>  
> +- cavium,cn9900-broken-unique-irqline
> +: Use single irq line for all the SMMUv3 interrupts.
> +   Set for Caviun ThunderX2 silicon that doesn't support
> +   MSI and also doesn't have unique irq lines for gerror,
> +   eventq and cmdq-sync.

I think we're better off just supporting a new (optional) named interrupt
as "combined", and then allowing that to be used instead of the others.

>  ** Example
>  
>  smmu@2b40 {
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index 2dea4a9..6c0c632 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -598,6 +598,7 @@ struct arm_smmu_device {
>  
>  #define ARM_SMMU_OPT_SKIP_PREFETCH   (1 << 0)
>  #define ARM_SMMU_OPT_PAGE0_REGS_ONLY (1 << 1)
> +#define ARM_SMMU_OPT_SHARED_IRQ  (1 << 2)

Please call this COMBINED instead of SHARED (similarly elsewhere). That
said, not sure we need this.

>   u32 options;
>  
>   struct arm_smmu_cmdqcmdq;
> @@ -665,6 +666,7 @@ struct arm_smmu_option_prop {
>  static struct arm_smmu_option_prop arm_smmu_options[] = {
>   { ARM_SMMU_OPT_SKIP_PREFETCH, "hisilicon,broken-prefetch-cmd" },
>   { ARM_SMMU_OPT_PAGE0_REGS_ONLY, "cavium,cn9900-broken-page1-regspace"},
> + { ARM_SMMU_OPT_SHARED_IRQ, "cavium,cn9900-broken-unique-irqline"},
>   { 0, NULL},
>  };
>  
> @@ -1313,6 +1315,21 @@ static irqreturn_t arm_smmu_gerror_handler(int irq, 
> void *dev)
>   writel(gerror, smmu->base + ARM_SMMU_GERRORN);
>   return IRQ_HANDLED;
>  }
> +/* Shared irq handler*/
> +static irqreturn_t arm_smmu_shared_irq_thread(int irq, void *dev)
> +{
> + struct arm_smmu_device *smmu = dev;
> + irqreturn_t ret;
> +
> + ret = arm_smmu_gerror_handler(irq, dev);
> + if (ret == IRQ_NONE) {
> + arm_smmu_evtq_thread(irq, dev);
> + arm_smmu_cmdq_sync_handler(irq, dev);
> + if (smmu->features & ARM_SMMU_FEAT_PRI)
> + arm_smmu_priq_thread(irq, dev);
> + }
> + return IRQ_HANDLED;
> +}

This isn't quite right, because you're now running low-level handlers (like
the gerror handler) in threaded context. You're better off registering a
low-level handler too (see below) which can kick gerror and cmdq_sync
before unconditionally returning IRQ_WAKE_THREAD.

>  
>  /* IO_PGTABLE API */
>  static void __arm_smmu_tlb_sync(struct arm_smmu_device *smmu)
> @@ -2230,18 +2247,9 @@ static void arm_smmu_setup_msis(struct arm_smmu_device 
> *smmu)
>   devm_add_action(dev, arm_smmu_free_msis, dev);
>  }
>  
> -static int arm_smmu_setup_irqs(struct arm_smmu_device *smmu)
> +static void arm_smmu_setup_unique_irqs(struct arm_smmu_device *smmu)
>  {
> - int ret, irq;
> - u32 irqen_flags = IRQ_CTRL_EVTQ_IRQEN | IRQ_CTRL_GERROR_IRQEN;
> -
> - /* Disable IRQs first */
> - ret = arm_smmu_write_reg_sync(smmu, 0, ARM_SMMU_IRQ_CTRL,
> -   ARM_SMMU_IRQ_CTRLACK);
> - if (ret) {
> - dev_err(smmu->dev, "failed to disable irqs\n");
> - return ret;
> - }
> + int irq, ret;
>  
>   arm_smmu_setup_msis(smmu);
>  
> @@ -2284,10 +2292,46 @@ static int arm_smmu_setup_irqs(struct arm_smmu_device 
> *smmu)
>   if (ret < 0)
>   dev_warn(smmu->dev,
>"failed to enable priq irq\n");
> - else
> - irqen_flags |= IRQ_CTRL_PRIQ_IRQEN;
>   }
>   }
> +}
> +
> +static void arm_smmu_setup_shared_irqs(struct arm_smmu_device *smmu)
> +{
> + int ret, irq;
> +
> + /* Single irq is used for all queues, request single interrupt lines */
> + irq = smmu->evtq.q.irq;
> + if (irq) 

Re: [PATCH v2 2/2] iommu/arm-smmu-v3:Enable ACPI based HiSilicon erratum 161010801

[Lorenzo Pieralisi]

On Tue, Jun 20, 2017 at 03:39:30PM +, Shameerali Kolothum Thodi wrote:
> 
> 
> > -Original Message-
> > From: Robin Murphy [mailto:robin.mur...@arm.com]
> > Sent: Tuesday, June 20, 2017 4:16 PM
> > To: Shameerali Kolothum Thodi; Lorenzo Pieralisi
> > Cc: marc.zyng...@arm.com; sudeep.ho...@arm.com; will.dea...@arm.com;
> > hanjun@linaro.org; Gabriele Paoloni; John Garry; iommu@lists.linux-
> > foundation.org; linux-arm-ker...@lists.infradead.org; linux-
> > a...@vger.kernel.org; de...@acpica.org; Linuxarm; Wangzhou (B);
> > Guohanjun (Hanjun Guo)
> > Subject: Re: [PATCH v2 2/2] iommu/arm-smmu-v3:Enable ACPI based
> > HiSilicon erratum 161010801
> > 
> > On 20/06/17 15:07, Shameerali Kolothum Thodi wrote:
> > >
> > >
> > >> -Original Message-
> > >> From: Lorenzo Pieralisi [mailto:lorenzo.pieral...@arm.com]
> > >> Sent: Tuesday, June 20, 2017 11:29 AM
> > >> To: Shameerali Kolothum Thodi
> > >> Cc: marc.zyng...@arm.com; sudeep.ho...@arm.com;
> > will.dea...@arm.com;
> > >> robin.mur...@arm.com; hanjun@linaro.org; Gabriele Paoloni; John
> > >> Garry; iommu@lists.linux-foundation.org; linux-arm-
> > >> ker...@lists.infradead.org; linux-a...@vger.kernel.org;
> > de...@acpica.org;
> > >> Linuxarm; Wangzhou (B); Guohanjun (Hanjun Guo)
> > >> Subject: Re: [PATCH v2 2/2] iommu/arm-smmu-v3:Enable ACPI based
> > >> HiSilicon erratum 161010801
> > >>
> > >> Hi Shameer,
> > >>
> > >> On Mon, Jun 19, 2017 at 04:45:00PM +0100, shameer wrote:
> > >>> The HiSilicon erratum 161010801 describes the limitation of HiSilicon
> > >>> platforms Hip06/Hip07 to support the SMMU mappings for MSI
> > >> transactions.
> > >>>
> > >>> On these platforms GICv3 ITS translator is presented with the deviceID
> > >>> by extending the MSI payload data to 64 bits to include the deviceID.
> > >>> Hence, the PCIe controller on this platforms has to differentiate the
> > >>> MSI payload against other DMA payload and has to modify the MSI
> > >> payload.
> > >>> This basically makes it difficult for this platforms to have a SMMU
> > >>> translation for MSI.
> > >>>
> > >>> This patch implements a ACPI table based quirk to reserve the hw msi
> > >>> regions in the smmu-v3 driver which means these address regions will
> > >>> not be translated and will be excluded from iova allocations.
> > >>>
> > >>> Signed-off-by: shameer 
> > >>> ---
> > >>>  drivers/iommu/arm-smmu-v3.c | 29 -
> > >>>  1 file changed, 24 insertions(+), 5 deletions(-)
> > >>>
> > >>> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-
> > smmu-
> > >> v3.c
> > >>> index abe4b88..f03c63b 100644
> > >>> --- a/drivers/iommu/arm-smmu-v3.c
> > >>> +++ b/drivers/iommu/arm-smmu-v3.c
> > >>> @@ -597,6 +597,7 @@ struct arm_smmu_device {
> > >>> u32 features;
> > >>>
> > >>>  #define ARM_SMMU_OPT_SKIP_PREFETCH (1 << 0)
> > >>> +#define ARM_SMMU_OPT_RESV_HW_MSI   (1 << 1)
> > >>> u32 options;
> > >>>
> > >>> struct arm_smmu_cmdqcmdq;
> > >>> @@ -1904,14 +1905,31 @@ static void
> > arm_smmu_get_resv_regions(struct
> > >> device *dev,
> > >>>   struct list_head *head)
> > >>>  {
> > >>> struct iommu_resv_region *region;
> > >>> +   struct arm_smmu_device *smmu;
> > >>> +   struct iommu_fwspec *fwspec = dev->iommu_fwspec;
> > >>> int prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO;
> > >>>
> > >>> -   region = iommu_alloc_resv_region(MSI_IOVA_BASE,
> > >> MSI_IOVA_LENGTH,
> > >>> -prot, IOMMU_RESV_SW_MSI);
> > >>> -   if (!region)
> > >>> -   return;
> > >>> +   smmu = arm_smmu_get_by_fwnode(fwspec->iommu_fwnode);
> > >>> +
> > >>> +   if (smmu && (smmu->options & ARM_SMMU_OPT_RESV_HW_MSI)
> > >> &&
> > >>> + dev_is_pci(dev)) {
> > >>
> > >> IORT changes are fine to me, I am still no big fan of this supposedly
> > >> generic option that is _really_ platform specific (in particular as I
> > >> said before the quirk depends on the PCI host bridge but in this
> > >> patchset I see no such dependency. In short - the quirk is hooked off
> > >> the SMMUv3 model which implicitly implies a PCI host bridge
> > >> configuration IIUC). It is Will and Robin decision though, I am not sure
> > >> you can make it any tidier (given that on ACPI you may not even have
> > >> a PCI host bridge specific _HID to base your check above on).
> > >
> > > Thanks Lorenzo. I understand your point. As far as our platform is
> > > concerned, I think It is ok to remove the dev_is_pci() check and make
> > > it generic, if that helps.  I don't think it will harm us other than 
> > > couple of
> > >  "HW MSI region resv failed: " logs  for our platform devices.
> > 
> > I think the answer there is that iort_iommu_its_get_resv_regions()
> > really should distinguish between "this 

Re: [PATCH v7 11/36] x86/mm: Add SME support for read_cr3_pa()

[Tom Lendacky]

On 6/20/2017 11:17 AM, Andy Lutomirski wrote:

On Fri, Jun 16, 2017 at 11:51 AM, Tom Lendacky  wrote:

The cr3 register entry can contain the SME encryption mask that indicates
the PGD is encrypted.  The encryption mask should not be used when
creating a virtual address from the cr3 register, so remove the SME
encryption mask in the read_cr3_pa() function.

During early boot SME will need to use a native version of read_cr3_pa(),
so create native_read_cr3_pa().

Signed-off-by: Tom Lendacky 
---
  arch/x86/include/asm/processor-flags.h |3 ++-
  arch/x86/include/asm/processor.h   |5 +
  2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/processor-flags.h 
b/arch/x86/include/asm/processor-flags.h
index 79aa2f9..cb6999c 100644
--- a/arch/x86/include/asm/processor-flags.h
+++ b/arch/x86/include/asm/processor-flags.h
@@ -2,6 +2,7 @@
  #define _ASM_X86_PROCESSOR_FLAGS_H

  #include 
+#include 

  #ifdef CONFIG_VM86
  #define X86_VM_MASKX86_EFLAGS_VM
@@ -33,7 +34,7 @@
   */
  #ifdef CONFIG_X86_64
  /* Mask off the address space ID bits. */
-#define CR3_ADDR_MASK 0x7000ull
+#define CR3_ADDR_MASK __sme_clr(0x7000ull)


Can you update the comment one line above, too?


Yup, will do.

Thanks,
Tom




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

Re: [PATCH v7 11/36] x86/mm: Add SME support for read_cr3_pa()

[Andy Lutomirski]

On Fri, Jun 16, 2017 at 11:51 AM, Tom Lendacky  wrote:
> The cr3 register entry can contain the SME encryption mask that indicates
> the PGD is encrypted.  The encryption mask should not be used when
> creating a virtual address from the cr3 register, so remove the SME
> encryption mask in the read_cr3_pa() function.
>
> During early boot SME will need to use a native version of read_cr3_pa(),
> so create native_read_cr3_pa().
>
> Signed-off-by: Tom Lendacky 
> ---
>  arch/x86/include/asm/processor-flags.h |3 ++-
>  arch/x86/include/asm/processor.h   |5 +
>  2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/include/asm/processor-flags.h 
> b/arch/x86/include/asm/processor-flags.h
> index 79aa2f9..cb6999c 100644
> --- a/arch/x86/include/asm/processor-flags.h
> +++ b/arch/x86/include/asm/processor-flags.h
> @@ -2,6 +2,7 @@
>  #define _ASM_X86_PROCESSOR_FLAGS_H
>
>  #include 
> +#include 
>
>  #ifdef CONFIG_VM86
>  #define X86_VM_MASKX86_EFLAGS_VM
> @@ -33,7 +34,7 @@
>   */
>  #ifdef CONFIG_X86_64
>  /* Mask off the address space ID bits. */
> -#define CR3_ADDR_MASK 0x7000ull
> +#define CR3_ADDR_MASK __sme_clr(0x7000ull)

Can you update the comment one line above, too?
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 2/2] iommu/arm-smmu-v3:Enable ACPI based HiSilicon erratum 161010801

[Robin Murphy]

On 20/06/17 16:51, Lorenzo Pieralisi wrote:
> On Tue, Jun 20, 2017 at 04:16:18PM +0100, Robin Murphy wrote:
>> On 20/06/17 15:07, Shameerali Kolothum Thodi wrote:
>>>
>>>
 -Original Message-
 From: Lorenzo Pieralisi [mailto:lorenzo.pieral...@arm.com]
 Sent: Tuesday, June 20, 2017 11:29 AM
 To: Shameerali Kolothum Thodi
 Cc: marc.zyng...@arm.com; sudeep.ho...@arm.com; will.dea...@arm.com;
 robin.mur...@arm.com; hanjun@linaro.org; Gabriele Paoloni; John
 Garry; iommu@lists.linux-foundation.org; linux-arm-
 ker...@lists.infradead.org; linux-a...@vger.kernel.org; de...@acpica.org;
 Linuxarm; Wangzhou (B); Guohanjun (Hanjun Guo)
 Subject: Re: [PATCH v2 2/2] iommu/arm-smmu-v3:Enable ACPI based
 HiSilicon erratum 161010801

 Hi Shameer,

 On Mon, Jun 19, 2017 at 04:45:00PM +0100, shameer wrote:
> The HiSilicon erratum 161010801 describes the limitation of HiSilicon
> platforms Hip06/Hip07 to support the SMMU mappings for MSI
 transactions.
>
> On these platforms GICv3 ITS translator is presented with the deviceID
> by extending the MSI payload data to 64 bits to include the deviceID.
> Hence, the PCIe controller on this platforms has to differentiate the
> MSI payload against other DMA payload and has to modify the MSI
 payload.
> This basically makes it difficult for this platforms to have a SMMU
> translation for MSI.
>
> This patch implements a ACPI table based quirk to reserve the hw msi
> regions in the smmu-v3 driver which means these address regions will
> not be translated and will be excluded from iova allocations.
>
> Signed-off-by: shameer 
> ---
>  drivers/iommu/arm-smmu-v3.c | 29 -
>  1 file changed, 24 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-
 v3.c
> index abe4b88..f03c63b 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -597,6 +597,7 @@ struct arm_smmu_device {
>   u32 features;
>
>  #define ARM_SMMU_OPT_SKIP_PREFETCH   (1 << 0)
> +#define ARM_SMMU_OPT_RESV_HW_MSI (1 << 1)
>   u32 options;
>
>   struct arm_smmu_cmdqcmdq;
> @@ -1904,14 +1905,31 @@ static void arm_smmu_get_resv_regions(struct
 device *dev,
> struct list_head *head)
>  {
>   struct iommu_resv_region *region;
> + struct arm_smmu_device *smmu;
> + struct iommu_fwspec *fwspec = dev->iommu_fwspec;
>   int prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO;
>
> - region = iommu_alloc_resv_region(MSI_IOVA_BASE,
 MSI_IOVA_LENGTH,
> -  prot, IOMMU_RESV_SW_MSI);
> - if (!region)
> - return;
> + smmu = arm_smmu_get_by_fwnode(fwspec->iommu_fwnode);
> +
> + if (smmu && (smmu->options & ARM_SMMU_OPT_RESV_HW_MSI)
 &&
> +   dev_is_pci(dev)) {

 IORT changes are fine to me, I am still no big fan of this supposedly
 generic option that is _really_ platform specific (in particular as I
 said before the quirk depends on the PCI host bridge but in this
 patchset I see no such dependency. In short - the quirk is hooked off
 the SMMUv3 model which implicitly implies a PCI host bridge
 configuration IIUC). It is Will and Robin decision though, I am not sure
 you can make it any tidier (given that on ACPI you may not even have
 a PCI host bridge specific _HID to base your check above on).
>>>
>>> Thanks Lorenzo. I understand your point. As far as our platform is 
>>> concerned, I think It is ok to remove the dev_is_pci() check and make
>>> it generic, if that helps.  I don't think it will harm us other than couple 
>>> of
>>>  "HW MSI region resv failed: " logs  for our platform devices.
>>
>> I think the answer there is that iort_iommu_its_get_resv_regions()
>> really should distinguish between "this device just doesn't have an ITS
>> mapping" and "something actually went wrong", such that you don't treat
>> the former as an error. That's almost certainly cleaner than e.g. trying
>> to check if dev has an associated MSI domain here before calling it.
> 
> I would agree, even though this way we would end up reserving the ITS
> address regions for all devices mapping to an ITS even though they may
> or may not be affected by the quirk (because IIUC that's just a PCI
> bridge problem), which is what my point is all about.

As I'm sure I've said before, there's no foreseeable issue with that.
For DMA, all it means is that we'll preallocate an identity mapping of
the whole ITS rather than dynamically mapping pages of it as devices
request MSIs; no functional change - if anything, it's slightly more
efficient. For VFIO, it just means that 

Re: [PATCH v7 08/36] x86/mm: Add support to enable SME in early boot processing

[Tom Lendacky]

On 6/20/2017 2:38 AM, Borislav Petkov wrote:

On Fri, Jun 16, 2017 at 01:51:15PM -0500, Tom Lendacky wrote:

Add support to the early boot code to use Secure Memory Encryption (SME).
Since the kernel has been loaded into memory in a decrypted state, encrypt
the kernel in place and update the early pagetables with the memory
encryption mask so that new pagetable entries will use memory encryption.

The routines to set the encryption mask and perform the encryption are
stub routines for now with functionality to be added in a later patch.

Because of the need to have the routines available to head_64.S, the
mem_encrypt.c is always built and #ifdefs in mem_encrypt.c will provide
functionality or stub routines depending on CONFIG_AMD_MEM_ENCRYPT.

Signed-off-by: Tom Lendacky 
---
  arch/x86/include/asm/mem_encrypt.h |8 +++
  arch/x86/kernel/head64.c   |   33 +-
  arch/x86/kernel/head_64.S  |   39 ++--
  arch/x86/mm/Makefile   |4 +---
  arch/x86/mm/mem_encrypt.c  |   24 ++
  5 files changed, 93 insertions(+), 15 deletions(-)


...


diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
index b99d469..9a78277 100644
--- a/arch/x86/mm/mem_encrypt.c
+++ b/arch/x86/mm/mem_encrypt.c
@@ -11,6 +11,9 @@
   */
  
  #include 

+#include 
+
+#ifdef CONFIG_AMD_MEM_ENCRYPT
  
  /*

   * Since SME related variables are set early in the boot process they must
@@ -19,3 +22,24 @@
   */
  unsigned long sme_me_mask __section(.data) = 0;
  EXPORT_SYMBOL_GPL(sme_me_mask);
+
+void __init sme_encrypt_kernel(void)
+{
+}


Just the minor:

void __init sme_encrypt_kernel(void) { }

in case you have to respin.


I have to re-spin for the kbuild test error.  But given that this
function will be filled in later it's probably not worth doing the
space savings here.

Thanks,
Tom



Reviewed-by: Borislav Petkov 


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

Re: [PATCH v2 2/2] iommu/arm-smmu-v3:Enable ACPI based HiSilicon erratum 161010801

[Lorenzo Pieralisi]

On Tue, Jun 20, 2017 at 04:16:18PM +0100, Robin Murphy wrote:
> On 20/06/17 15:07, Shameerali Kolothum Thodi wrote:
> > 
> > 
> >> -Original Message-
> >> From: Lorenzo Pieralisi [mailto:lorenzo.pieral...@arm.com]
> >> Sent: Tuesday, June 20, 2017 11:29 AM
> >> To: Shameerali Kolothum Thodi
> >> Cc: marc.zyng...@arm.com; sudeep.ho...@arm.com; will.dea...@arm.com;
> >> robin.mur...@arm.com; hanjun@linaro.org; Gabriele Paoloni; John
> >> Garry; iommu@lists.linux-foundation.org; linux-arm-
> >> ker...@lists.infradead.org; linux-a...@vger.kernel.org; de...@acpica.org;
> >> Linuxarm; Wangzhou (B); Guohanjun (Hanjun Guo)
> >> Subject: Re: [PATCH v2 2/2] iommu/arm-smmu-v3:Enable ACPI based
> >> HiSilicon erratum 161010801
> >>
> >> Hi Shameer,
> >>
> >> On Mon, Jun 19, 2017 at 04:45:00PM +0100, shameer wrote:
> >>> The HiSilicon erratum 161010801 describes the limitation of HiSilicon
> >>> platforms Hip06/Hip07 to support the SMMU mappings for MSI
> >> transactions.
> >>>
> >>> On these platforms GICv3 ITS translator is presented with the deviceID
> >>> by extending the MSI payload data to 64 bits to include the deviceID.
> >>> Hence, the PCIe controller on this platforms has to differentiate the
> >>> MSI payload against other DMA payload and has to modify the MSI
> >> payload.
> >>> This basically makes it difficult for this platforms to have a SMMU
> >>> translation for MSI.
> >>>
> >>> This patch implements a ACPI table based quirk to reserve the hw msi
> >>> regions in the smmu-v3 driver which means these address regions will
> >>> not be translated and will be excluded from iova allocations.
> >>>
> >>> Signed-off-by: shameer 
> >>> ---
> >>>  drivers/iommu/arm-smmu-v3.c | 29 -
> >>>  1 file changed, 24 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-
> >> v3.c
> >>> index abe4b88..f03c63b 100644
> >>> --- a/drivers/iommu/arm-smmu-v3.c
> >>> +++ b/drivers/iommu/arm-smmu-v3.c
> >>> @@ -597,6 +597,7 @@ struct arm_smmu_device {
> >>>   u32 features;
> >>>
> >>>  #define ARM_SMMU_OPT_SKIP_PREFETCH   (1 << 0)
> >>> +#define ARM_SMMU_OPT_RESV_HW_MSI (1 << 1)
> >>>   u32 options;
> >>>
> >>>   struct arm_smmu_cmdqcmdq;
> >>> @@ -1904,14 +1905,31 @@ static void arm_smmu_get_resv_regions(struct
> >> device *dev,
> >>> struct list_head *head)
> >>>  {
> >>>   struct iommu_resv_region *region;
> >>> + struct arm_smmu_device *smmu;
> >>> + struct iommu_fwspec *fwspec = dev->iommu_fwspec;
> >>>   int prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO;
> >>>
> >>> - region = iommu_alloc_resv_region(MSI_IOVA_BASE,
> >> MSI_IOVA_LENGTH,
> >>> -  prot, IOMMU_RESV_SW_MSI);
> >>> - if (!region)
> >>> - return;
> >>> + smmu = arm_smmu_get_by_fwnode(fwspec->iommu_fwnode);
> >>> +
> >>> + if (smmu && (smmu->options & ARM_SMMU_OPT_RESV_HW_MSI)
> >> &&
> >>> +   dev_is_pci(dev)) {
> >>
> >> IORT changes are fine to me, I am still no big fan of this supposedly
> >> generic option that is _really_ platform specific (in particular as I
> >> said before the quirk depends on the PCI host bridge but in this
> >> patchset I see no such dependency. In short - the quirk is hooked off
> >> the SMMUv3 model which implicitly implies a PCI host bridge
> >> configuration IIUC). It is Will and Robin decision though, I am not sure
> >> you can make it any tidier (given that on ACPI you may not even have
> >> a PCI host bridge specific _HID to base your check above on).
> > 
> > Thanks Lorenzo. I understand your point. As far as our platform is 
> > concerned, I think It is ok to remove the dev_is_pci() check and make
> > it generic, if that helps.  I don't think it will harm us other than couple 
> > of
> >  "HW MSI region resv failed: " logs  for our platform devices.
> 
> I think the answer there is that iort_iommu_its_get_resv_regions()
> really should distinguish between "this device just doesn't have an ITS
> mapping" and "something actually went wrong", such that you don't treat
> the former as an error. That's almost certainly cleaner than e.g. trying
> to check if dev has an associated MSI domain here before calling it.

I would agree, even though this way we would end up reserving the ITS
address regions for all devices mapping to an ITS even though they may
or may not be affected by the quirk (because IIUC that's just a PCI
bridge problem), which is what my point is all about.

It does not seem to be clean-cut, however we slice it.

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

RE: [PATCH v2 2/2] iommu/arm-smmu-v3:Enable ACPI based HiSilicon erratum 161010801

[Shameerali Kolothum Thodi]

> -Original Message-
> From: Robin Murphy [mailto:robin.mur...@arm.com]
> Sent: Tuesday, June 20, 2017 4:16 PM
> To: Shameerali Kolothum Thodi; Lorenzo Pieralisi
> Cc: marc.zyng...@arm.com; sudeep.ho...@arm.com; will.dea...@arm.com;
> hanjun@linaro.org; Gabriele Paoloni; John Garry; iommu@lists.linux-
> foundation.org; linux-arm-ker...@lists.infradead.org; linux-
> a...@vger.kernel.org; de...@acpica.org; Linuxarm; Wangzhou (B);
> Guohanjun (Hanjun Guo)
> Subject: Re: [PATCH v2 2/2] iommu/arm-smmu-v3:Enable ACPI based
> HiSilicon erratum 161010801
> 
> On 20/06/17 15:07, Shameerali Kolothum Thodi wrote:
> >
> >
> >> -Original Message-
> >> From: Lorenzo Pieralisi [mailto:lorenzo.pieral...@arm.com]
> >> Sent: Tuesday, June 20, 2017 11:29 AM
> >> To: Shameerali Kolothum Thodi
> >> Cc: marc.zyng...@arm.com; sudeep.ho...@arm.com;
> will.dea...@arm.com;
> >> robin.mur...@arm.com; hanjun@linaro.org; Gabriele Paoloni; John
> >> Garry; iommu@lists.linux-foundation.org; linux-arm-
> >> ker...@lists.infradead.org; linux-a...@vger.kernel.org;
> de...@acpica.org;
> >> Linuxarm; Wangzhou (B); Guohanjun (Hanjun Guo)
> >> Subject: Re: [PATCH v2 2/2] iommu/arm-smmu-v3:Enable ACPI based
> >> HiSilicon erratum 161010801
> >>
> >> Hi Shameer,
> >>
> >> On Mon, Jun 19, 2017 at 04:45:00PM +0100, shameer wrote:
> >>> The HiSilicon erratum 161010801 describes the limitation of HiSilicon
> >>> platforms Hip06/Hip07 to support the SMMU mappings for MSI
> >> transactions.
> >>>
> >>> On these platforms GICv3 ITS translator is presented with the deviceID
> >>> by extending the MSI payload data to 64 bits to include the deviceID.
> >>> Hence, the PCIe controller on this platforms has to differentiate the
> >>> MSI payload against other DMA payload and has to modify the MSI
> >> payload.
> >>> This basically makes it difficult for this platforms to have a SMMU
> >>> translation for MSI.
> >>>
> >>> This patch implements a ACPI table based quirk to reserve the hw msi
> >>> regions in the smmu-v3 driver which means these address regions will
> >>> not be translated and will be excluded from iova allocations.
> >>>
> >>> Signed-off-by: shameer 
> >>> ---
> >>>  drivers/iommu/arm-smmu-v3.c | 29 -
> >>>  1 file changed, 24 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-
> smmu-
> >> v3.c
> >>> index abe4b88..f03c63b 100644
> >>> --- a/drivers/iommu/arm-smmu-v3.c
> >>> +++ b/drivers/iommu/arm-smmu-v3.c
> >>> @@ -597,6 +597,7 @@ struct arm_smmu_device {
> >>>   u32 features;
> >>>
> >>>  #define ARM_SMMU_OPT_SKIP_PREFETCH   (1 << 0)
> >>> +#define ARM_SMMU_OPT_RESV_HW_MSI (1 << 1)
> >>>   u32 options;
> >>>
> >>>   struct arm_smmu_cmdqcmdq;
> >>> @@ -1904,14 +1905,31 @@ static void
> arm_smmu_get_resv_regions(struct
> >> device *dev,
> >>> struct list_head *head)
> >>>  {
> >>>   struct iommu_resv_region *region;
> >>> + struct arm_smmu_device *smmu;
> >>> + struct iommu_fwspec *fwspec = dev->iommu_fwspec;
> >>>   int prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO;
> >>>
> >>> - region = iommu_alloc_resv_region(MSI_IOVA_BASE,
> >> MSI_IOVA_LENGTH,
> >>> -  prot, IOMMU_RESV_SW_MSI);
> >>> - if (!region)
> >>> - return;
> >>> + smmu = arm_smmu_get_by_fwnode(fwspec->iommu_fwnode);
> >>> +
> >>> + if (smmu && (smmu->options & ARM_SMMU_OPT_RESV_HW_MSI)
> >> &&
> >>> +   dev_is_pci(dev)) {
> >>
> >> IORT changes are fine to me, I am still no big fan of this supposedly
> >> generic option that is _really_ platform specific (in particular as I
> >> said before the quirk depends on the PCI host bridge but in this
> >> patchset I see no such dependency. In short - the quirk is hooked off
> >> the SMMUv3 model which implicitly implies a PCI host bridge
> >> configuration IIUC). It is Will and Robin decision though, I am not sure
> >> you can make it any tidier (given that on ACPI you may not even have
> >> a PCI host bridge specific _HID to base your check above on).
> >
> > Thanks Lorenzo. I understand your point. As far as our platform is
> > concerned, I think It is ok to remove the dev_is_pci() check and make
> > it generic, if that helps.  I don't think it will harm us other than couple 
> > of
> >  "HW MSI region resv failed: " logs  for our platform devices.
> 
> I think the answer there is that iort_iommu_its_get_resv_regions()
> really should distinguish between "this device just doesn't have an ITS
> mapping" and "something actually went wrong", such that you don't treat
> the former as an error. That's almost certainly cleaner than e.g. trying
> to check if dev has an associated MSI domain here before calling it.

If I understood that correctly, the suggestion is to treat cases where device
doesn’t have any ITS node associated with it as 

Re: [PATCH v2 2/2] iommu/arm-smmu-v3:Enable ACPI based HiSilicon erratum 161010801

[Robin Murphy]

On 20/06/17 15:07, Shameerali Kolothum Thodi wrote:
> 
> 
>> -Original Message-
>> From: Lorenzo Pieralisi [mailto:lorenzo.pieral...@arm.com]
>> Sent: Tuesday, June 20, 2017 11:29 AM
>> To: Shameerali Kolothum Thodi
>> Cc: marc.zyng...@arm.com; sudeep.ho...@arm.com; will.dea...@arm.com;
>> robin.mur...@arm.com; hanjun@linaro.org; Gabriele Paoloni; John
>> Garry; iommu@lists.linux-foundation.org; linux-arm-
>> ker...@lists.infradead.org; linux-a...@vger.kernel.org; de...@acpica.org;
>> Linuxarm; Wangzhou (B); Guohanjun (Hanjun Guo)
>> Subject: Re: [PATCH v2 2/2] iommu/arm-smmu-v3:Enable ACPI based
>> HiSilicon erratum 161010801
>>
>> Hi Shameer,
>>
>> On Mon, Jun 19, 2017 at 04:45:00PM +0100, shameer wrote:
>>> The HiSilicon erratum 161010801 describes the limitation of HiSilicon
>>> platforms Hip06/Hip07 to support the SMMU mappings for MSI
>> transactions.
>>>
>>> On these platforms GICv3 ITS translator is presented with the deviceID
>>> by extending the MSI payload data to 64 bits to include the deviceID.
>>> Hence, the PCIe controller on this platforms has to differentiate the
>>> MSI payload against other DMA payload and has to modify the MSI
>> payload.
>>> This basically makes it difficult for this platforms to have a SMMU
>>> translation for MSI.
>>>
>>> This patch implements a ACPI table based quirk to reserve the hw msi
>>> regions in the smmu-v3 driver which means these address regions will
>>> not be translated and will be excluded from iova allocations.
>>>
>>> Signed-off-by: shameer 
>>> ---
>>>  drivers/iommu/arm-smmu-v3.c | 29 -
>>>  1 file changed, 24 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-
>> v3.c
>>> index abe4b88..f03c63b 100644
>>> --- a/drivers/iommu/arm-smmu-v3.c
>>> +++ b/drivers/iommu/arm-smmu-v3.c
>>> @@ -597,6 +597,7 @@ struct arm_smmu_device {
>>> u32 features;
>>>
>>>  #define ARM_SMMU_OPT_SKIP_PREFETCH (1 << 0)
>>> +#define ARM_SMMU_OPT_RESV_HW_MSI   (1 << 1)
>>> u32 options;
>>>
>>> struct arm_smmu_cmdqcmdq;
>>> @@ -1904,14 +1905,31 @@ static void arm_smmu_get_resv_regions(struct
>> device *dev,
>>>   struct list_head *head)
>>>  {
>>> struct iommu_resv_region *region;
>>> +   struct arm_smmu_device *smmu;
>>> +   struct iommu_fwspec *fwspec = dev->iommu_fwspec;
>>> int prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO;
>>>
>>> -   region = iommu_alloc_resv_region(MSI_IOVA_BASE,
>> MSI_IOVA_LENGTH,
>>> -prot, IOMMU_RESV_SW_MSI);
>>> -   if (!region)
>>> -   return;
>>> +   smmu = arm_smmu_get_by_fwnode(fwspec->iommu_fwnode);
>>> +
>>> +   if (smmu && (smmu->options & ARM_SMMU_OPT_RESV_HW_MSI)
>> &&
>>> + dev_is_pci(dev)) {
>>
>> IORT changes are fine to me, I am still no big fan of this supposedly
>> generic option that is _really_ platform specific (in particular as I
>> said before the quirk depends on the PCI host bridge but in this
>> patchset I see no such dependency. In short - the quirk is hooked off
>> the SMMUv3 model which implicitly implies a PCI host bridge
>> configuration IIUC). It is Will and Robin decision though, I am not sure
>> you can make it any tidier (given that on ACPI you may not even have
>> a PCI host bridge specific _HID to base your check above on).
> 
> Thanks Lorenzo. I understand your point. As far as our platform is 
> concerned, I think It is ok to remove the dev_is_pci() check and make
> it generic, if that helps.  I don't think it will harm us other than couple of
>  "HW MSI region resv failed: " logs  for our platform devices.

I think the answer there is that iort_iommu_its_get_resv_regions()
really should distinguish between "this device just doesn't have an ITS
mapping" and "something actually went wrong", such that you don't treat
the former as an error. That's almost certainly cleaner than e.g. trying
to check if dev has an associated MSI domain here before calling it.

Robin.

> 
> Hi Will/Robin,
> Please let me know your thoughts.
> 
> Thanks,
> Shameer
> 
> 

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

[PATCH v8 0/3] Cavium ThunderX2 SMMUv3 errata workarounds

[Geetha sowjanya]

Cavium ThunderX2 SMMUv3 implementation has two Silicon Erratas.
1. Errata ID #74
   SMMU register alias Page 1 is not implemented
2. Errata ID #126
   SMMU doesnt support unique IRQ lines and also MSI for gerror,
   eventq and cmdq-sync

The following patchset does software workaround for these two erratas.

This series is based on patchset.
https://www.spinics.net/lists/arm-kernel/msg578443.html

Changes since v7:
- Added new function "arm_smmu_v3_resource_size" in iort.c to get resource
  size.
- Added new SMMU option "SHARED_IRQ" to enable errata #126 workaround.
- Coding style issues fixed.
- Suggested changes in arm_smmu_device_probe addressed.
- Replaced ACPI_IORT_SMMU_CAVIUM_CN99XX macro with 
ACPI_IORT_SMMU_V3_CAVIUM_CN99XX
   
Changes since v6:
   - Changed device tree compatible string to vendor specific.
   - Rebased on Robin's latest "Update SMMU models for IORT rev. C" v2 patch.
 https://www.spinics.net/lists/arm-kernel/msg582809.html

Changes since v5:
  - Rebased on Robin's "Update SMMU models for IORT rev. C" patch.
 https://www.spinics.net/lists/arm-kernel/msg580728.html
  - Replaced ACPI_IORT_SMMU_V3_CAVIUM_CN99XX macro with 
ACPI_IORT_SMMU_CAVIUM_CN99XX

Changes since v4:
 - Replaced all page1 offset macros ARM_SMMU_EVTQ/PRIQ_PROD/CONS with
arm_smmu_page1_fixup(ARM_SMMU_EVTQ/PRIQ_PROD/CONS, smmu)

Changes since v3:
 - Merged patches 1, 2 and 4 of Version 3.
 - Modified the page1_offset_adjust() and get_irq_flags() implementation as
   suggested by Robin.

Changes since v2:
 - Updated "Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt" document 
with
   new SMMU option used to enable errata workaround.

Changes since v1:
 - Since the use of MIDR register is rejected and SMMU_IIDR is broken on this
   silicon, as suggested by Will Deacon modified the patches to use ThunderX2
   SMMUv3 IORT model number to enable errata workaround.

Geetha Sowjanya (1):
  iommu/arm-smmu-v3: Add workaround for Cavium ThunderX2 erratum #126

Linu Cherian (2):
  ACPI/IORT: Fixup SMMUv3 resource size for Cavium ThunderX2 SMMUv3
model
  iommu/arm-smmu-v3: Add workaround for Cavium ThunderX2 erratum #74

 Documentation/arm64/silicon-errata.txt |2 +
 .../devicetree/bindings/iommu/arm,smmu-v3.txt  |6 ++
 drivers/acpi/arm64/iort.c  |   14 +++-
 drivers/iommu/arm-smmu-v3.c|   93 
 4 files changed, 95 insertions(+), 20 deletions(-)

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

[PATCH v8 3/3] iommu/arm-smmu-v3: Add workaround for Cavium ThunderX2 erratum #126

[Geetha sowjanya]

From: Geetha Sowjanya 

Cavium ThunderX2 SMMU doesn't support MSI and also doesn't have unique irq
lines for gerror, eventq and cmdq-sync.

SHARED_IRQ option is set as a errata workaround, which allows to share the irq
line by register single irq handler for all the interrupts.

Signed-off-by: Geetha sowjanya 
---
 .../devicetree/bindings/iommu/arm,smmu-v3.txt  |5 ++
 drivers/iommu/arm-smmu-v3.c|   73 
 2 files changed, 64 insertions(+), 14 deletions(-)

diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt 
b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
index 6ecc48c..44b40e0 100644
--- a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
+++ b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
@@ -55,6 +55,11 @@ the PCIe specification.
  Set for Caviun ThunderX2 silicon that doesn't support
  SMMU page1 register space.
 
+- cavium,cn9900-broken-unique-irqline
+: Use single irq line for all the SMMUv3 interrupts.
+ Set for Caviun ThunderX2 silicon that doesn't support
+ MSI and also doesn't have unique irq lines for gerror,
+ eventq and cmdq-sync.
 ** Example
 
 smmu@2b40 {
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 2dea4a9..6c0c632 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -598,6 +598,7 @@ struct arm_smmu_device {
 
 #define ARM_SMMU_OPT_SKIP_PREFETCH (1 << 0)
 #define ARM_SMMU_OPT_PAGE0_REGS_ONLY   (1 << 1)
+#define ARM_SMMU_OPT_SHARED_IRQ(1 << 2)
u32 options;
 
struct arm_smmu_cmdqcmdq;
@@ -665,6 +666,7 @@ struct arm_smmu_option_prop {
 static struct arm_smmu_option_prop arm_smmu_options[] = {
{ ARM_SMMU_OPT_SKIP_PREFETCH, "hisilicon,broken-prefetch-cmd" },
{ ARM_SMMU_OPT_PAGE0_REGS_ONLY, "cavium,cn9900-broken-page1-regspace"},
+   { ARM_SMMU_OPT_SHARED_IRQ, "cavium,cn9900-broken-unique-irqline"},
{ 0, NULL},
 };
 
@@ -1313,6 +1315,21 @@ static irqreturn_t arm_smmu_gerror_handler(int irq, void 
*dev)
writel(gerror, smmu->base + ARM_SMMU_GERRORN);
return IRQ_HANDLED;
 }
+/* Shared irq handler*/
+static irqreturn_t arm_smmu_shared_irq_thread(int irq, void *dev)
+{
+   struct arm_smmu_device *smmu = dev;
+   irqreturn_t ret;
+
+   ret = arm_smmu_gerror_handler(irq, dev);
+   if (ret == IRQ_NONE) {
+   arm_smmu_evtq_thread(irq, dev);
+   arm_smmu_cmdq_sync_handler(irq, dev);
+   if (smmu->features & ARM_SMMU_FEAT_PRI)
+   arm_smmu_priq_thread(irq, dev);
+   }
+   return IRQ_HANDLED;
+}
 
 /* IO_PGTABLE API */
 static void __arm_smmu_tlb_sync(struct arm_smmu_device *smmu)
@@ -2230,18 +2247,9 @@ static void arm_smmu_setup_msis(struct arm_smmu_device 
*smmu)
devm_add_action(dev, arm_smmu_free_msis, dev);
 }
 
-static int arm_smmu_setup_irqs(struct arm_smmu_device *smmu)
+static void arm_smmu_setup_unique_irqs(struct arm_smmu_device *smmu)
 {
-   int ret, irq;
-   u32 irqen_flags = IRQ_CTRL_EVTQ_IRQEN | IRQ_CTRL_GERROR_IRQEN;
-
-   /* Disable IRQs first */
-   ret = arm_smmu_write_reg_sync(smmu, 0, ARM_SMMU_IRQ_CTRL,
- ARM_SMMU_IRQ_CTRLACK);
-   if (ret) {
-   dev_err(smmu->dev, "failed to disable irqs\n");
-   return ret;
-   }
+   int irq, ret;
 
arm_smmu_setup_msis(smmu);
 
@@ -2284,10 +2292,46 @@ static int arm_smmu_setup_irqs(struct arm_smmu_device 
*smmu)
if (ret < 0)
dev_warn(smmu->dev,
 "failed to enable priq irq\n");
-   else
-   irqen_flags |= IRQ_CTRL_PRIQ_IRQEN;
}
}
+}
+
+static void arm_smmu_setup_shared_irqs(struct arm_smmu_device *smmu)
+{
+   int ret, irq;
+
+   /* Single irq is used for all queues, request single interrupt lines */
+   irq = smmu->evtq.q.irq;
+   if (irq) {
+   ret = devm_request_threaded_irq(smmu->dev, irq, NULL,
+   arm_smmu_shared_irq_thread,
+   IRQF_ONESHOT | IRQF_SHARED,
+   "arm-smmu-v3-shared_irq", smmu);
+   if (ret < 0)
+   dev_warn(smmu->dev, "failed to enable shared irq\n");
+   }
+}
+
+static int arm_smmu_setup_irqs(struct arm_smmu_device *smmu)
+{
+   int ret;
+   u32 irqen_flags = IRQ_CTRL_EVTQ_IRQEN | IRQ_CTRL_GERROR_IRQEN;
+
+   /* Disable IRQs first */
+   ret = arm_smmu_write_reg_sync(smmu, 0, ARM_SMMU_IRQ_CTRL,
+ ARM_SMMU_IRQ_CTRLACK);
+  

[PATCH v8 2/3] iommu/arm-smmu-v3: Add workaround for Cavium ThunderX2 erratum #74

[Geetha sowjanya]

From: Linu Cherian 

Cavium ThunderX2 SMMU implementation doesn't support page 1 register space
and PAGE0_REGS_ONLY option is enabled as an errata workaround.
This option when turned on, replaces all page 1 offsets used for
EVTQ_PROD/CONS, PRIQ_PROD/CONS register access with page 0 offsets.

SMMU resource size checks are now based on SMMU option PAGE0_REGS_ONLY,
since resource size can be either 64k/128k.
For this, arm_smmu_device_dt_probe/acpi_probe has been moved before
platform_get_resource call, so that SMMU options are set beforehand.

Signed-off-by: Linu Cherian 
Signed-off-by: Geetha Sowjanya 
---
 Documentation/arm64/silicon-errata.txt |1 +
 .../devicetree/bindings/iommu/arm,smmu-v3.txt  |6 ++
 drivers/iommu/arm-smmu-v3.c|   68 ++-
 3 files changed, 57 insertions(+), 18 deletions(-)

diff --git a/Documentation/arm64/silicon-errata.txt 
b/Documentation/arm64/silicon-errata.txt
index 10f2ddd..4693a32 100644
--- a/Documentation/arm64/silicon-errata.txt
+++ b/Documentation/arm64/silicon-errata.txt
@@ -62,6 +62,7 @@ stable kernels.
 | Cavium | ThunderX GICv3  | #23154  | CAVIUM_ERRATUM_23154
|
 | Cavium | ThunderX Core   | #27456  | CAVIUM_ERRATUM_27456
|
 | Cavium | ThunderX SMMUv2 | #27704  | N/A 
|
+| Cavium | ThunderX2 SMMUv3| #74 | N/A 
|
 || | | 
|
 | Freescale/NXP  | LS2080A/LS1043A | A-008585| FSL_ERRATUM_A008585 
|
 || | | 
|
diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt 
b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
index be57550..6ecc48c 100644
--- a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
+++ b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
@@ -49,6 +49,12 @@ the PCIe specification.
 - hisilicon,broken-prefetch-cmd
 : Avoid sending CMD_PREFETCH_* commands to the SMMU.
 
+- cavium,cn9900-broken-page1-regspace
+: Replaces all page 1 offsets used for EVTQ_PROD/CONS,
+ PRIQ_PROD/CONS register access with page 0 offsets.
+ Set for Caviun ThunderX2 silicon that doesn't support
+ SMMU page1 register space.
+
 ** Example
 
 smmu@2b40 {
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 380969a..5de258f 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -597,6 +597,7 @@ struct arm_smmu_device {
u32 features;
 
 #define ARM_SMMU_OPT_SKIP_PREFETCH (1 << 0)
+#define ARM_SMMU_OPT_PAGE0_REGS_ONLY   (1 << 1)
u32 options;
 
struct arm_smmu_cmdqcmdq;
@@ -663,9 +664,20 @@ struct arm_smmu_option_prop {
 
 static struct arm_smmu_option_prop arm_smmu_options[] = {
{ ARM_SMMU_OPT_SKIP_PREFETCH, "hisilicon,broken-prefetch-cmd" },
+   { ARM_SMMU_OPT_PAGE0_REGS_ONLY, "cavium,cn9900-broken-page1-regspace"},
{ 0, NULL},
 };
 
+static inline void __iomem *arm_smmu_page1_fixup(unsigned long offset,
+struct arm_smmu_device *smmu)
+{
+   if ((offset > SZ_64K) &&
+   (smmu->options & ARM_SMMU_OPT_PAGE0_REGS_ONLY))
+   offset -= SZ_64K;
+
+   return smmu->base + offset;
+}
+
 static struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom)
 {
return container_of(dom, struct arm_smmu_domain, domain);
@@ -1961,8 +1973,8 @@ static int arm_smmu_init_one_queue(struct arm_smmu_device 
*smmu,
return -ENOMEM;
}
 
-   q->prod_reg = smmu->base + prod_off;
-   q->cons_reg = smmu->base + cons_off;
+   q->prod_reg = arm_smmu_page1_fixup(prod_off, smmu);
+   q->cons_reg = arm_smmu_page1_fixup(cons_off, smmu);
q->ent_dwords   = dwords;
 
q->q_base  = Q_BASE_RWA;
@@ -2363,8 +2375,10 @@ static int arm_smmu_device_reset(struct arm_smmu_device 
*smmu, bool bypass)
 
/* Event queue */
writeq_relaxed(smmu->evtq.q.q_base, smmu->base + ARM_SMMU_EVTQ_BASE);
-   writel_relaxed(smmu->evtq.q.prod, smmu->base + ARM_SMMU_EVTQ_PROD);
-   writel_relaxed(smmu->evtq.q.cons, smmu->base + ARM_SMMU_EVTQ_CONS);
+   writel_relaxed(smmu->evtq.q.prod,
+  arm_smmu_page1_fixup(ARM_SMMU_EVTQ_PROD, smmu));
+   writel_relaxed(smmu->evtq.q.cons,
+  arm_smmu_page1_fixup(ARM_SMMU_EVTQ_CONS, smmu));
 
enables |= CR0_EVTQEN;
ret = arm_smmu_write_reg_sync(smmu, enables, ARM_SMMU_CR0,
@@ -2379,9 +2393,9 @@ static int arm_smmu_device_reset(struct 

[PATCH v8 1/3] ACPI/IORT: Fixup SMMUv3 resource size for Cavium ThunderX2 SMMUv3 model

[Geetha sowjanya]

From: Linu Cherian 

Cavium ThunderX2 implementation doesn't support second page in SMMU
register space. Hence, resource size is set as 64k for this model.

Signed-off-by: Linu Cherian 
Signed-off-by: Geetha Sowjanya 
---
 drivers/acpi/arm64/iort.c |   15 ++-
 1 files changed, 14 insertions(+), 1 deletions(-)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index c5fecf9..c166f3e 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -828,6 +828,18 @@ static int __init arm_smmu_v3_count_resources(struct 
acpi_iort_node *node)
return num_res;
 }
 
+static unsigned long arm_smmu_v3_resource_size(struct acpi_iort_smmu_v3 *smmu)
+{
+   /*
+* Override the size, for Cavium ThunderX2 implementation
+* which doesn't support the page 1 SMMU register space.
+*/
+   if (smmu->model == ACPI_IORT_SMMU_V3_CAVIUM_CN99XX)
+   return SZ_64K;
+
+   return SZ_128K;
+}
+
 static void __init arm_smmu_v3_init_resources(struct resource *res,
  struct acpi_iort_node *node)
 {
@@ -838,7 +850,8 @@ static void __init arm_smmu_v3_init_resources(struct 
resource *res,
smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
 
res[num_res].start = smmu->base_address;
-   res[num_res].end = smmu->base_address + SZ_128K - 1;
+   res[num_res].end = smmu->base_address +
+   arm_smmu_v3_resource_size(smmu) - 1;
res[num_res].flags = IORESOURCE_MEM;
 
num_res++;
-- 
1.7.1

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

RE: [PATCH v2 2/2] iommu/arm-smmu-v3:Enable ACPI based HiSilicon erratum 161010801

[Shameerali Kolothum Thodi]

> -Original Message-
> From: Lorenzo Pieralisi [mailto:lorenzo.pieral...@arm.com]
> Sent: Tuesday, June 20, 2017 11:29 AM
> To: Shameerali Kolothum Thodi
> Cc: marc.zyng...@arm.com; sudeep.ho...@arm.com; will.dea...@arm.com;
> robin.mur...@arm.com; hanjun@linaro.org; Gabriele Paoloni; John
> Garry; iommu@lists.linux-foundation.org; linux-arm-
> ker...@lists.infradead.org; linux-a...@vger.kernel.org; de...@acpica.org;
> Linuxarm; Wangzhou (B); Guohanjun (Hanjun Guo)
> Subject: Re: [PATCH v2 2/2] iommu/arm-smmu-v3:Enable ACPI based
> HiSilicon erratum 161010801
> 
> Hi Shameer,
> 
> On Mon, Jun 19, 2017 at 04:45:00PM +0100, shameer wrote:
> > The HiSilicon erratum 161010801 describes the limitation of HiSilicon
> > platforms Hip06/Hip07 to support the SMMU mappings for MSI
> transactions.
> >
> > On these platforms GICv3 ITS translator is presented with the deviceID
> > by extending the MSI payload data to 64 bits to include the deviceID.
> > Hence, the PCIe controller on this platforms has to differentiate the
> > MSI payload against other DMA payload and has to modify the MSI
> payload.
> > This basically makes it difficult for this platforms to have a SMMU
> > translation for MSI.
> >
> > This patch implements a ACPI table based quirk to reserve the hw msi
> > regions in the smmu-v3 driver which means these address regions will
> > not be translated and will be excluded from iova allocations.
> >
> > Signed-off-by: shameer 
> > ---
> >  drivers/iommu/arm-smmu-v3.c | 29 -
> >  1 file changed, 24 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-
> v3.c
> > index abe4b88..f03c63b 100644
> > --- a/drivers/iommu/arm-smmu-v3.c
> > +++ b/drivers/iommu/arm-smmu-v3.c
> > @@ -597,6 +597,7 @@ struct arm_smmu_device {
> > u32 features;
> >
> >  #define ARM_SMMU_OPT_SKIP_PREFETCH (1 << 0)
> > +#define ARM_SMMU_OPT_RESV_HW_MSI   (1 << 1)
> > u32 options;
> >
> > struct arm_smmu_cmdqcmdq;
> > @@ -1904,14 +1905,31 @@ static void arm_smmu_get_resv_regions(struct
> device *dev,
> >   struct list_head *head)
> >  {
> > struct iommu_resv_region *region;
> > +   struct arm_smmu_device *smmu;
> > +   struct iommu_fwspec *fwspec = dev->iommu_fwspec;
> > int prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO;
> >
> > -   region = iommu_alloc_resv_region(MSI_IOVA_BASE,
> MSI_IOVA_LENGTH,
> > -prot, IOMMU_RESV_SW_MSI);
> > -   if (!region)
> > -   return;
> > +   smmu = arm_smmu_get_by_fwnode(fwspec->iommu_fwnode);
> > +
> > +   if (smmu && (smmu->options & ARM_SMMU_OPT_RESV_HW_MSI)
> &&
> > + dev_is_pci(dev)) {
> 
> IORT changes are fine to me, I am still no big fan of this supposedly
> generic option that is _really_ platform specific (in particular as I
> said before the quirk depends on the PCI host bridge but in this
> patchset I see no such dependency. In short - the quirk is hooked off
> the SMMUv3 model which implicitly implies a PCI host bridge
> configuration IIUC). It is Will and Robin decision though, I am not sure
> you can make it any tidier (given that on ACPI you may not even have
> a PCI host bridge specific _HID to base your check above on).

Thanks Lorenzo. I understand your point. As far as our platform is 
concerned, I think It is ok to remove the dev_is_pci() check and make
it generic, if that helps.  I don't think it will harm us other than couple of
 "HW MSI region resv failed: " logs  for our platform devices.

Hi Will/Robin,
Please let me know your thoughts.

Thanks,
Shameer


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

Re: [PATCH 0/8] io-pgtable lock removal

[Robin Murphy]

On 15/06/17 01:40, Ray Jui wrote:
> Hi Robin,
> 
> I have applied this patch series on top of v4.12-rc4, and ran various
> Ethernet and NVMf target throughput tests on it.
> 
> To give you some background of my setup:
> 
> The system is a ARMv8 based system with 8 cores. It has various PCIe
> root complexes that can be used to connect to PCIe endpoint devices
> including NIC cards and NVMe SSDs.
> 
> I'm particularly interested in the performance of the PCIe root complex
> that connects to the NIC card, and during my test, IOMMU is
> enabled/disabled against that particular PCIe root complex. The root
> complexes connected to NVMe SSDs remain unchanged (i.e., without IOMMU).
> 
> For the Ethernet throughput out of 50G link:
> 
> Note during the multiple TCP session test, each session will be spread
> to different CPU cores for optimized performance
> 
> Without IOMMU:
> 
> TX TCP x1 - 29.7 Gbps
> TX TCP x4 - 30.5 Gbps
> TX TCP x8 - 28 Gbps
> 
> RX TCP x1 - 15 Gbps
> RX TCP x4 - 33.7 Gbps
> RX TCP x8 - 36 Gbps
> 
> With IOMMU, but without your latest patch:
> 
> TX TCP x1 - 15.2 Gbps
> TX TCP x4 - 14.3 Gbps
> TX TCP x8 - 13 Gbps
> 
> RX TCP x1 - 7.88 Gbps
> RX TCP x4 - 13.2 Gbps
> RX TCP x8 - 12.6 Gbps
> 
> With IOMMU and your latest patch:
> 
> TX TCP x1 - 21.4 Gbps
> TX TCP x4 - 30.5 Gbps
> TX TCP x8 - 21.3 Gbps
> 
> RX TCP x1 - 7.7 Gbps
> RX TCP x4 - 20.1 Gbps
> RX TCP x8 - 27.1 Gbps

Cool, those seem more or less in line with expectations. Nate's
currently cooking a patch to further reduce the overhead when unmapping
multi-page buffers, which we believe should make up most of the rest of
the difference.

> With the NVMf target test with 4 SSDs, fio based test, random read, 4k,
> 8 jobs:
> 
> Without IOMMU:
> 
> IOPS = 1080K
> 
> With IOMMU, but without your latest patch:
> 
> IOPS = 520K
> 
> With IOMMU and your latest patch:
> 
> IOPS = 500K ~ 850K (a lot of variation observed during the same test run)

That does seem a bit off - are you able to try some perf profiling to
get a better idea of where the overhead appears to be?

> As you can see, performance has improved significantly with this patch
> series! That is very impressive!
> 
> However, it is still off, compared to the test runs without the IOMMU.
> I'm wondering if more improvement is expected.
> 
> In addition, a much larger throughput variation is observed in the tests
> with these latest patches, when multiple CPUs are involved. I'm
> wondering if that is caused by some remaining lock in the driver?

Assuming this is the platform with MMU-500, there shouldn't be any locks
left, since that shouldn't have the hardware ATOS registers for
iova_to_phys().

> Also, in a few occasions, I observed the following message during the
> test, when multiple cores are involved:
> 
> arm-smmu 6400.mmu: TLB sync timed out -- SMMU may be deadlocked

That's particularly worrying, because it means we spent over a second
waiting for something that normally shouldn't take more than a few
hundred cycles. The only time I've ever actually seen that happen is if
TLBSYNC is issued while a context fault is pending - on MMU-500 it seems
that the sync just doesn't proceed until the fault is cleared - but that
stemmed from interrupts not being wired up correctly (on FPGAs) such
that we never saw the fault reported in the first place :/

Robin.

> 
> Thanks,
> 
> Ray
> 
> On 6/9/17 12:28 PM, Nate Watterson wrote:
>> Hi Robin,
>>
>> On 6/8/2017 7:51 AM, Robin Murphy wrote:
>>> Hi all,
>>>
>>> Here's the cleaned up nominally-final version of the patches everybody's
>>> keen to see. #1 is just a non-critical thing-I-spotted-in-passing fix,
>>> #2-#4 do some preparatory work (and bid farewell to everyone's least
>>> favourite bit of code, hooray!), and #5-#8 do the dirty deed itself.
>>>
>>> The branch I've previously shared has been updated too:
>>>
>>>git://linux-arm.org/linux-rm  iommu/pgtable
>>>
>>> All feedback welcome, as I'd really like to land this for 4.13.
>>>
>>
>> I tested the series on a QDF2400 development platform and see notable
>> performance improvements particularly in workloads that make concurrent
>> accesses to a single iommu_domain.
>>
>>> Robin.
>>>
>>>
>>> Robin Murphy (8):
>>>iommu/io-pgtable-arm-v7s: Check table PTEs more precisely
>>>iommu/io-pgtable-arm: Improve split_blk_unmap
>>>iommu/io-pgtable-arm-v7s: Refactor split_blk_unmap
>>>iommu/io-pgtable: Introduce explicit coherency
>>>iommu/io-pgtable-arm: Support lockless operation
>>>iommu/io-pgtable-arm-v7s: Support lockless operation
>>>iommu/arm-smmu: Remove io-pgtable spinlock
>>>iommu/arm-smmu-v3: Remove io-pgtable spinlock
>>>
>>>   drivers/iommu/arm-smmu-v3.c|  36 ++-
>>>   drivers/iommu/arm-smmu.c   |  48 --
>>>   drivers/iommu/io-pgtable-arm-v7s.c | 173
>>> +
>>>   drivers/iommu/io-pgtable-arm.c | 190
>>> -
>>>   drivers/iommu/io-pgtable.h 

Re: clean up and modularize arch dma_mapping interface

[Christoph Hellwig]

On Tue, Jun 20, 2017 at 11:19:02AM +0200, Daniel Vetter wrote:
> Ack for the 2 drm patches, but I can also pick them up through drm-misc if
> you prefer that (but then it'll be 4.14).

Nah, I'll plan to set up a dma-mapping tree so that we'll have common
place for dma-mapping work.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: new dma-mapping tree, was Re: clean up and modularize arch dma_mapping interface V2

[Christoph Hellwig]

On Tue, Jun 20, 2017 at 11:04:00PM +1000, Stephen Rothwell wrote:
> git://git.linaro.org/people/mszyprowski/linux-dma-mapping.git#dma-mapping-next
> 
> Contacts: Marek Szyprowski and Kyungmin Park (cc'd)
> 
> I have called your tree dma-mapping-hch for now.  The other tree has
> not been updated since 4.9-rc1 and I am not sure how general it is.
> Marek, Kyungmin, any comments?

I'd be happy to join efforts - co-maintainers and reviers are always
welcome.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: new dma-mapping tree, was Re: clean up and modularize arch dma_mapping interface V2

[Christoph Hellwig]

On Tue, Jun 20, 2017 at 02:14:36PM +0100, Robin Murphy wrote:
> Hi Christoph,
> 
> On 20/06/17 13:41, Christoph Hellwig wrote:
> > On Fri, Jun 16, 2017 at 08:10:15PM +0200, Christoph Hellwig wrote:
> >> I plan to create a new dma-mapping tree to collect all this work.
> >> Any volunteers for co-maintainers, especially from the iommu gang?
> > 
> > Ok, I've created the new tree:
> > 
> >git://git.infradead.org/users/hch/dma-mapping.git for-next
> > 
> > Gitweb:
> > 
> >
> > http://git.infradead.org/users/hch/dma-mapping.git/shortlog/refs/heads/for-next
> > 
> > And below is the patch to add the MAINTAINERS entry, additions welcome.
> 
> I'm happy to be a reviewer, since I've been working in this area for
> some time, particularly with the dma-iommu code and arm64 DMA ops.

Great, I'll add you!
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: new dma-mapping tree, was Re: clean up and modularize arch dma_mapping interface V2

[Robin Murphy]

Hi Christoph,

On 20/06/17 13:41, Christoph Hellwig wrote:
> On Fri, Jun 16, 2017 at 08:10:15PM +0200, Christoph Hellwig wrote:
>> I plan to create a new dma-mapping tree to collect all this work.
>> Any volunteers for co-maintainers, especially from the iommu gang?
> 
> Ok, I've created the new tree:
> 
>git://git.infradead.org/users/hch/dma-mapping.git for-next
> 
> Gitweb:
> 
>
> http://git.infradead.org/users/hch/dma-mapping.git/shortlog/refs/heads/for-next
> 
> And below is the patch to add the MAINTAINERS entry, additions welcome.

I'm happy to be a reviewer, since I've been working in this area for
some time, particularly with the dma-iommu code and arm64 DMA ops.

Robin.

> Stephen, can you add this to linux-next?
> 
> ---
> From 335979c41912e6c101a20b719862b2d837370df1 Mon Sep 17 00:00:00 2001
> From: Christoph Hellwig 
> Date: Tue, 20 Jun 2017 11:17:30 +0200
> Subject: MAINTAINERS: add entry for dma mapping helpers
> 
> This code has been spread between getting in through arch trees, the iommu
> tree, -mm and the drivers tree.  There will be a lot of work in this area,
> including consolidating various arch implementations into more common
> code, so ensure we have a proper git tree that facilitates cooperation with
> the architecture maintainers.
> 
> Signed-off-by: Christoph Hellwig 
> ---
>  MAINTAINERS | 13 +
>  1 file changed, 13 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 09b5ab6a8a5c..56859d53a424 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2595,6 +2595,19 @@ S: Maintained
>  F:   net/bluetooth/
>  F:   include/net/bluetooth/
>  
> +DMA MAPPING HELPERS
> +M:   Christoph Hellwig 
> +L:   linux-ker...@vger.kernel.org
> +T:   git git://git.infradead.org/users/hch/dma-mapping.git
> +W:   http://git.infradead.org/users/hch/dma-mapping.git
> +S:   Supported
> +F:   lib/dma-debug.c
> +F:   lib/dma-noop.c
> +F:   lib/dma-virt.c
> +F:   drivers/base/dma-mapping.c
> +F:   drivers/base/dma-coherent.c
> +F:   include/linux/dma-mapping.h
> +
>  BONDING DRIVER
>  M:   Jay Vosburgh 
>  M:   Veaceslav Falico 
> 

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

RE: [PATCH v2 2/2] iommu/arm-smmu-v3:Enable ACPI based HiSilicon erratum 161010801

[Shameerali Kolothum Thodi]

> -Original Message-
> From: Robin Murphy [mailto:robin.mur...@arm.com]
> Sent: Monday, June 19, 2017 6:42 PM
> To: Shameerali Kolothum Thodi; lorenzo.pieral...@arm.com;
> marc.zyng...@arm.com; sudeep.ho...@arm.com; will.dea...@arm.com;
> hanjun@linaro.org
> Cc: Gabriele Paoloni; John Garry; iommu@lists.linux-foundation.org; linux-
> arm-ker...@lists.infradead.org; linux-a...@vger.kernel.org;
> de...@acpica.org; Linuxarm; Wangzhou (B); Guohanjun (Hanjun Guo)
> Subject: Re: [PATCH v2 2/2] iommu/arm-smmu-v3:Enable ACPI based
> HiSilicon erratum 161010801
> 
> On 19/06/17 16:45, shameer wrote:
> > The HiSilicon erratum 161010801 describes the limitation of HiSilicon
> > platforms Hip06/Hip07 to support the SMMU mappings for MSI
> transactions.
> >
> > On these platforms GICv3 ITS translator is presented with the deviceID
> > by extending the MSI payload data to 64 bits to include the deviceID.
> > Hence, the PCIe controller on this platforms has to differentiate the
> > MSI payload against other DMA payload and has to modify the MSI
> payload.
> > This basically makes it difficult for this platforms to have a SMMU
> > translation for MSI.
> >
> > This patch implements a ACPI table based quirk to reserve the hw msi
> > regions in the smmu-v3 driver which means these address regions will
> > not be translated and will be excluded from iova allocations.
> >
> > Signed-off-by: shameer 
> > ---
> >  drivers/iommu/arm-smmu-v3.c | 29 -
> >  1 file changed, 24 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-
> v3.c
> > index abe4b88..f03c63b 100644
> > --- a/drivers/iommu/arm-smmu-v3.c
> > +++ b/drivers/iommu/arm-smmu-v3.c
> > @@ -597,6 +597,7 @@ struct arm_smmu_device {
> > u32 features;
> >
> >  #define ARM_SMMU_OPT_SKIP_PREFETCH (1 << 0)
> > +#define ARM_SMMU_OPT_RESV_HW_MSI   (1 << 1)
> > u32 options;
> >
> > struct arm_smmu_cmdqcmdq;
> > @@ -1904,14 +1905,31 @@ static void arm_smmu_get_resv_regions(struct
> device *dev,
> >   struct list_head *head)
> >  {
> > struct iommu_resv_region *region;
> > +   struct arm_smmu_device *smmu;
> > +   struct iommu_fwspec *fwspec = dev->iommu_fwspec;
> > int prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO;
> >
> > -   region = iommu_alloc_resv_region(MSI_IOVA_BASE,
> MSI_IOVA_LENGTH,
> > -prot, IOMMU_RESV_SW_MSI);
> > -   if (!region)
> > -   return;
> > +   smmu = arm_smmu_get_by_fwnode(fwspec->iommu_fwnode);
> > +
> > +   if (smmu && (smmu->options & ARM_SMMU_OPT_RESV_HW_MSI)
> &&
> 
> AFAICS, iommu_get_resv_regions is only ever called on devices which are
> at least already part of an iommu_group, so smmu should never
> legitimately be NULL. I'd say if you really want to be robust against
> flagrant API misuse, at least WARN_ON and bail out immediately.

Ok.  I will address this in next revision.

Thanks,
Shameer

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

Re: new dma-mapping tree, was Re: clean up and modularize arch dma_mapping interface V2

[Stephen Rothwell]

Hi Christoph,

On Tue, 20 Jun 2017 14:41:40 +0200 Christoph Hellwig  wrote:
>
> On Fri, Jun 16, 2017 at 08:10:15PM +0200, Christoph Hellwig wrote:
> > I plan to create a new dma-mapping tree to collect all this work.
> > Any volunteers for co-maintainers, especially from the iommu gang?  
> 
> Ok, I've created the new tree:
> 
>git://git.infradead.org/users/hch/dma-mapping.git for-next
> 
> Gitweb:
> 
>
> http://git.infradead.org/users/hch/dma-mapping.git/shortlog/refs/heads/for-next
> 
> And below is the patch to add the MAINTAINERS entry, additions welcome.
> 
> Stephen, can you add this to linux-next?

Added from tomorrow.

I have another tree called dma-mapping:

git://git.linaro.org/people/mszyprowski/linux-dma-mapping.git#dma-mapping-next

Contacts: Marek Szyprowski and Kyungmin Park (cc'd)

I have called your tree dma-mapping-hch for now.  The other tree has
not been updated since 4.9-rc1 and I am not sure how general it is.
Marek, Kyungmin, any comments?

Thanks for adding your subsystem tree as a participant of linux-next.  As
you may know, this is not a judgement of your code.  The purpose of
linux-next is for integration testing and to lower the impact of
conflicts between subsystems in the next merge window. 

You will need to ensure that the patches/commits in your tree/series have
been:
 * submitted under GPL v2 (or later) and include the Contributor's
Signed-off-by,
 * posted to the relevant mailing list,
 * reviewed by you (or another maintainer of your subsystem tree),
 * successfully unit tested, and 
 * destined for the current or next Linux merge window.

Basically, this should be just what you would send to Linus (or ask him
to fetch).  It is allowed to be rebased if you deem it necessary.

-- 
Cheers,
Stephen Rothwell 
s...@canb.auug.org.au
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

new dma-mapping tree, was Re: clean up and modularize arch dma_mapping interface V2

[Christoph Hellwig]

On Fri, Jun 16, 2017 at 08:10:15PM +0200, Christoph Hellwig wrote:
> I plan to create a new dma-mapping tree to collect all this work.
> Any volunteers for co-maintainers, especially from the iommu gang?

Ok, I've created the new tree:

   git://git.infradead.org/users/hch/dma-mapping.git for-next

Gitweb:

   
http://git.infradead.org/users/hch/dma-mapping.git/shortlog/refs/heads/for-next

And below is the patch to add the MAINTAINERS entry, additions welcome.

Stephen, can you add this to linux-next?

---
>From 335979c41912e6c101a20b719862b2d837370df1 Mon Sep 17 00:00:00 2001
From: Christoph Hellwig 
Date: Tue, 20 Jun 2017 11:17:30 +0200
Subject: MAINTAINERS: add entry for dma mapping helpers

This code has been spread between getting in through arch trees, the iommu
tree, -mm and the drivers tree.  There will be a lot of work in this area,
including consolidating various arch implementations into more common
code, so ensure we have a proper git tree that facilitates cooperation with
the architecture maintainers.

Signed-off-by: Christoph Hellwig 
---
 MAINTAINERS | 13 +
 1 file changed, 13 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 09b5ab6a8a5c..56859d53a424 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2595,6 +2595,19 @@ S:   Maintained
 F: net/bluetooth/
 F: include/net/bluetooth/
 
+DMA MAPPING HELPERS
+M: Christoph Hellwig 
+L: linux-ker...@vger.kernel.org
+T: git git://git.infradead.org/users/hch/dma-mapping.git
+W: http://git.infradead.org/users/hch/dma-mapping.git
+S: Supported
+F: lib/dma-debug.c
+F: lib/dma-noop.c
+F: lib/dma-virt.c
+F: drivers/base/dma-mapping.c
+F: drivers/base/dma-coherent.c
+F: include/linux/dma-mapping.h
+
 BONDING DRIVER
 M: Jay Vosburgh 
 M: Veaceslav Falico 
-- 
2.11.0

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

Re: [PATCH 2/7] dt-bindings: PCI: Describe ATS property for root complex nodes

[Jean-Philippe Brucker]

On 06/06/2017 12:11 PM, Jean-Philippe Brucker wrote:
> On 05/06/17 18:20, Rob Herring wrote:
>> pci-host-{e,}cam-generic is a special case. I'm okay with having a 
>> property for that I suppose. We should not require this property though 
>> and allow for it to be implied by the SoC specific compatible as well.
> 
> The property isn't useful for host-generic since a host like Qemu can
> easily disable ATS in the IOMMU. It would only be here for consistency.
> Changing IOMMU registers wouldn't be as simple for a firmware running on
> real hardware.

Sorry I was mistaken here. AMD Seattle hardware, for instance, implements
a pure host-ecam-generic. So it's not exclusively used by Qemu. A quick
search in the kernel also brings up alpine and thunder2. So I guess I'll
move ats-supported documentation to host-generic-pci.txt.

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

Re: [PATCH 1/1] iommu/arm-smmu-v3: replace writel with writel_relaxed in queue_inc_prod

[Robin Murphy]

On 20/06/17 12:04, Zhen Lei wrote:
> This function is protected by spinlock, and the latter will do memory
> barrier implicitly. So that we can safely use writel_relaxed. In fact, the
> dmb operation will lengthen the time protected by lock, which indirectly
> increase the locking confliction in the stress scene.

If you remove the DSB between writing the commands (to Normal memory)
and writing the pointer (to Device memory), how can you guarantee that
the complete command is visible to the SMMU and it isn't going to try to
consume stale memory contents? The spinlock is irrelevant since it's
taken *before* the command is written.

Robin.

> Signed-off-by: Zhen Lei 
> ---
>  drivers/iommu/arm-smmu-v3.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index 380969a..d2fbee3 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -728,7 +728,7 @@ static void queue_inc_prod(struct arm_smmu_queue *q)
>   u32 prod = (Q_WRP(q, q->prod) | Q_IDX(q, q->prod)) + 1;
> 
>   q->prod = Q_OVF(q, q->prod) | Q_WRP(q, prod) | Q_IDX(q, prod);
> - writel(q->prod, q->prod_reg);
> + writel_relaxed(q->prod, q->prod_reg);
>  }
> 
>  /*
> --
> 2.5.0
> 
> 

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

[PATCH 1/1] iommu/arm-smmu-v3: replace writel with writel_relaxed in queue_inc_prod

[Zhen Lei]

This function is protected by spinlock, and the latter will do memory
barrier implicitly. So that we can safely use writel_relaxed. In fact, the
dmb operation will lengthen the time protected by lock, which indirectly
increase the locking confliction in the stress scene.

Signed-off-by: Zhen Lei 
---
 drivers/iommu/arm-smmu-v3.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 380969a..d2fbee3 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -728,7 +728,7 @@ static void queue_inc_prod(struct arm_smmu_queue *q)
u32 prod = (Q_WRP(q, q->prod) | Q_IDX(q, q->prod)) + 1;

q->prod = Q_OVF(q, q->prod) | Q_WRP(q, prod) | Q_IDX(q, prod);
-   writel(q->prod, q->prod_reg);
+   writel_relaxed(q->prod, q->prod_reg);
 }

 /*
--
2.5.0


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

Re: [PATCH v2 2/2] iommu/arm-smmu-v3:Enable ACPI based HiSilicon erratum 161010801

[Lorenzo Pieralisi]

Hi Shameer,

On Mon, Jun 19, 2017 at 04:45:00PM +0100, shameer wrote:
> The HiSilicon erratum 161010801 describes the limitation of HiSilicon
> platforms Hip06/Hip07 to support the SMMU mappings for MSI transactions.
> 
> On these platforms GICv3 ITS translator is presented with the deviceID
> by extending the MSI payload data to 64 bits to include the deviceID.
> Hence, the PCIe controller on this platforms has to differentiate the
> MSI payload against other DMA payload and has to modify the MSI payload.
> This basically makes it difficult for this platforms to have a SMMU
> translation for MSI.
> 
> This patch implements a ACPI table based quirk to reserve the hw msi
> regions in the smmu-v3 driver which means these address regions will
> not be translated and will be excluded from iova allocations.
> 
> Signed-off-by: shameer 
> ---
>  drivers/iommu/arm-smmu-v3.c | 29 -
>  1 file changed, 24 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index abe4b88..f03c63b 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -597,6 +597,7 @@ struct arm_smmu_device {
>   u32 features;
>  
>  #define ARM_SMMU_OPT_SKIP_PREFETCH   (1 << 0)
> +#define ARM_SMMU_OPT_RESV_HW_MSI (1 << 1)
>   u32 options;
>  
>   struct arm_smmu_cmdqcmdq;
> @@ -1904,14 +1905,31 @@ static void arm_smmu_get_resv_regions(struct device 
> *dev,
> struct list_head *head)
>  {
>   struct iommu_resv_region *region;
> + struct arm_smmu_device *smmu;
> + struct iommu_fwspec *fwspec = dev->iommu_fwspec;
>   int prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO;
>  
> - region = iommu_alloc_resv_region(MSI_IOVA_BASE, MSI_IOVA_LENGTH,
> -  prot, IOMMU_RESV_SW_MSI);
> - if (!region)
> - return;
> + smmu = arm_smmu_get_by_fwnode(fwspec->iommu_fwnode);
> +
> + if (smmu && (smmu->options & ARM_SMMU_OPT_RESV_HW_MSI) &&
> +   dev_is_pci(dev)) {

IORT changes are fine to me, I am still no big fan of this supposedly
generic option that is _really_ platform specific (in particular as I
said before the quirk depends on the PCI host bridge but in this
patchset I see no such dependency. In short - the quirk is hooked off
the SMMUv3 model which implicitly implies a PCI host bridge
configuration IIUC). It is Will and Robin decision though, I am not sure
you can make it any tidier (given that on ACPI you may not even have
a PCI host bridge specific _HID to base your check above on).

Thanks,
Lorenzo

> + int ret = -EINVAL;
> +
> + if (!is_of_node(smmu->dev->fwnode))
> + ret = iort_iommu_its_get_resv_regions(dev, head);
>  
> - list_add_tail(>list, head);
> + if (ret) {
> + dev_warn(dev, "HW MSI region resv failed: %d\n", ret);
> + return;
> + }
> + } else {
> + region = iommu_alloc_resv_region(MSI_IOVA_BASE, MSI_IOVA_LENGTH,
> +  prot, IOMMU_RESV_SW_MSI);
> + if (!region)
> + return;
> +
> + list_add_tail(>list, head);
> + }
>  
>   iommu_dma_get_resv_regions(dev, head);
>  }
> @@ -2611,6 +2629,7 @@ static void parse_driver_acpi_options(struct 
> acpi_iort_smmu_v3 *iort_smmu,
>   switch (iort_smmu->model) {
>   case ACPI_IORT_SMMU_HISILICON_HI161X:
>   smmu->options |= ARM_SMMU_OPT_SKIP_PREFETCH;
> + smmu->options |= ARM_SMMU_OPT_RESV_HW_MSI;
>   break;
>   default:
>   break;
> -- 
> 1.9.1
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v7 1/3] ACPI/IORT: Fixup SMMUv3 resource size for Cavium ThunderX2 SMMUv3 model

[Robert Richter]

On 20.06.17 10:19:43, Robert Richter wrote:
> On 30.05.17 17:33:39, Geetha sowjanya wrote:
> > From: Linu Cherian 

> > +   /*
> > +* Override the size, for Cavium ThunderX2 implementation
> > +* which doesn't support the page 1 SMMU register space.
> > +*/
> > +   if (smmu->model == ACPI_IORT_SMMU_CAVIUM_CN99XX)
> 
> Geetha,
> 
> please resubmit the series since the macro changed to
> ACPI_IORT_SMMU_V3_CAVIUM_CN99XX:
> 
>  
> https://github.com/acpica/acpica/commit/d00a4eb86e64bb4fa70f57ab5e5ca0a4ca2ad8ef#diff-a572aac2ccc26fe4a901616d7fdba859R1053

Rafael, Bob,

btw, I haven't seen

 https://github.com/acpica/acpica/commit/d00a4eb86e64

yet in linux-pm:linux-next. We would like to see this in 4.13 also. Is
it on the way already?

Thanks,

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

Re: [PATCH v7 11/36] x86/mm: Add SME support for read_cr3_pa()

[Borislav Petkov]

On Fri, Jun 16, 2017 at 01:51:55PM -0500, Tom Lendacky wrote:
> The cr3 register entry can contain the SME encryption mask that indicates
> the PGD is encrypted.  The encryption mask should not be used when
> creating a virtual address from the cr3 register, so remove the SME
> encryption mask in the read_cr3_pa() function.
> 
> During early boot SME will need to use a native version of read_cr3_pa(),
> so create native_read_cr3_pa().
> 
> Signed-off-by: Tom Lendacky 
> ---
>  arch/x86/include/asm/processor-flags.h |3 ++-
>  arch/x86/include/asm/processor.h   |5 +
>  2 files changed, 7 insertions(+), 1 deletion(-)

Reviewed-by: Borislav Petkov 

-- 
Regards/Gruss,
Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v7 1/3] ACPI/IORT: Fixup SMMUv3 resource size for Cavium ThunderX2 SMMUv3 model

[Robert Richter]

On 30.05.17 17:33:39, Geetha sowjanya wrote:
> From: Linu Cherian 
> 
> Cavium ThunderX2 implementation doesn't support second page in SMMU
> register space. Hence, resource size is set as 64k for this model.
> 
> Signed-off-by: Linu Cherian 
> Signed-off-by: Geetha Sowjanya 
> ---
>  drivers/acpi/arm64/iort.c |   10 +-
>  1 files changed, 9 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> index c5fecf9..bba2b59 100644
> --- a/drivers/acpi/arm64/iort.c
> +++ b/drivers/acpi/arm64/iort.c
> @@ -833,12 +833,20 @@ static void __init arm_smmu_v3_init_resources(struct 
> resource *res,
>  {
>   struct acpi_iort_smmu_v3 *smmu;
>   int num_res = 0;
> + unsigned long size = SZ_128K;
>  
>   /* Retrieve SMMUv3 specific data */
>   smmu = (struct acpi_iort_smmu_v3 *)node->node_data;
>  
> + /*
> +  * Override the size, for Cavium ThunderX2 implementation
> +  * which doesn't support the page 1 SMMU register space.
> +  */
> + if (smmu->model == ACPI_IORT_SMMU_CAVIUM_CN99XX)

Geetha,

please resubmit the series since the macro changed to
ACPI_IORT_SMMU_V3_CAVIUM_CN99XX:

 
https://github.com/acpica/acpica/commit/d00a4eb86e64bb4fa70f57ab5e5ca0a4ca2ad8ef#diff-a572aac2ccc26fe4a901616d7fdba859R1053

-Robert

> + size = SZ_64K;
> +
>   res[num_res].start = smmu->base_address;
> - res[num_res].end = smmu->base_address + SZ_128K - 1;
> + res[num_res].end = smmu->base_address + size - 1;
>   res[num_res].flags = IORESOURCE_MEM;
>  
>   num_res++;
> -- 
> 1.7.1
> 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v7 08/36] x86/mm: Add support to enable SME in early boot processing

[Borislav Petkov]

On Fri, Jun 16, 2017 at 01:51:15PM -0500, Tom Lendacky wrote:
> Add support to the early boot code to use Secure Memory Encryption (SME).
> Since the kernel has been loaded into memory in a decrypted state, encrypt
> the kernel in place and update the early pagetables with the memory
> encryption mask so that new pagetable entries will use memory encryption.
> 
> The routines to set the encryption mask and perform the encryption are
> stub routines for now with functionality to be added in a later patch.
> 
> Because of the need to have the routines available to head_64.S, the
> mem_encrypt.c is always built and #ifdefs in mem_encrypt.c will provide
> functionality or stub routines depending on CONFIG_AMD_MEM_ENCRYPT.
> 
> Signed-off-by: Tom Lendacky 
> ---
>  arch/x86/include/asm/mem_encrypt.h |8 +++
>  arch/x86/kernel/head64.c   |   33 +-
>  arch/x86/kernel/head_64.S  |   39 
> ++--
>  arch/x86/mm/Makefile   |4 +---
>  arch/x86/mm/mem_encrypt.c  |   24 ++
>  5 files changed, 93 insertions(+), 15 deletions(-)

...

> diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
> index b99d469..9a78277 100644
> --- a/arch/x86/mm/mem_encrypt.c
> +++ b/arch/x86/mm/mem_encrypt.c
> @@ -11,6 +11,9 @@
>   */
>  
>  #include 
> +#include 
> +
> +#ifdef CONFIG_AMD_MEM_ENCRYPT
>  
>  /*
>   * Since SME related variables are set early in the boot process they must
> @@ -19,3 +22,24 @@
>   */
>  unsigned long sme_me_mask __section(.data) = 0;
>  EXPORT_SYMBOL_GPL(sme_me_mask);
> +
> +void __init sme_encrypt_kernel(void)
> +{
> +}

Just the minor:

void __init sme_encrypt_kernel(void) { }

in case you have to respin.

Reviewed-by: Borislav Petkov 

-- 
Regards/Gruss,
Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v7 03/36] x86, mpparse, x86/acpi, x86/PCI, x86/dmi, SFI: Use memremap for RAM mappings

[Borislav Petkov]

On Fri, Jun 16, 2017 at 01:50:23PM -0500, Tom Lendacky wrote:
> The ioremap() function is intended for mapping MMIO. For RAM, the
> memremap() function should be used. Convert calls from ioremap() to
> memremap() when re-mapping RAM.
> 
> This will be used later by SME to control how the encryption mask is
> applied to memory mappings, with certain memory locations being mapped
> decrypted vs encrypted.
> 
> Signed-off-by: Tom Lendacky 
> ---
>  arch/x86/include/asm/dmi.h   |8 
>  arch/x86/kernel/acpi/boot.c  |6 +++---
>  arch/x86/kernel/kdebugfs.c   |   34 +++---
>  arch/x86/kernel/ksysfs.c |   28 ++--
>  arch/x86/kernel/mpparse.c|   10 +-
>  arch/x86/pci/common.c|4 ++--
>  drivers/firmware/dmi-sysfs.c |5 +++--
>  drivers/firmware/pcdp.c  |4 ++--
>  drivers/sfi/sfi_core.c   |   22 +++---
>  9 files changed, 55 insertions(+), 66 deletions(-)

Reviewed-by: Borislav Petkov 

-- 
Regards/Gruss,
Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu