Re: [RFC PATCH v2 07/20] x86: Provide general kernel support for memory encryption

2016-09-07 Thread Borislav Petkov 
On Wed, Sep 07, 2016 at 09:30:54AM -0500, Tom Lendacky wrote:
> _PAGE_ENC is #defined as sme_me_mask and sme_me_mask has already been
> set (or not set) at this point - so it will be the mask if SME is
> active or 0 if SME is not active.

Yeah, I remember :-)

> sme_early_init() is merely propagating the mask to other structures.
> Since early_pmd_flags is mainly used in this file (one line in
> head_64.S is the other place) I felt it best to modify it here. But it
> can always be moved if you feel that is best.

Hmm, so would it work then if you stick it in early_pmd_flags'
definition like you do with the other masks? I.e.,

pmdval_t early_pmd_flags = __PAGE_KERNEL_LARGE | _PAGE_ENC & ~(_PAGE_GLOBAL | 
_PAGE_NX);

-- 
Regards/Gruss,
Boris.

ECO tip #101: Trim your mails when you reply.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [RFC PATCH v2 01/20] x86: Documentation for AMD Secure Memory Encryption (SME)

2016-09-07 Thread Borislav Petkov 
On Wed, Sep 07, 2016 at 09:02:38AM -0500, Tom Lendacky wrote:
> Ugh..  I thought I caught all of these.  Obviously not.  I'll go through
> all the patches on this.

What you could do is run all patches through scripts/checkpatch.pl
and fix those issues which make sense to you. What I'm saying is, you
shouldn't take its output to the letter but some of the stuff it catches
are valid.

Thanks.

-- 
Regards/Gruss,
Boris.

ECO tip #101: Trim your mails when you reply.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [RFC PATCH v2 07/20] x86: Provide general kernel support for memory encryption

2016-09-07 Thread Tom Lendacky 
On 09/06/2016 04:31 AM, Borislav Petkov wrote:
> On Mon, Aug 22, 2016 at 05:36:46PM -0500, Tom Lendacky wrote:
>> Adding general kernel support for memory encryption includes:
>> - Modify and create some page table macros to include the Secure Memory
>>   Encryption (SME) memory encryption mask
>> - Update kernel boot support to call an SME routine that checks for and
>>   sets the SME capability (the SME routine will grow later and for now
>>   is just a stub routine)
>> - Update kernel boot support to call an SME routine that encrypts the
>>   kernel (the SME routine will grow later and for now is just a stub
>>   routine)
>> - Provide an SME initialization routine to update the protection map with
>>   the memory encryption mask so that it is used by default
>>
>> Signed-off-by: Tom Lendacky 
> 
> ...
> 
>> diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
>> index 54a2372..88c7bae 100644
>> --- a/arch/x86/kernel/head64.c
>> +++ b/arch/x86/kernel/head64.c
>> @@ -28,6 +28,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>  
>>  /*
>>   * Manage page tables very early on.
>> @@ -42,7 +43,7 @@ static void __init reset_early_page_tables(void)
>>  {
>>  memset(early_level4_pgt, 0, sizeof(pgd_t)*(PTRS_PER_PGD-1));
>>  next_early_pgt = 0;
>> -write_cr3(__pa_nodebug(early_level4_pgt));
>> +write_cr3(__sme_pa_nodebug(early_level4_pgt));
>>  }
>>  
>>  /* Create a new PMD entry */
>> @@ -54,7 +55,7 @@ int __init early_make_pgtable(unsigned long address)
>>  pmdval_t pmd, *pmd_p;
>>  
>>  /* Invalid address or early pgt is done ?  */
>> -if (physaddr >= MAXMEM || read_cr3() != __pa_nodebug(early_level4_pgt))
>> +if (physaddr >= MAXMEM || read_cr3() != 
>> __sme_pa_nodebug(early_level4_pgt))
>>  return -1;
>>  
>>  again:
>> @@ -157,6 +158,11 @@ asmlinkage __visible void __init 
>> x86_64_start_kernel(char * real_mode_data)
>>  
>>  clear_page(init_level4_pgt);
>>  
>> +/* Update the early_pmd_flags with the memory encryption mask */
>> +early_pmd_flags |= _PAGE_ENC;
>> +
>> +sme_early_init();
>> +
> 
> So maybe this comes later but you're setting _PAGE_ENC unconditionally
> *before* sme_early_init().
> 
> I think you should set it in sme_early_init() and iff SME is enabled.

_PAGE_ENC is #defined as sme_me_mask and sme_me_mask has already been
set (or not set) at this point - so it will be the mask if SME is
active or 0 if SME is not active.  sme_early_init() is merely
propagating the mask to other structures.  Since early_pmd_flags is
mainly used in this file (one line in head_64.S is the other place) I
felt it best to modify it here.  But it can always be moved if you feel
that is best.

Thanks,
Tom

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

Re: [RFC PATCH v2 07/20] x86: Provide general kernel support for memory encryption

2016-09-07 Thread Tom Lendacky 
On 09/05/2016 10:22 AM, Borislav Petkov wrote:
> On Mon, Aug 22, 2016 at 05:36:46PM -0500, Tom Lendacky wrote:
>> Adding general kernel support for memory encryption includes:
>> - Modify and create some page table macros to include the Secure Memory
>>   Encryption (SME) memory encryption mask
>> - Update kernel boot support to call an SME routine that checks for and
>>   sets the SME capability (the SME routine will grow later and for now
>>   is just a stub routine)
>> - Update kernel boot support to call an SME routine that encrypts the
>>   kernel (the SME routine will grow later and for now is just a stub
>>   routine)
>> - Provide an SME initialization routine to update the protection map with
>>   the memory encryption mask so that it is used by default
>>
>> Signed-off-by: Tom Lendacky 
> 
> ...
> 
>> diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
>> index c98a559..30f7715 100644
>> --- a/arch/x86/kernel/head_64.S
>> +++ b/arch/x86/kernel/head_64.S
>> @@ -95,6 +95,13 @@ startup_64:
>>  jnz bad_address
>>  
>>  /*
>> + * Enable memory encryption (if available). Add the memory encryption
>> + * mask to %rbp to include it in the the page table fixup.
>> + */
>> +callsme_enable
>> +addqsme_me_mask(%rip), %rbp
>> +
>> +/*
>>   * Fixup the physical addresses in the page table
>>   */
>>  addq%rbp, early_level4_pgt + (L4_START_KERNEL*8)(%rip)
>> @@ -116,7 +123,8 @@ startup_64:
>>  movq%rdi, %rax
>>  shrq$PGDIR_SHIFT, %rax
>>  
>> -leaq(4096 + _KERNPG_TABLE)(%rbx), %rdx
>> +leaq(4096 + __KERNPG_TABLE)(%rbx), %rdx
>> +addqsme_me_mask(%rip), %rdx /* Apply mem encryption mask */
> 
> Please add comments over the line and not at the side...

Ok, will do.

Thanks,
Tom

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

Re: [RFC PATCH v2 07/20] x86: Provide general kernel support for memory encryption

2016-09-07 Thread Tom Lendacky 
On 09/05/2016 03:48 AM, Borislav Petkov wrote:
> On Mon, Aug 22, 2016 at 05:36:46PM -0500, Tom Lendacky wrote:
>> Adding general kernel support for memory encryption includes:
>> - Modify and create some page table macros to include the Secure Memory
>>   Encryption (SME) memory encryption mask
>> - Update kernel boot support to call an SME routine that checks for and
>>   sets the SME capability (the SME routine will grow later and for now
>>   is just a stub routine)
>> - Update kernel boot support to call an SME routine that encrypts the
>>   kernel (the SME routine will grow later and for now is just a stub
>>   routine)
>> - Provide an SME initialization routine to update the protection map with
>>   the memory encryption mask so that it is used by default
>>
>> Signed-off-by: Tom Lendacky 
>> ---
> 
> ...
> 
>> diff --git a/arch/x86/include/asm/pgtable_types.h 
>> b/arch/x86/include/asm/pgtable_types.h
>> index f1218f5..a01f0e1 100644
>> --- a/arch/x86/include/asm/pgtable_types.h
>> +++ b/arch/x86/include/asm/pgtable_types.h
>> @@ -3,6 +3,7 @@
>>  
>>  #include 
>>  #include 
>> +#include 
>>  
>>  #define FIRST_USER_ADDRESS  0UL
>>  
>> @@ -121,9 +122,9 @@
>>  
>>  #define _PAGE_PROTNONE  (_AT(pteval_t, 1) << _PAGE_BIT_PROTNONE)
>>  
>> -#define _PAGE_TABLE (_PAGE_PRESENT | _PAGE_RW | _PAGE_USER |\
>> +#define __PAGE_TABLE(_PAGE_PRESENT | _PAGE_RW | _PAGE_USER |
>> \
>>   _PAGE_ACCESSED | _PAGE_DIRTY)
> 
> Hmm, so this naming looks confusing and error-prone: the only difference
> is a single "_".
> 
> How about this instead:
> 
> #define _PAGE_TABLE_NO_ENC(_PAGE_PRESENT | _PAGE_RW | _PAGE_USER |
> \
>_PAGE_ACCESSED | _PAGE_DIRTY)
> 
> #define _PAGE_TABLE (_PAGE_TABLE_NO_ENC | _PAGE_ENC)
> 
> Or call it _PAGE_TABLE_BASE or whatever.
> 
> Ditto for __KERNPG_TABLE.
> 
> This way you can differentiate between the two and use the _NO_ENC one
> to define _PAGE_TABLE. And it will be absolutely clear when you use the
> _NO_ENC one, what you mean and that you don't want to have the enc mask
> in the PTE.
> 
> Should be less confusing IMO too.

Yup, makes sense.  I'll rework/rename.

Thanks,
Tom

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

Re: [RFC PATCH v2 07/20] x86: Provide general kernel support for memory encryption

2016-09-07 Thread Tom Lendacky 
On 09/02/2016 01:14 PM, Borislav Petkov wrote:
> On Mon, Aug 22, 2016 at 05:36:46PM -0500, Tom Lendacky wrote:
>> Adding general kernel support for memory encryption includes:
>> - Modify and create some page table macros to include the Secure Memory
>>   Encryption (SME) memory encryption mask
>> - Update kernel boot support to call an SME routine that checks for and
>>   sets the SME capability (the SME routine will grow later and for now
>>   is just a stub routine)
>> - Update kernel boot support to call an SME routine that encrypts the
>>   kernel (the SME routine will grow later and for now is just a stub
>>   routine)
>> - Provide an SME initialization routine to update the protection map with
>>   the memory encryption mask so that it is used by default
>>
>> Signed-off-by: Tom Lendacky 
>> ---
> 
> ...
> 
>> diff --git a/arch/x86/include/asm/mem_encrypt.h 
>> b/arch/x86/include/asm/mem_encrypt.h
>> index 747fc52..9f3e762 100644
>> --- a/arch/x86/include/asm/mem_encrypt.h
>> +++ b/arch/x86/include/asm/mem_encrypt.h
>> @@ -15,12 +15,21 @@
>>  
>>  #ifndef __ASSEMBLY__
>>  
>> +#include 
>> +
>>  #ifdef CONFIG_AMD_MEM_ENCRYPT
>>  
>>  extern unsigned long sme_me_mask;
>>  
>>  u8 sme_get_me_loss(void);
>>  
>> +void __init sme_early_init(void);
>> +
>> +#define __sme_pa(x) (__pa((x)) | sme_me_mask)
>> +#define __sme_pa_nodebug(x) (__pa_nodebug((x)) | sme_me_mask)
>> +
>> +#define __sme_va(x) (__va((x) & ~sme_me_mask))
> 
> So I'm wondering: why not push the masking off of the SME mask into the
> __va() macro instead of defining a specific __sme_va() one?
> 
> I mean, do you even see cases where __va() would need to have to
> sme_mask left in the virtual address?
> 
> Because if not, you could mask it out in __va() so that all __va() users
> get the "clean" va, without the enc bits.

That's a good point, yes, it could go in __va().  I'll make that change.

> 
> Hmmm.
> 
> Btw, this patch is huuuge. It would be nice if you could split it, if
> possible...

Ok, I'll look at how to make this a bit more manageable.

Thanks,
Tom

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

Re: [RFC PATCH v2 05/20] x86: Add the Secure Memory Encryption cpu feature

2016-09-07 Thread Tom Lendacky 
On 09/02/2016 09:09 AM, Borislav Petkov wrote:
> On Mon, Aug 22, 2016 at 05:36:22PM -0500, Tom Lendacky wrote:
>> Update the cpu features to include identifying and reporting on the
>> Secure Memory Encryption feature.
>>
>> Signed-off-by: Tom Lendacky 
>> ---
>>  arch/x86/include/asm/cpufeature.h|7 +--
>>  arch/x86/include/asm/cpufeatures.h   |5 -
>>  arch/x86/include/asm/disabled-features.h |3 ++-
>>  arch/x86/include/asm/required-features.h |3 ++-
>>  arch/x86/kernel/cpu/scattered.c  |1 +
>>  5 files changed, 14 insertions(+), 5 deletions(-)
> 
> ...
> 
>> diff --git a/arch/x86/kernel/cpu/scattered.c 
>> b/arch/x86/kernel/cpu/scattered.c
>> index 8cb57df..d86d9a5 100644
>> --- a/arch/x86/kernel/cpu/scattered.c
>> +++ b/arch/x86/kernel/cpu/scattered.c
>> @@ -37,6 +37,7 @@ void init_scattered_cpuid_features(struct cpuinfo_x86 *c)
>>  { X86_FEATURE_HW_PSTATE,CR_EDX, 7, 0x8007, 0 },
>>  { X86_FEATURE_CPB,  CR_EDX, 9, 0x8007, 0 },
>>  { X86_FEATURE_PROC_FEEDBACK,CR_EDX,11, 0x8007, 0 },
>> +{ X86_FEATURE_SME,  CR_EAX, 0, 0x801f, 0 },
> 
> If this is in scattered CPUID features, it doesn't need any of the
> (snipped) changes above - you solely need to reuse one of the free
> defines, i.e., something like this:

Ok, that's much easier. I'll do that.

Thanks,
Tom

> 
> ---
> --- a/arch/x86/include/asm/cpufeatures.h  2016-09-02 15:49:08.853374323 
> +0200
> +++ b/arch/x86/include/asm/cpufeatures.h  2016-09-02 15:52:34.477365610 
> +0200
> @@ -100,7 +100,7 @@
>  #define X86_FEATURE_XTOPOLOGY( 3*32+22) /* cpu topology enum 
> extensions */
>  #define X86_FEATURE_TSC_RELIABLE ( 3*32+23) /* TSC is known to be reliable */
>  #define X86_FEATURE_NONSTOP_TSC  ( 3*32+24) /* TSC does not stop in C 
> states */
> -/* free, was #define X86_FEATURE_CLFLUSH_MONITOR ( 3*32+25) * "" clflush 
> reqd with monitor */
> +#define X86_FEATURE_SME  ( 3*32+25) /* Secure Memory Encryption 
> */
>  #define X86_FEATURE_EXTD_APICID  ( 3*32+26) /* has extended APICID (8 
> bits) */
>  #define X86_FEATURE_AMD_DCM ( 3*32+27) /* multi-node processor */
>  #define X86_FEATURE_APERFMPERF   ( 3*32+28) /* APERFMPERF */
> --- a/arch/x86/kernel/cpu/scattered.c 2016-09-02 15:48:52.753375005 +0200
> +++ b/arch/x86/kernel/cpu/scattered.c 2016-09-02 15:51:32.437368239 +0200
> @@ -37,6 +37,7 @@ void init_scattered_cpuid_features(struc
>   { X86_FEATURE_HW_PSTATE,CR_EDX, 7, 0x8007, 0 },
>   { X86_FEATURE_CPB,  CR_EDX, 9, 0x8007, 0 },
>   { X86_FEATURE_PROC_FEEDBACK,CR_EDX,11, 0x8007, 0 },
> + { X86_FEATURE_SME,  CR_EAX, 0, 0x801f, 0 },
>   { 0, 0, 0, 0, 0 }
>   };
> 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [RFC PATCH v2 03/20] x86: Secure Memory Encryption (SME) build enablement

2016-09-07 Thread Tom Lendacky 
On 09/02/2016 06:03 AM, Borislav Petkov wrote:
> On Mon, Aug 22, 2016 at 05:35:59PM -0500, Tom Lendacky wrote:
>> Provide the Kconfig support to build the SME support in the kernel.
>>
>> Signed-off-by: Tom Lendacky 
>> ---
>>  arch/x86/Kconfig |9 +
>>  1 file changed, 9 insertions(+)
>>
>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>> index c580d8c..131f329 100644
>> --- a/arch/x86/Kconfig
>> +++ b/arch/x86/Kconfig
>> @@ -1357,6 +1357,15 @@ config X86_DIRECT_GBPAGES
>>supports them), so don't confuse the user by printing
>>that we have them enabled.
>>  
>> +config AMD_MEM_ENCRYPT
>> +bool "Secure Memory Encryption support for AMD"
> 
>"AMD Secure Memory Encryption support"

Ok.

Thanks,
Tom

> 
>> +depends on X86_64 && CPU_SUP_AMD
>> +---help---
>> +  Say yes to enable the encryption of system memory. This requires
>> +  an AMD processor that supports Secure Memory Encryption (SME).
>> +  The encryption of system memory is disabled by default but can be
>> +  enabled with the mem_encrypt=on command line option.
>> +
>>  # Common NUMA Features
>>  config NUMA
>>  bool "Numa Memory Allocation and Scheduler Support"
> 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [RFC PATCH v2 01/20] x86: Documentation for AMD Secure Memory Encryption (SME)

2016-09-07 Thread Tom Lendacky 
On 09/02/2016 03:50 AM, Borislav Petkov wrote:
> On Mon, Aug 22, 2016 at 05:35:39PM -0500, Tom Lendacky wrote:
>> This patch adds a Documenation entry to decribe the AMD Secure Memory
>> Encryption (SME) feature.
>>
>> Signed-off-by: Tom Lendacky 
>> ---
>>  Documentation/x86/amd-memory-encryption.txt |   35 
>> +++
>>  1 file changed, 35 insertions(+)
>>  create mode 100644 Documentation/x86/amd-memory-encryption.txt
>>
>> diff --git a/Documentation/x86/amd-memory-encryption.txt 
>> b/Documentation/x86/amd-memory-encryption.txt
>> new file mode 100644
>> index 000..f19c555
>> --- /dev/null
>> +++ b/Documentation/x86/amd-memory-encryption.txt
>> @@ -0,0 +1,35 @@
>> +Secure Memory Encryption (SME) is a feature found on AMD processors.
>> +
>> +SME provides the ability to mark individual pages of memory as encrypted 
>> using
>> +the standard x86 page tables.  A page that is marked encrpyted will be
> 
> s/encrpyted/encrypted/

Ugh..  I thought I caught all of these.  Obviously not.  I'll go through
all the patches on this.

> 
>> +automatically decrypted when read from DRAM and encrypted when written to
>> +DRAM.  SME can therefore be used to protect the contents of DRAM from 
>> physical
>> +attacks on the system.
>> +
>> +Support for SME can be determined through the CPUID instruction. The CPUID
>> +function 0x801f reports information related to SME:
>> +
>> +0x801f[eax]:
>> +Bit[0] indicates support for SME
>> +0x801f[ebx]:
>> +Bit[5:0]  pagetable bit number used to enable memory encryption
>> +Bit[11:6] reduction in physical address space, in bits, when
>> +  memory encryption is enabled (this only affects system
>> +  physical addresses, not guest physical addresses)
>> +
>> +If support for SME is present, MSR 0xc00100010 (SYS_CFG) can be used to
>> +determine if SME is enabled and/or to enable memory encryption:
>> +
>> +0xc0010010:
>> +Bit[23]   0 = memory encryption features are disabled
>> +  1 = memory encryption features are enabled
>> +
>> +Linux relies on BIOS to set this bit if BIOS has determined that the 
>> reduction
>> +in the physical address space as a result of enabling memory encryption (see
>> +CPUID information above) will not conflict with the address space resource
>> +requirements for the system.  If this bit is not set upon Linux startup then
>> +Linux itself will not set it and memory encryption will not be possible.
>> +
>> +SME support is configurable in the kernel through the AMD_MEM_ENCRYPT config
>> +option.
> 
> " ... is configurable through CONFIG_AMD_MEM_ENCRYPT."

Ok.

> 
>> Additionally, the mem_encrypt=on command line parameter is required
>> +to activate memory encryption.
> 
> I think you want to rewrite the logic here to say that people should use
> the BIOS option and if none is present for whatever reason, resort to
> the alternative "mem_encrypt=on" kernel command line option, no?

Yes, I'll work on rewording this section.

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

[PATCH v6.1] iommu/dma: Add support for mapping MSIs

2016-09-07 Thread Robin Murphy 
When an MSI doorbell is located downstream of an IOMMU, attaching
devices to a DMA ops domain and switching on translation leads to a rude
shock when their attempt to write to the physical address returned by
the irqchip driver faults (or worse, writes into some already-mapped
buffer) and no interrupt is forthcoming.

Address this by adding a hook for relevant irqchip drivers to call from
their compose_msi_msg() callback, to swizzle the physical address with
an appropriatly-mapped IOVA for any device attached to one of our DMA
ops domains.

CC: Thomas Gleixner 
CC: Jason Cooper 
CC: Marc Zyngier 
CC: linux-ker...@vger.kernel.org
Signed-off-by: Robin Murphy 

---

- Rework map_page() helper function plus insane lookup logic into
  straightforward get_page() helper
- Use phys_addr_t to further simplify address matching
- Fix the bit where I neglected to actually round the doorbell
  address to a page boundary (oops!)
- Make the locking hardirq-safe to satisfy lockdep
---
 drivers/iommu/dma-iommu.c| 136 ++-
 drivers/irqchip/irq-gic-v2m.c|   3 +
 drivers/irqchip/irq-gic-v3-its.c |   3 +
 include/linux/dma-iommu.h|   9 +++
 4 files changed, 136 insertions(+), 15 deletions(-)

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 00c8a08d56e7..4329d18080cf 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -25,10 +25,28 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
 
+struct iommu_dma_msi_page {
+   struct list_headlist;
+   dma_addr_t  iova;
+   phys_addr_t phys;
+};
+
+struct iommu_dma_cookie {
+   struct iova_domain  iovad;
+   struct list_headmsi_page_list;
+   spinlock_t  msi_lock;
+};
+
+static inline struct iova_domain *cookie_iovad(struct iommu_domain *domain)
+{
+   return &((struct iommu_dma_cookie *)domain->iova_cookie)->iovad;
+}
+
 int iommu_dma_init(void)
 {
return iova_cache_get();
@@ -43,15 +61,19 @@ int iommu_dma_init(void)
  */
 int iommu_get_dma_cookie(struct iommu_domain *domain)
 {
-   struct iova_domain *iovad;
+   struct iommu_dma_cookie *cookie;
 
if (domain->iova_cookie)
return -EEXIST;
 
-   iovad = kzalloc(sizeof(*iovad), GFP_KERNEL);
-   domain->iova_cookie = iovad;
+   cookie = kzalloc(sizeof(*cookie), GFP_KERNEL);
+   if (!cookie)
+   return -ENOMEM;
 
-   return iovad ? 0 : -ENOMEM;
+   spin_lock_init(>msi_lock);
+   INIT_LIST_HEAD(>msi_page_list);
+   domain->iova_cookie = cookie;
+   return 0;
 }
 EXPORT_SYMBOL(iommu_get_dma_cookie);
 
@@ -63,14 +85,20 @@ EXPORT_SYMBOL(iommu_get_dma_cookie);
  */
 void iommu_put_dma_cookie(struct iommu_domain *domain)
 {
-   struct iova_domain *iovad = domain->iova_cookie;
+   struct iommu_dma_cookie *cookie = domain->iova_cookie;
+   struct iommu_dma_msi_page *msi, *tmp;
 
-   if (!iovad)
+   if (!cookie)
return;
 
-   if (iovad->granule)
-   put_iova_domain(iovad);
-   kfree(iovad);
+   if (cookie->iovad.granule)
+   put_iova_domain(>iovad);
+
+   list_for_each_entry_safe(msi, tmp, >msi_page_list, list) {
+   list_del(>list);
+   kfree(msi);
+   }
+   kfree(cookie);
domain->iova_cookie = NULL;
 }
 EXPORT_SYMBOL(iommu_put_dma_cookie);
@@ -88,7 +116,7 @@ EXPORT_SYMBOL(iommu_put_dma_cookie);
  */
 int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base, u64 
size)
 {
-   struct iova_domain *iovad = domain->iova_cookie;
+   struct iova_domain *iovad = cookie_iovad(domain);
unsigned long order, base_pfn, end_pfn;
 
if (!iovad)
@@ -155,7 +183,7 @@ int dma_direction_to_prot(enum dma_data_direction dir, bool 
coherent)
 static struct iova *__alloc_iova(struct iommu_domain *domain, size_t size,
dma_addr_t dma_limit)
 {
-   struct iova_domain *iovad = domain->iova_cookie;
+   struct iova_domain *iovad = cookie_iovad(domain);
unsigned long shift = iova_shift(iovad);
unsigned long length = iova_align(iovad, size) >> shift;
 
@@ -171,7 +199,7 @@ static struct iova *__alloc_iova(struct iommu_domain 
*domain, size_t size,
 /* The IOVA allocator knows what we mapped, so just unmap whatever that was */
 static void __iommu_dma_unmap(struct iommu_domain *domain, dma_addr_t dma_addr)
 {
-   struct iova_domain *iovad = domain->iova_cookie;
+   struct iova_domain *iovad = cookie_iovad(domain);
unsigned long shift = iova_shift(iovad);
unsigned long pfn = dma_addr >> shift;
struct iova *iova = find_iova(iovad, pfn);
@@ -294,7 +322,7 @@ struct page **iommu_dma_alloc(struct device *dev, size_t 
size, gfp_t gfp,
void (*flush_page)(struct device *,

RE: [PATCH 7/8] iommu: of: Handle IOMMU lookup failure with deferred probing or error

2016-09-07 Thread Sricharan 
Hi Marek,

>Hi Sricharan,
>
>On 2016-08-09 00:49, Sricharan R wrote:
> > From: Laurent Pinchart 
> >
> > Failures to look up an IOMMU when parsing the DT iommus property need to
> > be handled separately from the .of_xlate() failures to support deferred
> > probing.
> >
> > The lack of a registered IOMMU can be caused by the lack of a driver for
> > the IOMMU, the IOMMU device probe not having been performed yet, having
> > been deferred, or having failed.
> >
> > The first case occurs when the device tree describes the bus master and
> > IOMMU topology correctly but no device driver exists for the IOMMU yet
> > or the device driver has not been compiled in. Return NULL, the caller
> > will configure the device without an IOMMU.
> >
> > The second and third cases are handled by deferring the probe of the bus
> > master device which will eventually get reprobed after the IOMMU.
> >
> > The last case is currently handled by deferring the probe of the bus
> > master device as well. A mechanism to either configure the bus master
> > device without an IOMMU or to fail the bus master device probe depending
> > on whether the IOMMU is optional or mandatory would be a good
> > enhancement.
> >
> > Signed-off-by: Laurent Pinchart
>
>
>It is a common practice to briefly describe here what has been changed
>since the original patch if you have modified it (see commit
>855ed04a3758b205e84b269f92d26ab36ed8e2f7 for the example).
>
 Sorry i missed updating it. i will update the log.

> > Signed-off-by: Sricharan R 
> > ---
> >  drivers/iommu/of_iommu.c | 21 +
> >  drivers/of/device.c  |  2 ++
> >  2 files changed, 19 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> > index 4c4219d..3994cf5 100644
> > --- a/drivers/iommu/of_iommu.c
> > +++ b/drivers/iommu/of_iommu.c
> > @@ -149,7 +149,7 @@ const struct iommu_ops *of_iommu_configure(struct
>device *dev,
> >  {
> >  struct of_phandle_args iommu_spec;
> >  struct device_node *np = NULL;
> > -struct iommu_ops *ops = NULL;
> > +const struct iommu_ops *ops = NULL;
> >  int idx = 0;
> >
> >  if (dev_is_pci(dev)) {
> > @@ -189,8 +189,21 @@ const struct iommu_ops
>*of_iommu_configure(struct device *dev,
> >  np = iommu_spec.np;
> >  ops = of_iommu_get_ops(np);
> >
> > -if (!ops || !ops->of_xlate || ops->of_xlate(dev, _spec))
> > +if (!ops) {
> > +const struct of_device_id *oid;
> > +
> > +oid = of_match_node(&__iommu_of_table, np);
> > +ops = oid ? ERR_PTR(-EPROBE_DEFER) : NULL;
> >  goto err_put_node;
> > +}
> > +
> > +if (!ops->of_xlate || ops->of_xlate(dev, _spec)) {
> > +ops = NULL;
> > +goto err_put_node;
> > +}
> > +
> > +if (ops->add_device)
> > +ops = ops->add_device(dev) ? ops : NULL;
>
>ops->add_device() returns ZERO on success or error code on failure, so
>the above
>line should be changed to:
> ops = (ops->add_device(dev) == 0) ? ops : NULL;

 Yes, i fixed it while testing, but somehow have missed it while sending it.
  will fix it.

Regards,
 Sricharan


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

RE: [PATCH 1/8] arm: dma-mapping: Don't override dma_ops in arch_setup_dma_ops()

2016-09-07 Thread Sricharan 
Hi Marek,

>Hi Sricharan and Laurent,
>
>
>On 2016-08-09 00:49, Sricharan R wrote:
>> From: Laurent Pinchart 
>>
>> The arch_setup_dma_ops() function is in charge of setting dma_ops with a
>> call to set_dma_ops(). set_dma_ops() is also called from
>>
>> - highbank and mvebu bus notifiers
>> - dmabounce (to be replaced with swiotlb)
>> - arm_iommu_attach_device
>>
>> (arm_iommu_attach_device is itself called from IOMMU and bus master
>> device drivers)
>>
>> To allow the arch_setup_dma_ops() call to be moved from device add time
>> to device probe time we must ensure that dma_ops already setup by any of
>> the above callers will not be overridden.
>>
>> Aftering replacing dmabounce with swiotlb, converting IOMMU drivers to
>> of_xlate and taking care of highbank and mvebu, the workaround should be
>> removed.
>>
>> Signed-off-by: Laurent Pinchart 
>> ---
>>   arch/arm/mm/dma-mapping.c | 9 +
>>   1 file changed, 9 insertions(+)
>>
>> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
>> index ff7ed56..4686d66 100644
>> --- a/arch/arm/mm/dma-mapping.c
>> +++ b/arch/arm/mm/dma-mapping.c
>> @@ -2259,6 +2259,15 @@ void arch_setup_dma_ops(struct device *dev, u64 
>> dma_base, u64 size,
>>  struct dma_map_ops *dma_ops;
>>
>>  dev->archdata.dma_coherent = coherent;
>> +
>> +/*
>> + * Don't override the dma_ops if they have already been set. Ideally
>> + * this should be the only location where dma_ops are set, remove this
>> + * check when all other callers of set_dma_ops will have disappeared.
>> + */
>> +if (dev->archdata.dma_ops)
>> +return;
>> +
>>  if (arm_setup_iommu_dma_ops(dev, dma_base, size, iommu))
>>  dma_ops = arm_get_iommu_dma_map_ops(coherent);
>>  else
>
>Please add also set_dma_ops(dev, NULL) to arm_teardown_iommu_dma_ops()
>function
>to this patch, because otherwise dma_ops won't be configured properly if
>deferred
>probe of the given device happens: iommu dma ops will be set for the
>first probe
>try, then iommu structures will be teared down after EPROBEDEFER error,
>and then
>before next probe ties dma_ops will be already set, but no iommu
>structures are
>available.
>
>For a convenience, this a fixup patch:
>https://git.linaro.org/people/marek.szyprowski/linux-srpol.git/commitdiff/55adefd43cee9d4beb15cb1bbd805c5059b56b4f
>
 Right, this and will also add the remove_device during deconfigure as well.

Regards,
 Sricharan

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

RE: [PATCH 7/8] iommu: of: Handle IOMMU lookup failure with deferred probing or error

2016-09-07 Thread Sricharan 
Hi Marek,

>Hi Sricharan,
>
>
>On 2016-08-12 17:40, Sricharan wrote:
>> Hi Tomaz,
>>
 +   if (ops->add_device)
 +   ops = ops->add_device(dev) ? ops : NULL;
>>> Patch description fails to mention anything about this change. Also it
>>> looks slightly incorrect to lose the error condition here. I think we
>>> should at least print some error message here and tell the user that
>>> we are falling back to non-IOMMU setup.
>>  Ok, will have to improve the patch description to add this and will
>>  fix the error value as well. Will also add the remove_device during
>>   deconfigure as well.
>>
>>>
  of_node_put(np);
  idx++;
 @@ -200,7 +213,7 @@ const struct iommu_ops *of_iommu_configure(struct 
 device *dev,

   err_put_node:
  of_node_put(np);
 -   return NULL;
 +   return ops;
   }

   void __init of_iommu_init(void)
 @@ -211,7 +224,7 @@ void __init of_iommu_init(void)
  for_each_matching_node_and_match(np, matches, ) {
  const of_iommu_init_fn init_fn = match->data;

 -   if (init_fn(np))
 +   if (init_fn && init_fn(np))
>>> When is it possible to have NULL init_fn?
>> ya wrong, should not be needed.
>
>init_fn can be NULL if you convert IOMMU driver to use platform device
>infrastructure based initialization and you don't need to do anything
>before the driver gets probed, so please keep this check.
>
>I used this approach here:
>https://git.linaro.org/people/marek.szyprowski/linux-srpol.git/commitdiff/a30735973573128b14bb4a25cf4debaa0979a655
>(this commit is a part of v4.8-clocks-pm branch)
>
>IOMMU_OF_DECLARE() with NULL init_fn is needed to notify IOMMU core that
>this IOMMU driver is available in the system and core has to defer
>probing of drivers before the IOMMU driver gets initialized from its
>controller's platform device.
>
   ok, thanks, understand. I was not thinking about the non-dt case.
I will keep this check then.

Regards,
 Sricharan 

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