[PATCH] iommu/vt-d: Fix status code for Allocate/Free PASID command

[Zenghui Yu]

As per Intel vt-d spec, Rev 3.0 (section 10.4.45 "Virtual Command Response
Register"), the status code of "No PASID available" error in response to
the Allocate PASID command is 2, not 1. The same for "Invalid PASID" error
in response to the Free PASID command.

We will otherwise see confusing kernel log under the command failure from
guest side. Fix it.

Fixes: 24f27d32ab6b ("iommu/vt-d: Enlightened PASID allocation")
Signed-off-by: Zenghui Yu 
---
 drivers/iommu/intel/pasid.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/intel/pasid.h b/drivers/iommu/intel/pasid.h
index 97dfcffbf495..444c0bec221a 100644
--- a/drivers/iommu/intel/pasid.h
+++ b/drivers/iommu/intel/pasid.h
@@ -30,8 +30,8 @@
 #define VCMD_VRSP_IP   0x1
 #define VCMD_VRSP_SC(e)(((e) >> 1) & 0x3)
 #define VCMD_VRSP_SC_SUCCESS   0
-#define VCMD_VRSP_SC_NO_PASID_AVAIL1
-#define VCMD_VRSP_SC_INVALID_PASID 1
+#define VCMD_VRSP_SC_NO_PASID_AVAIL2
+#define VCMD_VRSP_SC_INVALID_PASID 2
 #define VCMD_VRSP_RESULT_PASID(e)  (((e) >> 8) & 0xf)
 #define VCMD_CMD_OPERAND(e)((e) << 8)
 /*
-- 
2.19.1

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

Re: [PATCH v12 10/10] iommu/arm-smmu-v3: Add stall support for platform devices

[Zhou Wang]

On 2021/2/27 0:29, Jean-Philippe Brucker wrote:
> Hi Zhou,
> 
> On Fri, Feb 26, 2021 at 05:43:27PM +0800, Zhou Wang wrote:
>> On 2021/2/1 19:14, Jean-Philippe Brucker wrote:
>>> Hi Zhou,
>>>
>>> On Mon, Feb 01, 2021 at 09:18:42AM +0800, Zhou Wang wrote:
> @@ -1033,8 +1076,7 @@ int arm_smmu_write_ctx_desc(struct arm_smmu_domain 
> *smmu_domain, int ssid,
>   FIELD_PREP(CTXDESC_CD_0_ASID, cd->asid) |
>   CTXDESC_CD_0_V;
>  
> - /* STALL_MODEL==0b10 && CD.S==0 is ILLEGAL */
> - if (smmu->features & ARM_SMMU_FEAT_STALL_FORCE)
> + if (smmu_domain->stall_enabled)

 Could we add ssid checking here? like: if (smmu_domain->stall_enabled && 
 ssid).
 The reason is if not CD.S will also be set when ssid is 0, which is not 
 needed.
>>>
>>> Some drivers may want to get stall events on SSID 0:
>>> https://lore.kernel.org/kvm/20210125090402.1429-1-lushenm...@huawei.com/#t
>>>
>>> Are you seeing an issue with stall events on ssid 0?  Normally there
>>> shouldn't be any fault on this context, but if they happen and no handler
>>> is registered, the SMMU driver will just abort them and report them like a
>>> non-stall event.
>>
>> Hi Jean,
>>
>> I notice that there is problem. In my case, I expect that CD0 is for kernel
>> and other CDs are for user space. Normally there shouldn't be any fault in
>> kernel, however, we have RAS case which is for some reason there may has
>> invalid address access from hardware device.
>>
>> So at least there are two different address access failures: 1. hardware RAS 
>> problem;
>> 2. software fault fail(e.g. kill process when doing DMA). Handlings for these
>> two are different: for 1, we should reset hardware device; for 2, stop 
>> related
>> DMA is enough.
> 
> Right, and in case 2 there should be no report printed since it can be
> triggered by user, while you probably want to be loud in case 1.
> 
>> Currently if SMMU returns the same signal(by SMMU resume abort), master 
>> device
>> driver can not tell these two kinds of cases.
> 
> This part I don't understand. So the SMMU sends a RESUME(abort) command,
> and then the master reports the DMA error to the device driver, which
> cannot differentiate 1 from 2?  (I guess there is no SSID in this report?)
> But how does disabling stall change this?  The invalid DMA access will
> still be aborted by the SMMU.

This is about the hardware design. In D06 board, an invalid DMA access from
accelerator devices will be aborted, and an hardware error signal will be
returned to accelerator devices, which reports it as a RAS error irq.
while for the stall case, error signal triggered by SMMU resume abort is
also reported as same RAS error irq. This is problem in D60 board.

In next generation of hardware, a new irq will be added to report SMMU resume
abort information, it works with related registers in accelerator devices to
get related hardware queue, which need to be stopped.

So if CD0.S is 1, invalid DMA access in kernel will be reported into above
new added irq, which has not enough information to tell RAS errors(there are 10+
hardware RAS errors) from SMMU resume abort.

> 
> Hypothetically, would it work if all stall events that could not be
> handled went to the device driver?  Those reports would contain the SSID
> (or lack thereof), so you could reset the device in case 1 and ignore case
> 2. Though resetting the device in the middle of a stalled transaction

As above, it is hard to tell RAS errors and SMMU resume abort in SMMU resume 
abort
now :(

> probably comes with its own set of problems.
> 
>> From the basic concept, if a CD is used for kernel, its S bit should not be 
>> set.
>> How about we add iommu domain check here too, if DMA domain we do not set S 
>> bit for
>> CD0, if unmanaged domain we set S bit for all CDs?
> 
> I think disabling stall for CD0 of a DMA domain makes sense in general,
> even though I don't really understand how that fixes your issue. But

As above, if disabling stall for CD0, an invalid DMA access will be handled
by RAS error irq.

> someone might come up with a good use-case for receiving stall events on

If A DMA access in kernel fails, I think there should be a RAS issue :)
So better to disable CD0 stall for DMA domain.

Best,
Zhou

> DMA mappings, so I'm wondering whether the alternative solution where we
> report unhandled stall events to the device driver would also work for
> you.
> 
> Thanks,
> Jean
> 
> .
> 

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

Re: [PATCH] iommu/amd: Fix event counter availability check

[Shuah Khan]

On 2/26/21 2:44 PM, Paul Menzel wrote:

[cc: +suravee, +jörg]

Dear Alex, dear Shuah, dear Suravee, dear Jörg,


Am 03.06.20 um 08:54 schrieb Alexander Monakov:

On Tue, 2 Jun 2020, Shuah Khan wrote:


I changed the logic to read config to get max banks and counters
before checking if counters are writable and tried writing to all.
The result is the same and all of them aren't writable. However,
when disable the writable check and assume they are, I can run

[snip]

This is similar to what I did. I also noticed that counters can
be successfully used with perf if the initial check is ignored.
I was considering sending a patch to remove the check and adjust
the event counting logic to use counters as read-only, but after
a bit more investigation I've noticed how late pci_enable_device
is done, and came up with this patch. It's a path of less resistance:
I'd expect maintainers to be more averse to removing the check
rather than fixing it so it works as intended (even though I think
the check should not be there in the first place).

However:

The ability to modify the counters is needed only for sampling the
events (getting an interrupt when a counter overflows). There's no
code to do that for these AMD IOMMU counters. A solution I would
prefer is to not write to those counters at all. It would simplify or
even remove a bunch of code. I can submit a corresponding patch if
there's general agreement this path is ok.

What do you think?


I like this idea. Suravee, Jörg, what do you think?

Commit 6778ff5b21b (iommu/amd: Fix performance counter initialization) 
delays the boot up to 100 ms, which is over 20 % on fast systems, and 
also just a workaround, and does not seem to work always. The delay is 
also not mentioned in the commit message.





Sounds good to me. I can test this out on my system.

thanks,
-- Shuah

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

Re: [PATCH] iommu/amd: Fix event counter availability check

[Paul Menzel]

[cc: +suravee, +jörg]

Dear Alex, dear Shuah, dear Suravee, dear Jörg,


Am 03.06.20 um 08:54 schrieb Alexander Monakov:

On Tue, 2 Jun 2020, Shuah Khan wrote:


I changed the logic to read config to get max banks and counters
before checking if counters are writable and tried writing to all.
The result is the same and all of them aren't writable. However,
when disable the writable check and assume they are, I can run

[snip]

This is similar to what I did. I also noticed that counters can
be successfully used with perf if the initial check is ignored.
I was considering sending a patch to remove the check and adjust
the event counting logic to use counters as read-only, but after
a bit more investigation I've noticed how late pci_enable_device
is done, and came up with this patch. It's a path of less resistance:
I'd expect maintainers to be more averse to removing the check
rather than fixing it so it works as intended (even though I think
the check should not be there in the first place).

However:

The ability to modify the counters is needed only for sampling the
events (getting an interrupt when a counter overflows). There's no
code to do that for these AMD IOMMU counters. A solution I would
prefer is to not write to those counters at all. It would simplify or
even remove a bunch of code. I can submit a corresponding patch if
there's general agreement this path is ok.

What do you think?


I like this idea. Suravee, Jörg, what do you think?

Commit 6778ff5b21b (iommu/amd: Fix performance counter initialization) 
delays the boot up to 100 ms, which is over 20 % on fast systems, and 
also just a workaround, and does not seem to work always. The delay is 
also not mentioned in the commit message.



Kind regards,

Paul


[1]: 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6778ff5b21bd8e78c8bd547fd66437cf2657fd9b

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

Re: [PATCH] swiotlb: swiotlb_tbl_map_single() kernel BUG in iommu-helper.h:30

[Brad Larson]

>
> On Fri, Feb 26, 2021 at 12:43:07PM -0800, Brad Larson wrote:
> > Kernel Oops introduced in next-20210222 due to get_max_slots return arg
> size.
> > In the function find_slots() variable max_slots is zero when
> boundary_mask is
> > 0x.
>
> I am looking at the stable/for-linus-5.12 and what I sent out for
> a GIT PULL and I believe this is already squashed in:
>
> 531 static int find_slots(struct device *dev, phys_addr_t orig_addr,
>
> 532 size_t alloc_size)
>
> 533 {
>
> 534 unsigned long boundary_mask = dma_get_seg_boundary(dev);
>
> 535 dma_addr_t tbl_dma_addr =
>
> 536 phys_to_dma_unencrypted(dev, io_tlb_start) &
> boundary_mask;
> 537 unsigned long max_slots = get_max_slots(boundary_mask);
>
> Could you double-check please?
>

Yes this is squashed in the snippet you are showing.  The bug was
introduced in
next-20210222 and is still there when I updated to next-20210226 today.


On Fri, Feb 26, 2021 at 1:00 PM Konrad Rzeszutek Wilk <
konrad.w...@oracle.com> wrote:

> On Fri, Feb 26, 2021 at 12:43:07PM -0800, Brad Larson wrote:
> > Kernel Oops introduced in next-20210222 due to get_max_slots return arg
> size.
> > In the function find_slots() variable max_slots is zero when
> boundary_mask is
> > 0x.
>
> I am looking at the stable/for-linus-5.12 and what I sent out for
> a GIT PULL and I believe this is already squashed in:
>
> 531 static int find_slots(struct device *dev, phys_addr_t orig_addr,
>
> 532 size_t alloc_size)
>
> 533 {
>
> 534 unsigned long boundary_mask = dma_get_seg_boundary(dev);
>
> 535 dma_addr_t tbl_dma_addr =
>
> 536 phys_to_dma_unencrypted(dev, io_tlb_start) &
> boundary_mask;
> 537 unsigned long max_slots = get_max_slots(boundary_mask);
>
> Could you double-check please?
>
> >
> > [0.242119] kernel BUG at ./include/linux/iommu-helper.h:30!
> > [0.247793] Internal error: Oops - BUG: 0 [#1] SMP
> > [0.252595] Modules linked in:
> > [0.255657] CPU: 0 PID: 93 Comm: kworker/0:1 Not tainted
> 5.11.0-next-20210224+ #25
> > [0.263245] Hardware name: Elba ASIC Board (DT)
> > [0.267784] Workqueue: events_freezable mmc_rescan
> > [0.272592] pstate: 6085 (nZCv daIf -PAN -UAO -TCO BTYPE=--)
> > [0.278612] pc : swiotlb_tbl_map_single+0x2b0/0x6a0
> > [0.283505] lr : swiotlb_tbl_map_single+0x440/0x6a0
> > [0.288395] sp : ffc0122736b0
> > [0.291713] x29: ffc0122736b0 x28: ffc010e3
> > [0.297039] x27: bbf58000 x26: 
> > [0.302364] x25:  x24: 0001
> > [0.307689] x23:  x22: 
> > [0.313013] x21:  x20: 
> > [0.318338] x19: 001241fd4600 x18: ffc010d288c8
> > [0.323662] x17: 0007 x16: 0001
> > [0.328987] x15: ffc092273367 x14: 3a424c54204f4920
> > [0.334311] x13: 6572617774666f73 x12: 20726e2030207865
> > [0.339636] x11: 646e692078787820 x10: 3062653737317830
> > [0.344960] x9 : 2074666968732031 x8 : ff977cf82368
> > [0.350285] x7 : 0001 x6 : c000efff
> > [0.355609] x5 : 00017fe8 x4 : 
> > [0.360934] x3 :  x2 : 18b0d50da009d000
> > [0.366258] x1 :  x0 : 0042
> > [0.371583] Call trace:
> > [0.374032]  swiotlb_tbl_map_single+0x2b0/0x6a0
> > [0.378573]  swiotlb_map+0xa8/0x2b0
> >
> > Signed-off-by: Brad Larson 
> > ---
> >  kernel/dma/swiotlb.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
> > index 369e4c3a0f2b..c10e855a03bc 100644
> > --- a/kernel/dma/swiotlb.c
> > +++ b/kernel/dma/swiotlb.c
> > @@ -534,7 +534,7 @@ static int find_slots(struct device *dev,
> phys_addr_t orig_addr,
> >   unsigned long boundary_mask = dma_get_seg_boundary(dev);
> >   dma_addr_t tbl_dma_addr =
> >   phys_to_dma_unencrypted(dev, io_tlb_start) & boundary_mask;
> > - unsigned int max_slots = get_max_slots(boundary_mask);
> > + unsigned long max_slots = get_max_slots(boundary_mask);
> >   unsigned int iotlb_align_mask =
> >   dma_get_min_align_mask(dev) & ~(IO_TLB_SIZE - 1);
> >   unsigned int nslots = nr_slots(alloc_size), stride;
> > --
> > 2.17.1
> >
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH] swiotlb: swiotlb_tbl_map_single() kernel BUG in iommu-helper.h:30

[Brad Larson]

Kernel Oops introduced in next-20210222 due to get_max_slots return arg size.
In the function find_slots() variable max_slots is zero when boundary_mask is
0x.

[0.242119] kernel BUG at ./include/linux/iommu-helper.h:30!
[0.247793] Internal error: Oops - BUG: 0 [#1] SMP
[0.252595] Modules linked in:
[0.255657] CPU: 0 PID: 93 Comm: kworker/0:1 Not tainted 
5.11.0-next-20210224+ #25
[0.263245] Hardware name: Elba ASIC Board (DT)
[0.267784] Workqueue: events_freezable mmc_rescan
[0.272592] pstate: 6085 (nZCv daIf -PAN -UAO -TCO BTYPE=--)
[0.278612] pc : swiotlb_tbl_map_single+0x2b0/0x6a0
[0.283505] lr : swiotlb_tbl_map_single+0x440/0x6a0
[0.288395] sp : ffc0122736b0
[0.291713] x29: ffc0122736b0 x28: ffc010e3
[0.297039] x27: bbf58000 x26: 
[0.302364] x25:  x24: 0001
[0.307689] x23:  x22: 
[0.313013] x21:  x20: 
[0.318338] x19: 001241fd4600 x18: ffc010d288c8
[0.323662] x17: 0007 x16: 0001
[0.328987] x15: ffc092273367 x14: 3a424c54204f4920
[0.334311] x13: 6572617774666f73 x12: 20726e2030207865
[0.339636] x11: 646e692078787820 x10: 3062653737317830
[0.344960] x9 : 2074666968732031 x8 : ff977cf82368
[0.350285] x7 : 0001 x6 : c000efff
[0.355609] x5 : 00017fe8 x4 : 
[0.360934] x3 :  x2 : 18b0d50da009d000
[0.366258] x1 :  x0 : 0042
[0.371583] Call trace:
[0.374032]  swiotlb_tbl_map_single+0x2b0/0x6a0
[0.378573]  swiotlb_map+0xa8/0x2b0

Signed-off-by: Brad Larson 
---
 kernel/dma/swiotlb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 369e4c3a0f2b..c10e855a03bc 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -534,7 +534,7 @@ static int find_slots(struct device *dev, phys_addr_t 
orig_addr,
unsigned long boundary_mask = dma_get_seg_boundary(dev);
dma_addr_t tbl_dma_addr =
phys_to_dma_unencrypted(dev, io_tlb_start) & boundary_mask;
-   unsigned int max_slots = get_max_slots(boundary_mask);
+   unsigned long max_slots = get_max_slots(boundary_mask);
unsigned int iotlb_align_mask =
dma_get_min_align_mask(dev) & ~(IO_TLB_SIZE - 1);
unsigned int nslots = nr_slots(alloc_size), stride;
-- 
2.17.1

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

Re: [PATCH] swiotlb: swiotlb_tbl_map_single() kernel BUG in iommu-helper.h:30

[Konrad Rzeszutek Wilk]

On Fri, Feb 26, 2021 at 01:18:14PM -0800, Brad Larson wrote:
> On Fri, Feb 26, 2021 at 12:43:07PM -0800, Brad Larson wrote:
> > Kernel Oops introduced in next-20210222 due to get_max_slots return arg
> size.
> > In the function find_slots() variable max_slots is zero when
> boundary_mask is
> > 0x.
> 
> I am looking at the stable/for-linus-5.12 and what I sent out for
> a GIT PULL and I believe this is already squashed in:
> 
> 531 static int find_slots(struct device *dev, phys_addr_t orig_addr,  
>  
> 
> 532 size_t alloc_size)
>  
> 
> 533 { 
>  
> 534 unsigned long boundary_mask = dma_get_seg_boundary(dev);  
>  
> 
> 535 dma_addr_t tbl_dma_addr = 
>  
> 536 phys_to_dma_unencrypted(dev, io_tlb_start) &
> boundary_mask; 
> 537 unsigned long max_slots = get_max_slots(boundary_mask);
> 
> Could you double-check please?
> 
>  
> Yes this is squashed in the snippet you are showing.  The bug was introduced 
> in
> next-20210222 and is still there when I updated to next-20210226 today. 

Duh! It should be fixed now for the next one. Thank you!
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH] swiotlb: swiotlb_tbl_map_single() kernel BUG in iommu-helper.h:30

[Konrad Rzeszutek Wilk]

On Fri, Feb 26, 2021 at 12:43:07PM -0800, Brad Larson wrote:
> Kernel Oops introduced in next-20210222 due to get_max_slots return arg size.
> In the function find_slots() variable max_slots is zero when boundary_mask is
> 0x.

I am looking at the stable/for-linus-5.12 and what I sent out for
a GIT PULL and I believe this is already squashed in:

531 static int find_slots(struct device *dev, phys_addr_t orig_addr,

532 size_t alloc_size)  

533 {   

534 unsigned long boundary_mask = dma_get_seg_boundary(dev);

535 dma_addr_t tbl_dma_addr =   

536 phys_to_dma_unencrypted(dev, io_tlb_start) & boundary_mask; 

537 unsigned long max_slots = get_max_slots(boundary_mask);

Could you double-check please?

> 
> [0.242119] kernel BUG at ./include/linux/iommu-helper.h:30!
> [0.247793] Internal error: Oops - BUG: 0 [#1] SMP
> [0.252595] Modules linked in:
> [0.255657] CPU: 0 PID: 93 Comm: kworker/0:1 Not tainted 
> 5.11.0-next-20210224+ #25
> [0.263245] Hardware name: Elba ASIC Board (DT)
> [0.267784] Workqueue: events_freezable mmc_rescan
> [0.272592] pstate: 6085 (nZCv daIf -PAN -UAO -TCO BTYPE=--)
> [0.278612] pc : swiotlb_tbl_map_single+0x2b0/0x6a0
> [0.283505] lr : swiotlb_tbl_map_single+0x440/0x6a0
> [0.288395] sp : ffc0122736b0
> [0.291713] x29: ffc0122736b0 x28: ffc010e3
> [0.297039] x27: bbf58000 x26: 
> [0.302364] x25:  x24: 0001
> [0.307689] x23:  x22: 
> [0.313013] x21:  x20: 
> [0.318338] x19: 001241fd4600 x18: ffc010d288c8
> [0.323662] x17: 0007 x16: 0001
> [0.328987] x15: ffc092273367 x14: 3a424c54204f4920
> [0.334311] x13: 6572617774666f73 x12: 20726e2030207865
> [0.339636] x11: 646e692078787820 x10: 3062653737317830
> [0.344960] x9 : 2074666968732031 x8 : ff977cf82368
> [0.350285] x7 : 0001 x6 : c000efff
> [0.355609] x5 : 00017fe8 x4 : 
> [0.360934] x3 :  x2 : 18b0d50da009d000
> [0.366258] x1 :  x0 : 0042
> [0.371583] Call trace:
> [0.374032]  swiotlb_tbl_map_single+0x2b0/0x6a0
> [0.378573]  swiotlb_map+0xa8/0x2b0
> 
> Signed-off-by: Brad Larson 
> ---
>  kernel/dma/swiotlb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
> index 369e4c3a0f2b..c10e855a03bc 100644
> --- a/kernel/dma/swiotlb.c
> +++ b/kernel/dma/swiotlb.c
> @@ -534,7 +534,7 @@ static int find_slots(struct device *dev, phys_addr_t 
> orig_addr,
>   unsigned long boundary_mask = dma_get_seg_boundary(dev);
>   dma_addr_t tbl_dma_addr =
>   phys_to_dma_unencrypted(dev, io_tlb_start) & boundary_mask;
> - unsigned int max_slots = get_max_slots(boundary_mask);
> + unsigned long max_slots = get_max_slots(boundary_mask);
>   unsigned int iotlb_align_mask =
>   dma_get_min_align_mask(dev) & ~(IO_TLB_SIZE - 1);
>   unsigned int nslots = nr_slots(alloc_size), stride;
> -- 
> 2.17.1
> 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[Patch V2 06/13] platform-msi: Add device MSI infrastructure

[Megha Dey]

From: Thomas Gleixner 

Add device specific MSI domain infrastructure for devices which have their
own resource management and interrupt chip. These devices are not related
to PCI and contrary to platform MSI they do not share a common resource and
interrupt chip. They provide their own domain specific resource management
and interrupt chip.

This utilizes the new alloc/free override in a non evil way which avoids
having yet another set of specialized alloc/free functions. Just using
msi_domain_alloc/free_irqs() is sufficient

While initially it was suggested and tried to piggyback device MSI on
platform MSI, the better variant is to reimplement platform MSI on top of
device MSI.

Reviewed-by: Tony Luck 
Signed-off-by: Thomas Gleixner 
Signed-off-by: Megha Dey 
---
 drivers/base/platform-msi.c | 131 
 include/linux/irqdomain.h   |   1 +
 include/linux/msi.h |  24 
 kernel/irq/Kconfig  |   4 ++
 4 files changed, 160 insertions(+)

diff --git a/drivers/base/platform-msi.c b/drivers/base/platform-msi.c
index 9d9ccfc..6127b3b 100644
--- a/drivers/base/platform-msi.c
+++ b/drivers/base/platform-msi.c
@@ -419,3 +419,134 @@ int platform_msi_domain_alloc(struct irq_domain *domain, 
unsigned int virq,
 
return err;
 }
+
+#ifdef CONFIG_DEVICE_MSI
+/*
+ * Device specific MSI domain infrastructure for devices which have their
+ * own resource management and interrupt chip. These devices are not
+ * related to PCI and contrary to platform MSI they do not share a common
+ * resource and interrupt chip. They provide their own domain specific
+ * resource management and interrupt chip.
+ */
+
+static void device_msi_free_msi_entries(struct device *dev)
+{
+   struct list_head *msi_list = dev_to_msi_list(dev);
+   struct msi_desc *entry, *tmp;
+
+   list_for_each_entry_safe(entry, tmp, msi_list, list) {
+   list_del(>list);
+   free_msi_entry(entry);
+   }
+}
+
+/**
+ * device_msi_free_irqs - Free MSI interrupts assigned to  a device
+ * @dev:   Pointer to the device
+ *
+ * Frees the interrupt and the MSI descriptors.
+ */
+static void device_msi_free_irqs(struct irq_domain *domain, struct device *dev)
+{
+   __msi_domain_free_irqs(domain, dev);
+   device_msi_free_msi_entries(dev);
+}
+
+/**
+ * device_msi_alloc_irqs - Allocate MSI interrupts for a device
+ * @dev:   Pointer to the device
+ * @nvec:  Number of vectors
+ *
+ * Allocates the required number of MSI descriptors and the corresponding
+ * interrupt descriptors.
+ */
+static int device_msi_alloc_irqs(struct irq_domain *domain, struct device 
*dev, int nvec)
+{
+   int i, ret = -ENOMEM;
+
+   for (i = 0; i < nvec; i++) {
+   struct msi_desc *entry = alloc_msi_entry(dev, 1, NULL);
+
+   if (!entry)
+   goto fail;
+   list_add_tail(>list, dev_to_msi_list(dev));
+   }
+
+   ret = __msi_domain_alloc_irqs(domain, dev, nvec);
+   if (!ret)
+   return 0;
+fail:
+   device_msi_free_msi_entries(dev);
+   return ret;
+}
+
+static void device_msi_update_dom_ops(struct msi_domain_info *info)
+{
+   if (!info->ops->domain_alloc_irqs)
+   info->ops->domain_alloc_irqs = device_msi_alloc_irqs;
+   if (!info->ops->domain_free_irqs)
+   info->ops->domain_free_irqs = device_msi_free_irqs;
+   if (!info->ops->msi_prepare)
+   info->ops->msi_prepare = arch_msi_prepare;
+}
+
+/**
+ * device_msi_create_msi_irq_domain - Create an irq domain for devices
+ * @fwnode:Firmware node of the interrupt controller
+ * @info:  MSI domain info to configure the new domain
+ * @parent:Parent domain
+ */
+struct irq_domain *device_msi_create_irq_domain(struct fwnode_handle *fn,
+   struct msi_domain_info *info,
+   struct irq_domain *parent)
+{
+   struct irq_domain *domain;
+
+   if (info->flags & MSI_FLAG_USE_DEF_CHIP_OPS)
+   platform_msi_update_chip_ops(info);
+
+   if (info->flags & MSI_FLAG_USE_DEF_DOM_OPS)
+   device_msi_update_dom_ops(info);
+
+   msi_domain_set_default_info_flags(info);
+
+   domain = msi_create_irq_domain(fn, info, parent);
+   if (domain)
+   irq_domain_update_bus_token(domain, DOMAIN_BUS_DEVICE_MSI);
+   return domain;
+}
+
+#ifdef CONFIG_PCI
+#include 
+
+/**
+ * pci_subdevice_msi_create_irq_domain - Create an irq domain for subdevices
+ * @pdev:  Pointer to PCI device for which the subdevice domain is created
+ * @info:  MSI domain info to configure the new domain
+ */
+struct irq_domain *pci_subdevice_msi_create_irq_domain(struct pci_dev *pdev,
+  struct msi_domain_info 
*info)
+{
+   struct irq_domain *domain, *pdev_msi;
+   struct fwnode_handle *fn;
+
+   /*

[Patch V2 11/13] platform-msi: Add platform check for subdevice irq domain

[Megha Dey]

From: Lu Baolu 

The pci_subdevice_msi_create_irq_domain() should fail if the underlying
platform is not able to support IMS (Interrupt Message Storage). Otherwise,
the isolation of interrupt is not guaranteed.

For x86, IMS is only supported on bare metal for now. We could enable it
in the virtualization environments in the future if interrupt HYPERCALL
domain is supported or the hardware has the capability of interrupt
isolation for subdevices.

Cc: David Woodhouse 
Cc: Leon Romanovsky 
Cc: Kevin Tian 
Suggested-by: Thomas Gleixner 
Link: https://lore.kernel.org/linux-pci/87pn4nk7nn@nanos.tec.linutronix.de/
Link: https://lore.kernel.org/linux-pci/877dqrnzr3@nanos.tec.linutronix.de/
Link: https://lore.kernel.org/linux-pci/877dqqmc2h@nanos.tec.linutronix.de/
Reviewed-by: Tony Luck 
Signed-off-by: Lu Baolu 
Signed-off-by: Megha Dey 
---
 arch/x86/pci/common.c   | 72 +
 drivers/base/platform-msi.c |  8 +
 include/linux/msi.h |  2 ++
 3 files changed, 82 insertions(+)

diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
index 3507f45..64daa6a 100644
--- a/arch/x86/pci/common.c
+++ b/arch/x86/pci/common.c
@@ -12,6 +12,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 
 #include 
 #include 
@@ -724,3 +726,73 @@ struct pci_dev *pci_real_dma_dev(struct pci_dev *dev)
return dev;
 }
 #endif
+
+/*
+ * We want to figure out which context we are running in. But the hardware
+ * does not introduce a reliable way (instruction, CPUID leaf, MSR, whatever)
+ * which can be manipulated by the VMM to let the OS figure out where it runs.
+ * So we go with the below probably on_bare_metal() function as a replacement
+ * for definitely on_bare_metal() to go forward only for the very simple reason
+ * that this is the only option we have.
+ */
+static const char * const vmm_vendor_name[] = {
+   "QEMU", "Bochs", "KVM", "Xen", "VMware", "VMW", "VMware Inc.",
+   "innotek GmbH", "Oracle Corporation", "Parallels", "BHYVE"
+};
+
+static void read_type0_virtual_machine(const struct dmi_header *dm, void *p)
+{
+   u8 *data = (u8 *)dm + 0x13;
+
+   /* BIOS Information (Type 0) */
+   if (dm->type != 0 || dm->length < 0x14)
+   return;
+
+   /* Bit 4 of BIOS Characteristics Extension Byte 2*/
+   if (*data & BIT(4))
+   *((bool *)p) = true;
+}
+
+static bool smbios_virtual_machine(void)
+{
+   bool bit_present = false;
+
+   dmi_walk(read_type0_virtual_machine, _present);
+
+   return bit_present;
+}
+
+static bool on_bare_metal(struct device *dev)
+{
+   int i;
+
+   if (boot_cpu_has(X86_FEATURE_HYPERVISOR))
+   return false;
+
+   if (smbios_virtual_machine())
+   return false;
+
+   if (iommu_capable(dev->bus, IOMMU_CAP_VIOMMU_HINT))
+   return false;
+
+   for (i = 0; i < ARRAY_SIZE(vmm_vendor_name); i++)
+   if (dmi_match(DMI_SYS_VENDOR, vmm_vendor_name[i]))
+   return false;
+
+   pr_info("System running on bare metal, report to bugzilla.kernel.org if 
not the case.");
+
+   return true;
+}
+
+bool arch_support_pci_device_msi(struct pci_dev *pdev)
+{
+   /*
+* When we are running in a VMM context, the device IMS could only be
+* enabled when the underlying hardware supports interrupt isolation
+* of the subdevice, or any mechanism (trap, hypercall) is added so
+* that changes in the interrupt message store could be managed by the
+* VMM. For now, we only support the device IMS when we are running on
+* the bare metal.
+*/
+   return on_bare_metal(>dev);
+}
diff --git a/drivers/base/platform-msi.c b/drivers/base/platform-msi.c
index 6127b3b..c4a0d9c 100644
--- a/drivers/base/platform-msi.c
+++ b/drivers/base/platform-msi.c
@@ -519,6 +519,11 @@ struct irq_domain *device_msi_create_irq_domain(struct 
fwnode_handle *fn,
 #ifdef CONFIG_PCI
 #include 
 
+bool __weak arch_support_pci_device_msi(struct pci_dev *pdev)
+{
+   return false;
+}
+
 /**
  * pci_subdevice_msi_create_irq_domain - Create an irq domain for subdevices
  * @pdev:  Pointer to PCI device for which the subdevice domain is created
@@ -530,6 +535,9 @@ struct irq_domain 
*pci_subdevice_msi_create_irq_domain(struct pci_dev *pdev,
struct irq_domain *domain, *pdev_msi;
struct fwnode_handle *fn;
 
+   if (!arch_support_pci_device_msi(pdev))
+   return NULL;
+
/*
 * Retrieve the MSI domain of the underlying PCI device's MSI
 * domain. The PCI device domain's parent domain is also the parent
diff --git a/include/linux/msi.h b/include/linux/msi.h
index e915932..24abec0 100644
--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -489,6 +489,8 @@ struct irq_domain 
*pci_subdevice_msi_create_irq_domain(struct pci_dev *pdev,
 # endif
 #endif /* CONFIG_DEVICE_MSI */
 
+bool 

[Patch V2 09/13] iommu/vt-d: Add DEV-MSI support

[Megha Dey]

Add required support in the interrupt remapping driver for devices
which generate dev-msi interrupts and use the intel remapping
domain as the parent domain. Set the source-id of all dev-msi
interrupt requests to the parent PCI device associated with it.

Reviewed-by: Tony Luck 
Signed-off-by: Megha Dey 
---
 drivers/iommu/intel/irq_remapping.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/intel/irq_remapping.c 
b/drivers/iommu/intel/irq_remapping.c
index 611ef52..2a55e54 100644
--- a/drivers/iommu/intel/irq_remapping.c
+++ b/drivers/iommu/intel/irq_remapping.c
@@ -1282,6 +1282,9 @@ static void intel_irq_remapping_prepare_irte(struct 
intel_ir_data *data,
case X86_IRQ_ALLOC_TYPE_PCI_MSIX:
set_msi_sid(irte, msi_desc_to_pci_dev(info->desc));
break;
+   case X86_IRQ_ALLOC_TYPE_DEV_MSI:
+   set_msi_sid(irte, to_pci_dev(info->desc->dev->parent));
+   break;
default:
BUG_ON(1);
break;
@@ -1325,7 +1328,8 @@ static int intel_irq_remapping_alloc(struct irq_domain 
*domain,
if (!info || !iommu)
return -EINVAL;
if (nr_irqs > 1 && info->type != X86_IRQ_ALLOC_TYPE_PCI_MSI &&
-   info->type != X86_IRQ_ALLOC_TYPE_PCI_MSIX)
+   info->type != X86_IRQ_ALLOC_TYPE_PCI_MSIX &&
+   info->type != X86_IRQ_ALLOC_TYPE_DEV_MSI)
return -EINVAL;
 
/*
-- 
2.7.4

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

[Patch V2 00/13] Introduce dev-msi and interrupt message store

[Megha Dey]

Provide support for device specific MSI implementations for devices which
have their own resource management and interrupt chip. These devices are
not related to PCI and contrary to platform MSI they do not share a
common resource and interrupt chip. They provide their own domain specific
resource management and interrupt chip.

On top of this, add support for Interrupt Message Store or IMS[1], which
is a scalable device specific interrupt mechanism to support devices which
may need more than 2048 interrupts. With IMS, there is theoretically no
upper bound on the number of interrupts a device can support. The normal
IMS use case is for guest usages but it can very well be used by host too.

Introduce a generic IMS irq chip and domain which stores the interrupt
messages as an array in device memory. Allocation and freeing of interrupts
happens via the generic msi_domain_alloc/free_irqs() interface. One only
needs to ensure the interrupt domain is stored in the underlying device struct.

These patches can be divided into following steps:

1. X86 specific preparation for device MSI
   x86/irq: Add DEV_MSI allocation type
   x86/msi: Rename and rework pci_msi_prepare() to cover non-PCI MSI

2. Generic device MSI infrastructure
   platform-msi: Provide default irq_chip:: Ack
   genirq/proc: Take buslock on affinity write
   genirq/msi: Provide and use msi_domain_set_default_info_flags()
   platform-msi: Add device MSI infrastructure
   irqdomain/msi: Provide msi_alloc/free_store() callbacks
   genirq: Set auxiliary data for an interrupt
   genirq/msi: Provide helpers to return Linux IRQ/dev_msi hw IRQ number

3. Interrupt Message Store (IMS) irq domain/chip implementation for device array
   irqchip: Add IMS (Interrupt Message Store) driver
   iommu/vt-d: Add DEV-MSI support

4. Add platform check for subdevice irq domain
   iommu: Add capability IOMMU_CAP_VIOMMU
   platform-msi: Add platform check for subdevice irq domain

The device IMS (Interrupt Message Storage) should not be enabled in any
virtualization environments unless there is a HYPERCALL domain which
makes the changes in the message store monitored by the hypervisor.[2]
As the initial step, we allow the IMS to be enabled only if we are
running on the bare metal. It's easy to enable IMS in the virtualization
environments if above preconditions are met in the future.

These patches have been tested with the IDXD driver:
https://github.com/intel/idxd-driver idxd-stage2.5
Most of these patches are originally by Thomas:

https://lore.kernel.org/linux-hyperv/20200826111628.794979...@linutronix.de/
and are rebased on latest kernel.

This patches series has undergone a lot of changes since first submitted as an 
RFC
in September 2019. I have divided the changes into 3 stages for better 
understanding:

Stage 1: Standalone RFC IMS series[3]
-
https://lore.kernel.org/lkml/1568338328-22458-1-git-send-email-megha@linux.intel.com/
V1:
1. Extend existing platform-msi to support IMS
2. platform_msi_domain_alloc_irqs_group is introduced to allocate IMS
   interrupts, tagged with a group ID.
3. To free vectors of a particular group, platform_msi_domain_free_irqs_group
   API in introduced

Stage 2: dev-msi/IMS merged with Dave Jiang's IDXD series[2]

V1: (changes from stage 1):
1. Introduced a new list for platform-msi descriptors
2. Introduced dynamic allocation for IMS interrupts
3. shifted creation of ims domain from arch/x86 to drivers/
4. Removed arch specific callbacks
5. Introduced enum platform_msi_type
6. Added more technical details to the cover letter
7. Merge common code between platform-msi.c and ims-msi.c
8. Introduce new structures platform_msi_ops and platform_msi_funcs
9. Addressed Andriy Shevchenko's comments on RFC V1 patch series
10. Dropped the dynamic group allocation scheme.
11. Use RCU lock instead of mutex lock to protect the device list

V2:
1. IMS made dev-msi
2. With recommendations from Jason/Thomas/Dan on making IMS more generic
3. Pass a non-pci generic device(struct device) for IMS management instead of 
mdev
4. Remove all references to mdev and symbol_get/put
5. Remove all references to IMS in common code and replace with dev-msi
6. Remove dynamic allocation of platform-msi interrupts: no groups,no
   new msi list or list helpers
7. Create a generic dev-msi domain with and without interrupt remapping enabled.
8. Introduce dev_msi_domain_alloc_irqs and dev_msi_domain_free_irqs apis

V3:
1. No need to add support for 2 different dev-msi irq domains, a common
   once can be used for both the cases(with IR enabled/disabled)
2. Add arch specific function to specify additions to msi_prepare callback
   instead of making the callback a weak function
3. Call platform ops directly instead of a wrapper function
4. Make mask/unmask callbacks as void functions
5. dev->msi_domain should be updated at the device driver level before
 

[Patch V2 13/13] genirq/msi: Provide helpers to return Linux IRQ/dev_msi hw IRQ number

[Megha Dey]

From: Dave Jiang 

Add new helpers to get the Linux IRQ number and device specific index
for given device-relative vector so that the drivers don't need to
allocate their own arrays to keep track of the vectors and hwirq for
the multi vector device MSI case.

Reviewed-by: Tony Luck 
Signed-off-by: Dave Jiang 
Signed-off-by: Megha Dey 
---
 include/linux/msi.h |  2 ++
 kernel/irq/msi.c| 44 
 2 files changed, 46 insertions(+)

diff --git a/include/linux/msi.h b/include/linux/msi.h
index 24abec0..d60a6ba 100644
--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -451,6 +451,8 @@ struct irq_domain *platform_msi_create_irq_domain(struct 
fwnode_handle *fwnode,
 int platform_msi_domain_alloc_irqs(struct device *dev, unsigned int nvec,
   irq_write_msi_msg_t write_msi_msg);
 void platform_msi_domain_free_irqs(struct device *dev);
+int msi_irq_vector(struct device *dev, unsigned int nr);
+int dev_msi_hwirq(struct device *dev, unsigned int nr);
 
 /* When an MSI domain is used as an intermediate domain */
 int msi_domain_prepare_irqs(struct irq_domain *domain, struct device *dev,
diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c
index 047b59d..f2a8f55 100644
--- a/kernel/irq/msi.c
+++ b/kernel/irq/msi.c
@@ -581,4 +581,48 @@ struct msi_domain_info *msi_get_domain_info(struct 
irq_domain *domain)
return (struct msi_domain_info *)domain->host_data;
 }
 
+/**
+ * msi_irq_vector - Get the Linux IRQ number of a device vector
+ * @dev: device to operate on
+ * @nr: device-relative interrupt vector index (0-based).
+ *
+ * Returns the Linux IRQ number of a device vector.
+ */
+int msi_irq_vector(struct device *dev, unsigned int nr)
+{
+   struct msi_desc *entry;
+   int i = 0;
+
+   for_each_msi_entry(entry, dev) {
+   if (i == nr)
+   return entry->irq;
+   i++;
+   }
+   WARN_ON_ONCE(1);
+   return -EINVAL;
+}
+EXPORT_SYMBOL_GPL(msi_irq_vector);
+
+/**
+ * dev_msi_hwirq - Get the device MSI hw IRQ number of a device vector
+ * @dev: device to operate on
+ * @nr: device-relative interrupt vector index (0-based).
+ *
+ * Return the dev_msi hw IRQ number of a device vector.
+ */
+int dev_msi_hwirq(struct device *dev, unsigned int nr)
+{
+   struct msi_desc *entry;
+   int i = 0;
+
+   for_each_msi_entry(entry, dev) {
+   if (i == nr)
+   return entry->device_msi.hwirq;
+   i++;
+   }
+   WARN_ON_ONCE(1);
+   return -EINVAL;
+}
+EXPORT_SYMBOL_GPL(dev_msi_hwirq);
+
 #endif /* CONFIG_GENERIC_MSI_IRQ_DOMAIN */
-- 
2.7.4

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

[Patch V2 05/13] genirq/msi: Provide and use msi_domain_set_default_info_flags()

[Megha Dey]

From: Thomas Gleixner 

MSI interrupts have some common flags which should be set not only for
PCI/MSI interrupts.

Move the PCI/MSI flag setting into a common function so it can be reused.

Reviewed-by: Tony Luck 
Signed-off-by: Thomas Gleixner 
Signed-off-by: Megha Dey 
---
 drivers/pci/msi.c   |  7 +--
 include/linux/msi.h |  1 +
 kernel/irq/msi.c| 24 
 3 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 3162f88..20d2512 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -1492,12 +1492,7 @@ struct irq_domain *pci_msi_create_irq_domain(struct 
fwnode_handle *fwnode,
if (info->flags & MSI_FLAG_USE_DEF_CHIP_OPS)
pci_msi_domain_update_chip_ops(info);
 
-   info->flags |= MSI_FLAG_ACTIVATE_EARLY;
-   if (IS_ENABLED(CONFIG_GENERIC_IRQ_RESERVATION_MODE))
-   info->flags |= MSI_FLAG_MUST_REACTIVATE;
-
-   /* PCI-MSI is oneshot-safe */
-   info->chip->flags |= IRQCHIP_ONESHOT_SAFE;
+   msi_domain_set_default_info_flags(info);
 
domain = msi_create_irq_domain(fwnode, info, parent);
if (!domain)
diff --git a/include/linux/msi.h b/include/linux/msi.h
index f3e54d2..f6e52de 100644
--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -454,6 +454,7 @@ int platform_msi_domain_alloc(struct irq_domain *domain, 
unsigned int virq,
 void platform_msi_domain_free(struct irq_domain *domain, unsigned int virq,
  unsigned int nvec);
 void *platform_msi_get_host_data(struct irq_domain *domain);
+void msi_domain_set_default_info_flags(struct msi_domain_info *info);
 #endif /* CONFIG_GENERIC_MSI_IRQ_DOMAIN */
 
 #ifdef CONFIG_PCI_MSI_IRQ_DOMAIN
diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c
index b338d62..c54316d 100644
--- a/kernel/irq/msi.c
+++ b/kernel/irq/msi.c
@@ -70,6 +70,30 @@ void get_cached_msi_msg(unsigned int irq, struct msi_msg 
*msg)
 EXPORT_SYMBOL_GPL(get_cached_msi_msg);
 
 #ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN
+void msi_domain_set_default_info_flags(struct msi_domain_info *info)
+{
+   /* Required so that a device latches a valid MSI message on startup */
+   info->flags |= MSI_FLAG_ACTIVATE_EARLY;
+
+   /*
+* Interrupt reservation mode allows to stear the MSI message of an
+* inactive device to a special (usually spurious interrupt) target.
+* This allows to prevent interrupt vector exhaustion e.g. on x86.
+* But (PCI)MSI interrupts are activated early - see above - so the
+* interrupt request/startup sequence would not try to allocate a
+* usable vector which means that the device interrupts would end
+* up on the special vector and issue spurious interrupt messages.
+* Setting the reactivation flag ensures that when the interrupt
+* is requested the activation is invoked again so that a real
+* vector can be allocated.
+*/
+   if (IS_ENABLED(CONFIG_GENERIC_IRQ_RESERVATION_MODE))
+   info->flags |= MSI_FLAG_MUST_REACTIVATE;
+
+   /* MSI is oneshot-safe at least in theory */
+   info->chip->flags |= IRQCHIP_ONESHOT_SAFE;
+}
+
 static inline void irq_chip_write_msi_msg(struct irq_data *data,
  struct msi_msg *msg)
 {
-- 
2.7.4

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

[Patch V2 07/13] irqdomain/msi: Provide msi_alloc/free_store() callbacks

[Megha Dey]

From: Thomas Gleixner 

For devices which don't have a standard storage for MSI messages like the
upcoming IMS (Interrupt Message Store) it's required to allocate storage
space before allocating interrupts and after freeing them.

This could be achieved with the existing callbacks, but that would be
awkward because they operate on msi_alloc_info_t which is not uniform
across architectures. Also these callbacks are invoked per interrupt but
the allocation might have bulk requirements depending on the device.

As such devices can operate on different architectures it is simpler to
have separate callbacks which operate on struct device. The resulting
storage information has to be stored in struct msi_desc so the underlying
irq chip implementation can retrieve it for the relevant operations.

Reviewed-by: Tony Luck 
Signed-off-by: Thomas Gleixner 
Signed-off-by: Megha Dey 
---
 include/linux/msi.h |  8 
 kernel/irq/msi.c| 11 +++
 2 files changed, 19 insertions(+)

diff --git a/include/linux/msi.h b/include/linux/msi.h
index 46e879c..e915932 100644
--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -323,6 +323,10 @@ struct msi_domain_info;
  * function.
  * @domain_free_irqs:  Optional function to override the default free
  * function.
+ * @msi_alloc_store:   Optional callback to allocate storage in a device
+ * specific non-standard MSI store
+ * @msi_alloc_free:Optional callback to free storage in a device
+ * specific non-standard MSI store
  *
  * @get_hwirq, @msi_init and @msi_free are callbacks used by
  * msi_create_irq_domain() and related interfaces
@@ -372,6 +376,10 @@ struct msi_domain_ops {
 struct device *dev, int nvec);
void(*domain_free_irqs)(struct irq_domain *domain,
struct device *dev);
+   int (*msi_alloc_store)(struct irq_domain *domain,
+  struct device *dev, int nvec);
+   void(*msi_free_store)(struct irq_domain *domain,
+ struct device *dev);
 };
 
 /**
diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c
index c54316d..047b59d 100644
--- a/kernel/irq/msi.c
+++ b/kernel/irq/msi.c
@@ -434,6 +434,12 @@ int __msi_domain_alloc_irqs(struct irq_domain *domain, 
struct device *dev,
if (ret)
return ret;
 
+   if (ops->msi_alloc_store) {
+   ret = ops->msi_alloc_store(domain, dev, nvec);
+   if (ret)
+   return ret;
+   }
+
for_each_msi_entry(desc, dev) {
ops->set_desc(, desc);
 
@@ -529,6 +535,8 @@ int msi_domain_alloc_irqs(struct irq_domain *domain, struct 
device *dev,
 
 void __msi_domain_free_irqs(struct irq_domain *domain, struct device *dev)
 {
+   struct msi_domain_info *info = domain->host_data;
+   struct msi_domain_ops *ops = info->ops;
struct msi_desc *desc;
 
for_each_msi_entry(desc, dev) {
@@ -542,6 +550,9 @@ void __msi_domain_free_irqs(struct irq_domain *domain, 
struct device *dev)
desc->irq = 0;
}
}
+
+   if (ops->msi_free_store)
+   ops->msi_free_store(domain, dev);
 }
 
 /**
-- 
2.7.4

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

[Patch V2 03/13] platform-msi: Provide default irq_chip:: Ack

[Megha Dey]

From: Thomas Gleixner 

For the upcoming device MSI support it's required to have a default
irq_chip::ack implementation (irq_chip_ack_parent) so the drivers do not
need to care.

Reviewed-by: Tony Luck 
Signed-off-by: Thomas Gleixner 
Signed-off-by: Megha Dey 
---
 drivers/base/platform-msi.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/base/platform-msi.c b/drivers/base/platform-msi.c
index 2c1e2e0..9d9ccfc 100644
--- a/drivers/base/platform-msi.c
+++ b/drivers/base/platform-msi.c
@@ -101,6 +101,8 @@ static void platform_msi_update_chip_ops(struct 
msi_domain_info *info)
chip->irq_mask = irq_chip_mask_parent;
if (!chip->irq_unmask)
chip->irq_unmask = irq_chip_unmask_parent;
+   if (!chip->irq_ack)
+   chip->irq_ack = irq_chip_ack_parent;
if (!chip->irq_eoi)
chip->irq_eoi = irq_chip_eoi_parent;
if (!chip->irq_set_affinity)
-- 
2.7.4

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

[Patch V2 12/13] irqchip: Add IMS (Interrupt Message Store) driver

[Megha Dey]

Generic IMS(Interrupt Message Store) irq chips and irq domain
implementations for IMS based devices which store the interrupt messages
in an array in device memory.

Allocation and freeing of interrupts happens via the generic
msi_domain_alloc/free_irqs() interface. No special purpose IMS magic
required as long as the interrupt domain is stored in the underlying
device struct. The irq_set_auxdata() is used to program the pasid into
the IMS entry.

[Megha: Fixed compile time errors
Added necessary dependencies to IMS_MSI_ARRAY config
Fixed polarity of IMS_VECTOR_CTRL
Added reads after writes to flush writes to device
Added set_desc ops to IMS msi domain ops
Tested the IMS infrastructure with the IDXD driver]

Reviewed-by: Tony Luck 
Signed-off-by: Thomas Gleixner 
Signed-off-by: Megha Dey 
---
 drivers/irqchip/Kconfig |  14 +++
 drivers/irqchip/Makefile|   1 +
 drivers/irqchip/irq-ims-msi.c   | 211 
 include/linux/irqchip/irq-ims-msi.h |  68 
 4 files changed, 294 insertions(+)
 create mode 100644 drivers/irqchip/irq-ims-msi.c
 create mode 100644 include/linux/irqchip/irq-ims-msi.h

diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index e74fa20..2fb0c24 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -586,4 +586,18 @@ config MST_IRQ
help
  Support MStar Interrupt Controller.
 
+config IMS_MSI
+   depends on PCI
+   select DEVICE_MSI
+   bool
+
+config IMS_MSI_ARRAY
+   bool "IMS Interrupt Message Store MSI controller for device memory 
storage arrays"
+   depends on PCI
+   select IMS_MSI
+   select GENERIC_MSI_IRQ_DOMAIN
+   help
+ Support for IMS Interrupt Message Store MSI controller
+ with IMS slot storage in a slot array in device memory
+
 endmenu
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index c59b95a..e903201 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -113,3 +113,4 @@ obj-$(CONFIG_LOONGSON_PCH_MSI)  += 
irq-loongson-pch-msi.o
 obj-$(CONFIG_MST_IRQ)  += irq-mst-intc.o
 obj-$(CONFIG_SL28CPLD_INTC)+= irq-sl28cpld.o
 obj-$(CONFIG_MACH_REALTEK_RTL) += irq-realtek-rtl.o
+obj-$(CONFIG_IMS_MSI)  += irq-ims-msi.o
diff --git a/drivers/irqchip/irq-ims-msi.c b/drivers/irqchip/irq-ims-msi.c
new file mode 100644
index 000..fa23207
--- /dev/null
+++ b/drivers/irqchip/irq-ims-msi.c
@@ -0,0 +1,211 @@
+// SPDX-License-Identifier: GPL-2.0
+// (C) Copyright 2021 Thomas Gleixner 
+/*
+ * Shared interrupt chips and irq domains for IMS devices
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+#ifdef CONFIG_IMS_MSI_ARRAY
+
+struct ims_array_data {
+   struct ims_array_info   info;
+   unsigned long   map[0];
+};
+
+static inline void iowrite32_and_flush(u32 value, void __iomem *addr)
+{
+   iowrite32(value, addr);
+   ioread32(addr);
+}
+
+static void ims_array_mask_irq(struct irq_data *data)
+{
+   struct msi_desc *desc = irq_data_get_msi_desc(data);
+   struct ims_slot __iomem *slot = desc->device_msi.priv_iomem;
+   u32 __iomem *ctrl = >ctrl;
+
+   iowrite32_and_flush(ioread32(ctrl) | IMS_CTRL_VECTOR_MASKBIT, ctrl);
+}
+
+static void ims_array_unmask_irq(struct irq_data *data)
+{
+   struct msi_desc *desc = irq_data_get_msi_desc(data);
+   struct ims_slot __iomem *slot = desc->device_msi.priv_iomem;
+   u32 __iomem *ctrl = >ctrl;
+
+   iowrite32_and_flush(ioread32(ctrl) & ~IMS_CTRL_VECTOR_MASKBIT, ctrl);
+}
+
+static void ims_array_write_msi_msg(struct irq_data *data, struct msi_msg *msg)
+{
+   struct msi_desc *desc = irq_data_get_msi_desc(data);
+   struct ims_slot __iomem *slot = desc->device_msi.priv_iomem;
+
+   iowrite32(msg->address_lo, >address_lo);
+   iowrite32(msg->address_hi, >address_hi);
+   iowrite32_and_flush(msg->data, >data);
+}
+
+static int ims_array_set_auxdata(struct irq_data *data, unsigned int which,
+u64 auxval)
+{
+   struct msi_desc *desc = irq_data_get_msi_desc(data);
+   struct ims_slot __iomem *slot = desc->device_msi.priv_iomem;
+   u32 val, __iomem *ctrl = >ctrl;
+
+   if (which != IMS_AUXDATA_CONTROL_WORD)
+   return -EINVAL;
+   if (auxval & ~(u64)IMS_CONTROL_WORD_AUXMASK)
+   return -EINVAL;
+
+   val = ioread32(ctrl) & IMS_CONTROL_WORD_IRQMASK;
+   iowrite32_and_flush(val | (u32)auxval, ctrl);
+   return 0;
+}
+
+static const struct irq_chip ims_array_msi_controller = {
+   .name   = "IMS",
+   .irq_mask   = ims_array_mask_irq,
+   .irq_unmask = ims_array_unmask_irq,
+   .irq_write_msi_msg  = ims_array_write_msi_msg,
+   .irq_set_auxdata= ims_array_set_auxdata,
+   .irq_retrigger  = 

[Patch V2 04/13] genirq/proc: Take buslock on affinity write

[Megha Dey]

From: Thomas Gleixner 

Until now interrupt chips which support setting affinity are not locking
the associated bus lock for two reasons:

 - All chips which support affinity setting do not use buslock because they
   just can operated directly on the hardware.

 - All chips which use buslock do not support affinity setting because
   their interrupt chips are not capable. These chips are usually connected
   over a bus like I2C, SPI etc. and have an interrupt output which is
   conneted to CPU interrupt of some sort. So there is no way to set the
   affinity on the chip itself.

Upcoming hardware which is PCIE based sports a non standard MSI(X) variant
which stores the MSI message in RAM which is associated to e.g. a device
queue. The device manages this RAM and writes have to be issued via command
queues or similar mechanisms which is obviously not possible from interrupt
disabled, raw spinlock held context.

The buslock mechanism of irq chips can be utilized to support that. The
affinity write to the chip writes to shadow state, marks it pending and the
irq chip's irq_bus_sync_unlock() callback handles the command queue and
wait for completion similar to the other chip operations on I2C or SPI
buses.

Change the locking in irq_set_affinity() to bus_lock/unlock to help with
that. There are a few other callers than the proc interface, but none of
them is affected by this change as none of them affects an irq chip with
bus lock support.

Reviewed-by: Tony Luck 
Signed-off-by: Thomas Gleixner 
Signed-off-by: Megha Dey 
---
 kernel/irq/manage.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index dec3f73..85ede4e 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -443,16 +443,16 @@ int irq_update_affinity_desc(unsigned int irq,
 
 int __irq_set_affinity(unsigned int irq, const struct cpumask *mask, bool 
force)
 {
-   struct irq_desc *desc = irq_to_desc(irq);
+   struct irq_desc *desc;
unsigned long flags;
int ret;
 
+   desc = irq_get_desc_buslock(irq, , IRQ_GET_DESC_CHECK_GLOBAL);
if (!desc)
return -EINVAL;
 
-   raw_spin_lock_irqsave(>lock, flags);
ret = irq_set_affinity_locked(irq_desc_get_irq_data(desc), mask, force);
-   raw_spin_unlock_irqrestore(>lock, flags);
+   irq_put_desc_busunlock(desc, flags);
return ret;
 }
 
-- 
2.7.4

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

[Patch V2 10/13] iommu: Add capability IOMMU_CAP_VIOMMU_HINT

[Megha Dey]

From: Lu Baolu 

Some IOMMU specification defines some kind of hint mechanism, through
which BIOS can imply that OS runs in a virtualized environment. For
example, the caching mode defined in VT-d spec and NpCache capability
defined in the AMD IOMMU specification. This hint could also be used
outside of the IOMMU subsystem, where it could be used with other known
means (CPUID, smbios) to sense whether Linux is running in a virtualized
environment. Add a capability bit so that it could be used there.

Cc: Joerg Roedel 
Reviewed-by: Tony Luck 
Signed-off-by: Lu Baolu 
Signed-off-by: Megha Dey 
---
 drivers/iommu/amd/iommu.c| 2 ++
 drivers/iommu/intel/iommu.c  | 5 +
 drivers/iommu/virtio-iommu.c | 9 +
 include/linux/iommu.h| 2 ++
 4 files changed, 18 insertions(+)

diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index a69a8b5..a912318 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -2140,6 +2140,8 @@ static bool amd_iommu_capable(enum iommu_cap cap)
return (irq_remapping_enabled == 1);
case IOMMU_CAP_NOEXEC:
return false;
+   case IOMMU_CAP_VIOMMU_HINT:
+   return amd_iommu_np_cache;
default:
break;
}
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index ee09323..55fa198 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -294,6 +294,7 @@ static inline void context_clear_entry(struct context_entry 
*context)
  */
 static struct dmar_domain *si_domain;
 static int hw_pass_through = 1;
+static int intel_caching_mode;
 
 #define for_each_domain_iommu(idx, domain) \
for (idx = 0; idx < g_num_of_iommus; idx++) \
@@ -3253,6 +3254,8 @@ static int __init init_dmars(void)
 
if (!ecap_pass_through(iommu->ecap))
hw_pass_through = 0;
+   if (cap_caching_mode(iommu->cap))
+   intel_caching_mode = 1;
intel_svm_check(iommu);
}
 
@@ -5113,6 +5116,8 @@ static bool intel_iommu_capable(enum iommu_cap cap)
return domain_update_iommu_snooping(NULL) == 1;
if (cap == IOMMU_CAP_INTR_REMAP)
return irq_remapping_enabled == 1;
+   if (cap == IOMMU_CAP_VIOMMU_HINT)
+   return intel_caching_mode;
 
return false;
 }
diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
index 2bfdd57..e4941ca 100644
--- a/drivers/iommu/virtio-iommu.c
+++ b/drivers/iommu/virtio-iommu.c
@@ -931,7 +931,16 @@ static int viommu_of_xlate(struct device *dev, struct 
of_phandle_args *args)
return iommu_fwspec_add_ids(dev, args->args, 1);
 }
 
+static bool viommu_capable(enum iommu_cap cap)
+{
+   if (cap == IOMMU_CAP_VIOMMU_HINT)
+   return true;
+
+   return false;
+}
+
 static struct iommu_ops viommu_ops = {
+   .capable= viommu_capable,
.domain_alloc   = viommu_domain_alloc,
.domain_free= viommu_domain_free,
.attach_dev = viommu_attach_dev,
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 5e7fe51..9d0ade4 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -94,6 +94,8 @@ enum iommu_cap {
   transactions */
IOMMU_CAP_INTR_REMAP,   /* IOMMU supports interrupt isolation */
IOMMU_CAP_NOEXEC,   /* IOMMU_NOEXEC flag */
+   IOMMU_CAP_VIOMMU_HINT,  /* IOMMU can detect a hit for running in
+  VM */
 };
 
 /*
-- 
2.7.4

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

[Patch V2 08/13] genirq: Set auxiliary data for an interrupt

[Megha Dey]

Introduce a new function pointer in the irq_chip structure(irq_set_auxdata)
which is responsible for updating data which is stored in a shared register
or data storage. For example, the idxd driver uses the auxiliary data API
to enable/set and disable PASID field that is in the IMS entry (introduced
in a later patch) and that data are not typically present in MSI entry.

Reviewed-by: Tony Luck 
Signed-off-by: Megha Dey 
---
 include/linux/interrupt.h |  2 ++
 include/linux/irq.h   |  4 
 kernel/irq/manage.c   | 32 
 3 files changed, 38 insertions(+)

diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
index 967e257..461ed1c 100644
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -496,6 +496,8 @@ extern int irq_get_irqchip_state(unsigned int irq, enum 
irqchip_irq_state which,
 extern int irq_set_irqchip_state(unsigned int irq, enum irqchip_irq_state 
which,
 bool state);
 
+int irq_set_auxdata(unsigned int irq, unsigned int which, u64 val);
+
 #ifdef CONFIG_IRQ_FORCED_THREADING
 # ifdef CONFIG_PREEMPT_RT
 #  define force_irqthreads (true)
diff --git a/include/linux/irq.h b/include/linux/irq.h
index 2efde6a..fc19f32 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -491,6 +491,8 @@ static inline irq_hw_number_t irqd_to_hwirq(struct irq_data 
*d)
  * irq_request_resources
  * @irq_compose_msi_msg:   optional to compose message content for MSI
  * @irq_write_msi_msg: optional to write message content for MSI
+ * @irq_set_auxdata:   Optional function to update auxiliary data e.g. in
+ * shared registers
  * @irq_get_irqchip_state: return the internal state of an interrupt
  * @irq_set_irqchip_state: set the internal state of a interrupt
  * @irq_set_vcpu_affinity: optional to target a vCPU in a virtual machine
@@ -538,6 +540,8 @@ struct irq_chip {
void(*irq_compose_msi_msg)(struct irq_data *data, struct 
msi_msg *msg);
void(*irq_write_msi_msg)(struct irq_data *data, struct 
msi_msg *msg);
 
+   int (*irq_set_auxdata)(struct irq_data *data, unsigned int 
which, u64 auxval);
+
int (*irq_get_irqchip_state)(struct irq_data *data, enum 
irqchip_irq_state which, bool *state);
int (*irq_set_irqchip_state)(struct irq_data *data, enum 
irqchip_irq_state which, bool state);
 
diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index 85ede4e..68ff559 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -2860,3 +2860,35 @@ bool irq_check_status_bit(unsigned int irq, unsigned int 
bitmask)
return res;
 }
 EXPORT_SYMBOL_GPL(irq_check_status_bit);
+
+/**
+ * irq_set_auxdata - Set auxiliary data
+ * @irq:   Interrupt to update
+ * @which: Selector which data to update
+ * @auxval:Auxiliary data value
+ *
+ * Function to update auxiliary data for an interrupt, e.g. to update data
+ * which is stored in a shared register or data storage (e.g. IMS).
+ */
+int irq_set_auxdata(unsigned int irq, unsigned int which, u64 val)
+{
+   struct irq_desc *desc;
+   struct irq_data *data;
+   unsigned long flags;
+   int res = -ENODEV;
+
+   desc = irq_get_desc_buslock(irq, , 0);
+   if (!desc)
+   return -EINVAL;
+
+   for (data = >irq_data; data; data = irqd_get_parent_data(data)) {
+   if (data->chip->irq_set_auxdata) {
+   res = data->chip->irq_set_auxdata(data, which, val);
+   break;
+   }
+   }
+
+   irq_put_desc_busunlock(desc, flags);
+   return res;
+}
+EXPORT_SYMBOL_GPL(irq_set_auxdata);
-- 
2.7.4

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

[Patch V2 02/13] x86/msi: Rename and rework pci_msi_prepare() to cover non-PCI MSI

[Megha Dey]

From: Thomas Gleixner 

Rename it to x86_msi_prepare() and handle the allocation type setup
depending on the device type.

Add a new arch_msi_prepare define which will be utilized by the upcoming
device MSI support. Define it to NULL if not provided by an architecture
in the generic MSI header.

One arch specific function for MSI support is truly enough.

Reviewed-by: Tony Luck 
Signed-off-by: Thomas Gleixner 
Signed-off-by: Megha Dey 
---
 arch/x86/include/asm/msi.h  |  4 +++-
 arch/x86/kernel/apic/msi.c  | 27 ---
 drivers/pci/controller/pci-hyperv.c |  2 +-
 include/linux/msi.h |  4 
 4 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/arch/x86/include/asm/msi.h b/arch/x86/include/asm/msi.h
index b85147d..9bd214e 100644
--- a/arch/x86/include/asm/msi.h
+++ b/arch/x86/include/asm/msi.h
@@ -6,9 +6,11 @@
 
 typedef struct irq_alloc_info msi_alloc_info_t;
 
-int pci_msi_prepare(struct irq_domain *domain, struct device *dev, int nvec,
+int x86_msi_prepare(struct irq_domain *domain, struct device *dev, int nvec,
msi_alloc_info_t *arg);
 
+#define arch_msi_prepare   x86_msi_prepare
+
 /* Structs and defines for the X86 specific MSI message format */
 
 typedef struct x86_msi_data {
diff --git a/arch/x86/kernel/apic/msi.c b/arch/x86/kernel/apic/msi.c
index 44ebe25..84b16c7 100644
--- a/arch/x86/kernel/apic/msi.c
+++ b/arch/x86/kernel/apic/msi.c
@@ -153,26 +153,39 @@ static struct irq_chip pci_msi_controller = {
.flags  = IRQCHIP_SKIP_SET_WAKE,
 };
 
-int pci_msi_prepare(struct irq_domain *domain, struct device *dev, int nvec,
-   msi_alloc_info_t *arg)
+static void pci_msi_prepare(struct device *dev, msi_alloc_info_t *arg)
 {
-   struct pci_dev *pdev = to_pci_dev(dev);
-   struct msi_desc *desc = first_pci_msi_entry(pdev);
+   struct msi_desc *desc = first_msi_entry(dev);
 
-   init_irq_alloc_info(arg, NULL);
if (desc->msi_attrib.is_msix) {
arg->type = X86_IRQ_ALLOC_TYPE_PCI_MSIX;
} else {
arg->type = X86_IRQ_ALLOC_TYPE_PCI_MSI;
arg->flags |= X86_IRQ_ALLOC_CONTIGUOUS_VECTORS;
}
+}
+
+static void dev_msi_prepare(struct device *dev, msi_alloc_info_t *arg)
+{
+   arg->type = X86_IRQ_ALLOC_TYPE_DEV_MSI;
+}
+
+int x86_msi_prepare(struct irq_domain *domain, struct device *dev, int nvec,
+   msi_alloc_info_t *arg)
+{
+   init_irq_alloc_info(arg, NULL);
+
+   if (dev_is_pci(dev))
+   pci_msi_prepare(dev, arg);
+   else
+   dev_msi_prepare(dev, arg);
 
return 0;
 }
-EXPORT_SYMBOL_GPL(pci_msi_prepare);
+EXPORT_SYMBOL_GPL(x86_msi_prepare);
 
 static struct msi_domain_ops pci_msi_domain_ops = {
-   .msi_prepare= pci_msi_prepare,
+   .msi_prepare= x86_msi_prepare,
 };
 
 static struct msi_domain_info pci_msi_domain_info = {
diff --git a/drivers/pci/controller/pci-hyperv.c 
b/drivers/pci/controller/pci-hyperv.c
index 27a17a1..ac4fe8b7 100644
--- a/drivers/pci/controller/pci-hyperv.c
+++ b/drivers/pci/controller/pci-hyperv.c
@@ -1546,7 +1546,7 @@ static struct irq_chip hv_msi_irq_chip = {
 };
 
 static struct msi_domain_ops hv_msi_ops = {
-   .msi_prepare= pci_msi_prepare,
+   .msi_prepare= arch_msi_prepare,
.msi_free   = hv_msi_free,
 };
 
diff --git a/include/linux/msi.h b/include/linux/msi.h
index aef35fd..f3e54d2 100644
--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -473,4 +473,8 @@ static inline struct irq_domain 
*pci_msi_get_device_domain(struct pci_dev *pdev)
 }
 #endif /* CONFIG_PCI_MSI_IRQ_DOMAIN */
 
+#ifndef arch_msi_prepare
+# define arch_msi_prepare  NULL
+#endif
+
 #endif /* LINUX_MSI_H */
-- 
2.7.4

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

[Patch V2 01/13] x86/irq: Add DEV_MSI allocation type

[Megha Dey]

From: Thomas Gleixner 

For the upcoming device MSI support a new allocation type is
required.

Reviewed-by: Tony Luck 
Signed-off-by: Thomas Gleixner 
Signed-off-by: Megha Dey 
---
 arch/x86/include/asm/hw_irq.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/include/asm/hw_irq.h b/arch/x86/include/asm/hw_irq.h
index d465ece..0531b9c 100644
--- a/arch/x86/include/asm/hw_irq.h
+++ b/arch/x86/include/asm/hw_irq.h
@@ -41,6 +41,7 @@ enum irq_alloc_type {
X86_IRQ_ALLOC_TYPE_DMAR,
X86_IRQ_ALLOC_TYPE_AMDVI,
X86_IRQ_ALLOC_TYPE_UV,
+   X86_IRQ_ALLOC_TYPE_DEV_MSI,
 };
 
 struct ioapic_alloc_info {
-- 
2.7.4

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

Re: [PATCHv2 2/2] iommu/arm-smmu-qcom: Move the adreno smmu specific impl earlier

[Jordan Crouse]

On Fri, Feb 26, 2021 at 11:48:13AM -0700, Jordan Crouse wrote:
> On Fri, Feb 26, 2021 at 11:24:52AM -0600, Bjorn Andersson wrote:
> > On Fri 26 Feb 03:55 CST 2021, Sai Prakash Ranjan wrote:
> > 
> > > Adreno(GPU) SMMU and APSS(Application Processor SubSystem) SMMU
> > > both implement "arm,mmu-500" in some QTI SoCs and to run through
> > > adreno smmu specific implementation such as enabling split pagetables
> > > support, we need to match the "qcom,adreno-smmu" compatible first
> > > before apss smmu or else we will be running apps smmu implementation
> > > for adreno smmu and the additional features for adreno smmu is never
> > > set. For ex: we have "qcom,sc7280-smmu-500" compatible for both apps
> > > and adreno smmu implementing "arm,mmu-500", so the adreno smmu
> > > implementation is never reached because the current sequence checks
> > > for apps smmu compatible(qcom,sc7280-smmu-500) first and runs that
> > > specific impl and we never reach adreno smmu specific implementation.
> > > 
> > 
> > So you're saying that you have a single SMMU instance that's compatible
> > with both an entry in qcom_smmu_impl_of_match[] and "qcom,adreno-smmu"?
> > 
> > Per your proposed change we will pick the adreno ops _only_ for this
> > component, essentially disabling the non-Adreno quirks selected by the
> > qcom impl. As such keeping the non-adreno compatible in the
> > qcom_smmu_impl_init[] seems to only serve to obfuscate the situation.
> > 
> > Don't we somehow need the combined set of quirks? (At least if we're
> > running this with a standard UEFI based boot flow?)
> 
> We *do* need the combined set of quirks, so there has to be an adreno-smmu
> impelmentation that matches the "generic" implementation with a few extra
> function hooks added on. I'm not sure if there is a clever way to figure out 
> how
> to meld the implementation hooks at runtime but the alternative is to just 
> make
> sure that the adreno-smmu static struct calls the same quirks as its generic
> partner.

To clarify, the gpu-smmu doesn't strictly need the s2cr handoff or the cfg_probe
though it wouldn't hurt to have them since they would be essentially
passthroughs for the GPU. We do need to capture errata like the
sdm845_smmu500_reset which is already part of the upstream adreno
implementation.

I think the main takeaway is that if a new errata or quirk is added
for main mmu500 it needs to be considered for adreno-smmu too.

Jordan

> > > Suggested-by: Akhil P Oommen 
> > > Signed-off-by: Sai Prakash Ranjan 
> > > ---
> > >  drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 12 +---
> > >  1 file changed, 9 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c 
> > > b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> > > index bea3ee0dabc2..03f048aebb80 100644
> > > --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> > > +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> > > @@ -345,11 +345,17 @@ struct arm_smmu_device *qcom_smmu_impl_init(struct 
> > > arm_smmu_device *smmu)
> > >  {
> > >   const struct device_node *np = smmu->dev->of_node;
> > >  
> > > - if (of_match_node(qcom_smmu_impl_of_match, np))
> > > - return qcom_smmu_create(smmu, _smmu_impl);
> > > -
> > > + /*
> > > +  * Do not change this order of implementation, i.e., first adreno
> > > +  * smmu impl and then apss smmu since we can have both implementing
> > > +  * arm,mmu-500 in which case we will miss setting adreno smmu specific
> > > +  * features if the order is changed.
> > > +  */
> > >   if (of_device_is_compatible(np, "qcom,adreno-smmu"))
> > >   return qcom_smmu_create(smmu, _adreno_smmu_impl);
> > >  
> > > + if (of_match_node(qcom_smmu_impl_of_match, np))
> > > + return qcom_smmu_create(smmu, _smmu_impl);
> > > +
> > >   return smmu;
> > >  }
> > > -- 
> > > QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> > > of Code Aurora Forum, hosted by The Linux Foundation
> > > 
> > ___
> > iommu mailing list
> > iommu@lists.linux-foundation.org
> > https://lists.linuxfoundation.org/mailman/listinfo/iommu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCHv2 2/2] iommu/arm-smmu-qcom: Move the adreno smmu specific impl earlier

[Bjorn Andersson]

On Fri 26 Feb 12:23 CST 2021, Rob Clark wrote:

> On Fri, Feb 26, 2021 at 9:24 AM Bjorn Andersson
>  wrote:
> >
> > On Fri 26 Feb 03:55 CST 2021, Sai Prakash Ranjan wrote:
> >
> > > Adreno(GPU) SMMU and APSS(Application Processor SubSystem) SMMU
> > > both implement "arm,mmu-500" in some QTI SoCs and to run through
> > > adreno smmu specific implementation such as enabling split pagetables
> > > support, we need to match the "qcom,adreno-smmu" compatible first
> > > before apss smmu or else we will be running apps smmu implementation
> > > for adreno smmu and the additional features for adreno smmu is never
> > > set. For ex: we have "qcom,sc7280-smmu-500" compatible for both apps
> > > and adreno smmu implementing "arm,mmu-500", so the adreno smmu
> > > implementation is never reached because the current sequence checks
> > > for apps smmu compatible(qcom,sc7280-smmu-500) first and runs that
> > > specific impl and we never reach adreno smmu specific implementation.
> > >
> >
> > So you're saying that you have a single SMMU instance that's compatible
> > with both an entry in qcom_smmu_impl_of_match[] and "qcom,adreno-smmu"?
> >
> > Per your proposed change we will pick the adreno ops _only_ for this
> > component, essentially disabling the non-Adreno quirks selected by the
> > qcom impl. As such keeping the non-adreno compatible in the
> > qcom_smmu_impl_init[] seems to only serve to obfuscate the situation.
> >
> > Don't we somehow need the combined set of quirks? (At least if we're
> > running this with a standard UEFI based boot flow?)
> >
> 
> are you thinking of the apps-smmu handover of display context bank?
> That shouldn't change, the only thing that changes is that gpu-smmu
> becomes an mmu-500, whereas previously only apps-smmu was..
> 

The current logic picks one of:
1) is the compatible mentioned in qcom_smmu_impl_of_match[]
2) is the compatible an adreno
3) no quirks needed

The change flips the order of these, so the only way I can see this
change affecting things is if we expected a match on #2, but we got one
on #1.

Which implies that the instance that we want to act according to the
adreno impl was listed in qcom_smmu_impl_of_match[] - which either is
wrong, or there's a single instance that needs both behaviors.

(And I believe Jordan's answer confirms the latter - there's a single
SMMU instance that needs all them quirks at once)

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

Re: [PATCHv2 2/2] iommu/arm-smmu-qcom: Move the adreno smmu specific impl earlier

[Jordan Crouse]

On Fri, Feb 26, 2021 at 11:24:52AM -0600, Bjorn Andersson wrote:
> On Fri 26 Feb 03:55 CST 2021, Sai Prakash Ranjan wrote:
> 
> > Adreno(GPU) SMMU and APSS(Application Processor SubSystem) SMMU
> > both implement "arm,mmu-500" in some QTI SoCs and to run through
> > adreno smmu specific implementation such as enabling split pagetables
> > support, we need to match the "qcom,adreno-smmu" compatible first
> > before apss smmu or else we will be running apps smmu implementation
> > for adreno smmu and the additional features for adreno smmu is never
> > set. For ex: we have "qcom,sc7280-smmu-500" compatible for both apps
> > and adreno smmu implementing "arm,mmu-500", so the adreno smmu
> > implementation is never reached because the current sequence checks
> > for apps smmu compatible(qcom,sc7280-smmu-500) first and runs that
> > specific impl and we never reach adreno smmu specific implementation.
> > 
> 
> So you're saying that you have a single SMMU instance that's compatible
> with both an entry in qcom_smmu_impl_of_match[] and "qcom,adreno-smmu"?
> 
> Per your proposed change we will pick the adreno ops _only_ for this
> component, essentially disabling the non-Adreno quirks selected by the
> qcom impl. As such keeping the non-adreno compatible in the
> qcom_smmu_impl_init[] seems to only serve to obfuscate the situation.
> 
> Don't we somehow need the combined set of quirks? (At least if we're
> running this with a standard UEFI based boot flow?)

We *do* need the combined set of quirks, so there has to be an adreno-smmu
impelmentation that matches the "generic" implementation with a few extra
function hooks added on. I'm not sure if there is a clever way to figure out how
to meld the implementation hooks at runtime but the alternative is to just make
sure that the adreno-smmu static struct calls the same quirks as its generic
partner.

Jordan

> > Suggested-by: Akhil P Oommen 
> > Signed-off-by: Sai Prakash Ranjan 
> > ---
> >  drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 12 +---
> >  1 file changed, 9 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c 
> > b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> > index bea3ee0dabc2..03f048aebb80 100644
> > --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> > +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> > @@ -345,11 +345,17 @@ struct arm_smmu_device *qcom_smmu_impl_init(struct 
> > arm_smmu_device *smmu)
> >  {
> > const struct device_node *np = smmu->dev->of_node;
> >  
> > -   if (of_match_node(qcom_smmu_impl_of_match, np))
> > -   return qcom_smmu_create(smmu, _smmu_impl);
> > -
> > +   /*
> > +* Do not change this order of implementation, i.e., first adreno
> > +* smmu impl and then apss smmu since we can have both implementing
> > +* arm,mmu-500 in which case we will miss setting adreno smmu specific
> > +* features if the order is changed.
> > +*/
> > if (of_device_is_compatible(np, "qcom,adreno-smmu"))
> > return qcom_smmu_create(smmu, _adreno_smmu_impl);
> >  
> > +   if (of_match_node(qcom_smmu_impl_of_match, np))
> > +   return qcom_smmu_create(smmu, _smmu_impl);
> > +
> > return smmu;
> >  }
> > -- 
> > QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> > of Code Aurora Forum, hosted by The Linux Foundation
> > 
> ___
> iommu mailing list
> iommu@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCHv2 2/2] iommu/arm-smmu-qcom: Move the adreno smmu specific impl earlier

[Rob Clark]

On Fri, Feb 26, 2021 at 9:24 AM Bjorn Andersson
 wrote:
>
> On Fri 26 Feb 03:55 CST 2021, Sai Prakash Ranjan wrote:
>
> > Adreno(GPU) SMMU and APSS(Application Processor SubSystem) SMMU
> > both implement "arm,mmu-500" in some QTI SoCs and to run through
> > adreno smmu specific implementation such as enabling split pagetables
> > support, we need to match the "qcom,adreno-smmu" compatible first
> > before apss smmu or else we will be running apps smmu implementation
> > for adreno smmu and the additional features for adreno smmu is never
> > set. For ex: we have "qcom,sc7280-smmu-500" compatible for both apps
> > and adreno smmu implementing "arm,mmu-500", so the adreno smmu
> > implementation is never reached because the current sequence checks
> > for apps smmu compatible(qcom,sc7280-smmu-500) first and runs that
> > specific impl and we never reach adreno smmu specific implementation.
> >
>
> So you're saying that you have a single SMMU instance that's compatible
> with both an entry in qcom_smmu_impl_of_match[] and "qcom,adreno-smmu"?
>
> Per your proposed change we will pick the adreno ops _only_ for this
> component, essentially disabling the non-Adreno quirks selected by the
> qcom impl. As such keeping the non-adreno compatible in the
> qcom_smmu_impl_init[] seems to only serve to obfuscate the situation.
>
> Don't we somehow need the combined set of quirks? (At least if we're
> running this with a standard UEFI based boot flow?)
>

are you thinking of the apps-smmu handover of display context bank?
That shouldn't change, the only thing that changes is that gpu-smmu
becomes an mmu-500, whereas previously only apps-smmu was..

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

Re: [PATCHv2 2/2] iommu/arm-smmu-qcom: Move the adreno smmu specific impl earlier

[Bjorn Andersson]

On Fri 26 Feb 03:55 CST 2021, Sai Prakash Ranjan wrote:

> Adreno(GPU) SMMU and APSS(Application Processor SubSystem) SMMU
> both implement "arm,mmu-500" in some QTI SoCs and to run through
> adreno smmu specific implementation such as enabling split pagetables
> support, we need to match the "qcom,adreno-smmu" compatible first
> before apss smmu or else we will be running apps smmu implementation
> for adreno smmu and the additional features for adreno smmu is never
> set. For ex: we have "qcom,sc7280-smmu-500" compatible for both apps
> and adreno smmu implementing "arm,mmu-500", so the adreno smmu
> implementation is never reached because the current sequence checks
> for apps smmu compatible(qcom,sc7280-smmu-500) first and runs that
> specific impl and we never reach adreno smmu specific implementation.
> 

So you're saying that you have a single SMMU instance that's compatible
with both an entry in qcom_smmu_impl_of_match[] and "qcom,adreno-smmu"?

Per your proposed change we will pick the adreno ops _only_ for this
component, essentially disabling the non-Adreno quirks selected by the
qcom impl. As such keeping the non-adreno compatible in the
qcom_smmu_impl_init[] seems to only serve to obfuscate the situation.

Don't we somehow need the combined set of quirks? (At least if we're
running this with a standard UEFI based boot flow?)

Regards,
Bjorn

> Suggested-by: Akhil P Oommen 
> Signed-off-by: Sai Prakash Ranjan 
> ---
>  drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 12 +---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c 
> b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> index bea3ee0dabc2..03f048aebb80 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> @@ -345,11 +345,17 @@ struct arm_smmu_device *qcom_smmu_impl_init(struct 
> arm_smmu_device *smmu)
>  {
>   const struct device_node *np = smmu->dev->of_node;
>  
> - if (of_match_node(qcom_smmu_impl_of_match, np))
> - return qcom_smmu_create(smmu, _smmu_impl);
> -
> + /*
> +  * Do not change this order of implementation, i.e., first adreno
> +  * smmu impl and then apss smmu since we can have both implementing
> +  * arm,mmu-500 in which case we will miss setting adreno smmu specific
> +  * features if the order is changed.
> +  */
>   if (of_device_is_compatible(np, "qcom,adreno-smmu"))
>   return qcom_smmu_create(smmu, _adreno_smmu_impl);
>  
> + if (of_match_node(qcom_smmu_impl_of_match, np))
> + return qcom_smmu_create(smmu, _smmu_impl);
> +
>   return smmu;
>  }
> -- 
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation
> 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v12 10/10] iommu/arm-smmu-v3: Add stall support for platform devices

[Jean-Philippe Brucker]

Hi Zhou,

On Fri, Feb 26, 2021 at 05:43:27PM +0800, Zhou Wang wrote:
> On 2021/2/1 19:14, Jean-Philippe Brucker wrote:
> > Hi Zhou,
> > 
> > On Mon, Feb 01, 2021 at 09:18:42AM +0800, Zhou Wang wrote:
> >>> @@ -1033,8 +1076,7 @@ int arm_smmu_write_ctx_desc(struct arm_smmu_domain 
> >>> *smmu_domain, int ssid,
> >>>   FIELD_PREP(CTXDESC_CD_0_ASID, cd->asid) |
> >>>   CTXDESC_CD_0_V;
> >>>  
> >>> - /* STALL_MODEL==0b10 && CD.S==0 is ILLEGAL */
> >>> - if (smmu->features & ARM_SMMU_FEAT_STALL_FORCE)
> >>> + if (smmu_domain->stall_enabled)
> >>
> >> Could we add ssid checking here? like: if (smmu_domain->stall_enabled && 
> >> ssid).
> >> The reason is if not CD.S will also be set when ssid is 0, which is not 
> >> needed.
> > 
> > Some drivers may want to get stall events on SSID 0:
> > https://lore.kernel.org/kvm/20210125090402.1429-1-lushenm...@huawei.com/#t
> > 
> > Are you seeing an issue with stall events on ssid 0?  Normally there
> > shouldn't be any fault on this context, but if they happen and no handler
> > is registered, the SMMU driver will just abort them and report them like a
> > non-stall event.
> 
> Hi Jean,
> 
> I notice that there is problem. In my case, I expect that CD0 is for kernel
> and other CDs are for user space. Normally there shouldn't be any fault in
> kernel, however, we have RAS case which is for some reason there may has
> invalid address access from hardware device.
> 
> So at least there are two different address access failures: 1. hardware RAS 
> problem;
> 2. software fault fail(e.g. kill process when doing DMA). Handlings for these
> two are different: for 1, we should reset hardware device; for 2, stop related
> DMA is enough.

Right, and in case 2 there should be no report printed since it can be
triggered by user, while you probably want to be loud in case 1.

> Currently if SMMU returns the same signal(by SMMU resume abort), master device
> driver can not tell these two kinds of cases.

This part I don't understand. So the SMMU sends a RESUME(abort) command,
and then the master reports the DMA error to the device driver, which
cannot differentiate 1 from 2?  (I guess there is no SSID in this report?)
But how does disabling stall change this?  The invalid DMA access will
still be aborted by the SMMU.

Hypothetically, would it work if all stall events that could not be
handled went to the device driver?  Those reports would contain the SSID
(or lack thereof), so you could reset the device in case 1 and ignore case
2. Though resetting the device in the middle of a stalled transaction
probably comes with its own set of problems.

> From the basic concept, if a CD is used for kernel, its S bit should not be 
> set.
> How about we add iommu domain check here too, if DMA domain we do not set S 
> bit for
> CD0, if unmanaged domain we set S bit for all CDs?

I think disabling stall for CD0 of a DMA domain makes sense in general,
even though I don't really understand how that fixes your issue. But
someone might come up with a good use-case for receiving stall events on
DMA mappings, so I'm wondering whether the alternative solution where we
report unhandled stall events to the device driver would also work for
you.

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

Re: [PATCHv2 2/2] iommu/arm-smmu-qcom: Move the adreno smmu specific impl earlier

[Jordan Crouse]

On Fri, Feb 26, 2021 at 03:25:40PM +0530, Sai Prakash Ranjan wrote:
> Adreno(GPU) SMMU and APSS(Application Processor SubSystem) SMMU
> both implement "arm,mmu-500" in some QTI SoCs and to run through
> adreno smmu specific implementation such as enabling split pagetables
> support, we need to match the "qcom,adreno-smmu" compatible first
> before apss smmu or else we will be running apps smmu implementation
> for adreno smmu and the additional features for adreno smmu is never
> set. For ex: we have "qcom,sc7280-smmu-500" compatible for both apps
> and adreno smmu implementing "arm,mmu-500", so the adreno smmu
> implementation is never reached because the current sequence checks
> for apps smmu compatible(qcom,sc7280-smmu-500) first and runs that
> specific impl and we never reach adreno smmu specific implementation.
> 
> Suggested-by: Akhil P Oommen 

Acked-by: Jordan Crouse 

> Signed-off-by: Sai Prakash Ranjan 
> ---
>  drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 12 +---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c 
> b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> index bea3ee0dabc2..03f048aebb80 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> @@ -345,11 +345,17 @@ struct arm_smmu_device *qcom_smmu_impl_init(struct 
> arm_smmu_device *smmu)
>  {
>   const struct device_node *np = smmu->dev->of_node;
>  
> - if (of_match_node(qcom_smmu_impl_of_match, np))
> - return qcom_smmu_create(smmu, _smmu_impl);
> -
> + /*
> +  * Do not change this order of implementation, i.e., first adreno
> +  * smmu impl and then apss smmu since we can have both implementing
> +  * arm,mmu-500 in which case we will miss setting adreno smmu specific
> +  * features if the order is changed.
> +  */
>   if (of_device_is_compatible(np, "qcom,adreno-smmu"))
>   return qcom_smmu_create(smmu, _adreno_smmu_impl);
>  
> + if (of_match_node(qcom_smmu_impl_of_match, np))
> + return qcom_smmu_create(smmu, _smmu_impl);
> +
>   return smmu;
>  }
> -- 
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation
> 
> ___
> iommu mailing list
> iommu@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCHv2 2/2] iommu/arm-smmu-qcom: Move the adreno smmu specific impl earlier

[Sai Prakash Ranjan]

Adreno(GPU) SMMU and APSS(Application Processor SubSystem) SMMU
both implement "arm,mmu-500" in some QTI SoCs and to run through
adreno smmu specific implementation such as enabling split pagetables
support, we need to match the "qcom,adreno-smmu" compatible first
before apss smmu or else we will be running apps smmu implementation
for adreno smmu and the additional features for adreno smmu is never
set. For ex: we have "qcom,sc7280-smmu-500" compatible for both apps
and adreno smmu implementing "arm,mmu-500", so the adreno smmu
implementation is never reached because the current sequence checks
for apps smmu compatible(qcom,sc7280-smmu-500) first and runs that
specific impl and we never reach adreno smmu specific implementation.

Suggested-by: Akhil P Oommen 
Signed-off-by: Sai Prakash Ranjan 
---
 drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c 
b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
index bea3ee0dabc2..03f048aebb80 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
@@ -345,11 +345,17 @@ struct arm_smmu_device *qcom_smmu_impl_init(struct 
arm_smmu_device *smmu)
 {
const struct device_node *np = smmu->dev->of_node;
 
-   if (of_match_node(qcom_smmu_impl_of_match, np))
-   return qcom_smmu_create(smmu, _smmu_impl);
-
+   /*
+* Do not change this order of implementation, i.e., first adreno
+* smmu impl and then apss smmu since we can have both implementing
+* arm,mmu-500 in which case we will miss setting adreno smmu specific
+* features if the order is changed.
+*/
if (of_device_is_compatible(np, "qcom,adreno-smmu"))
return qcom_smmu_create(smmu, _adreno_smmu_impl);
 
+   if (of_match_node(qcom_smmu_impl_of_match, np))
+   return qcom_smmu_create(smmu, _smmu_impl);
+
return smmu;
 }
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

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

[PATCHv2 1/2] iommu/arm-smmu-qcom: Add SC7280 SMMU compatible

[Sai Prakash Ranjan]

Add compatible for SC7280 SMMU to use the Qualcomm Technologies, Inc.
specific implementation.

Signed-off-by: Sai Prakash Ranjan 
---
 drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c 
b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
index 98b3a1c2a181..bea3ee0dabc2 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
@@ -166,6 +166,7 @@ static const struct of_device_id 
qcom_smmu_client_of_match[] __maybe_unused = {
{ .compatible = "qcom,mdss" },
{ .compatible = "qcom,sc7180-mdss" },
{ .compatible = "qcom,sc7180-mss-pil" },
+   { .compatible = "qcom,sc7280-mdss" },
{ .compatible = "qcom,sc8180x-mdss" },
{ .compatible = "qcom,sdm845-mdss" },
{ .compatible = "qcom,sdm845-mss-pil" },
@@ -330,6 +331,7 @@ static struct arm_smmu_device *qcom_smmu_create(struct 
arm_smmu_device *smmu,
 static const struct of_device_id __maybe_unused qcom_smmu_impl_of_match[] = {
{ .compatible = "qcom,msm8998-smmu-v2" },
{ .compatible = "qcom,sc7180-smmu-500" },
+   { .compatible = "qcom,sc7280-smmu-500" },
{ .compatible = "qcom,sc8180x-smmu-500" },
{ .compatible = "qcom,sdm630-smmu-v2" },
{ .compatible = "qcom,sdm845-smmu-500" },
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

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

[PATCHv2 0/2] iommu/arm-smmu-qcom: Add SC7280 support

[Sai Prakash Ranjan]

Patch 1 adds the sc7280 smmu compatible.
Patch 2 moves the adreno smmu check before apss smmu to enable
adreno smmu specific implementation.

Changes in v2:
 * Add a comment to make sure this order is not changed in future (Jordan)

Sai Prakash Ranjan (2):
  iommu/arm-smmu-qcom: Add SC7280 SMMU compatible
  iommu/arm-smmu-qcom: Move the adreno smmu specific impl earlier

 drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 14 +++---
 1 file changed, 11 insertions(+), 3 deletions(-)


base-commit: 7060377ce06f9cd3ed6274c0f2310463feb5baec
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

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

Re: [PATCH v12 10/10] iommu/arm-smmu-v3: Add stall support for platform devices

[Zhou Wang]

On 2021/2/1 19:14, Jean-Philippe Brucker wrote:
> Hi Zhou,
> 
> On Mon, Feb 01, 2021 at 09:18:42AM +0800, Zhou Wang wrote:
>>> @@ -1033,8 +1076,7 @@ int arm_smmu_write_ctx_desc(struct arm_smmu_domain 
>>> *smmu_domain, int ssid,
>>> FIELD_PREP(CTXDESC_CD_0_ASID, cd->asid) |
>>> CTXDESC_CD_0_V;
>>>  
>>> -   /* STALL_MODEL==0b10 && CD.S==0 is ILLEGAL */
>>> -   if (smmu->features & ARM_SMMU_FEAT_STALL_FORCE)
>>> +   if (smmu_domain->stall_enabled)
>>
>> Could we add ssid checking here? like: if (smmu_domain->stall_enabled && 
>> ssid).
>> The reason is if not CD.S will also be set when ssid is 0, which is not 
>> needed.
> 
> Some drivers may want to get stall events on SSID 0:
> https://lore.kernel.org/kvm/20210125090402.1429-1-lushenm...@huawei.com/#t
> 
> Are you seeing an issue with stall events on ssid 0?  Normally there
> shouldn't be any fault on this context, but if they happen and no handler
> is registered, the SMMU driver will just abort them and report them like a
> non-stall event.

Hi Jean,

I notice that there is problem. In my case, I expect that CD0 is for kernel
and other CDs are for user space. Normally there shouldn't be any fault in
kernel, however, we have RAS case which is for some reason there may has
invalid address access from hardware device.

So at least there are two different address access failures: 1. hardware RAS 
problem;
2. software fault fail(e.g. kill process when doing DMA). Handlings for these
two are different: for 1, we should reset hardware device; for 2, stop related
DMA is enough.

Currently if SMMU returns the same signal(by SMMU resume abort), master device
driver can not tell these two kinds of cases.

>From the basic concept, if a CD is used for kernel, its S bit should not be 
>set.
How about we add iommu domain check here too, if DMA domain we do not set S bit 
for
CD0, if unmanaged domain we set S bit for all CDs?

Thanks,
Zhou

> 
> Thanks,
> Jean
> 
> .
> 

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

Re: [PATCH v4 12/14] swiotlb: Add restricted DMA alloc/free support.

[Claire Chang]

On Fri, Feb 26, 2021 at 1:17 PM Christoph Hellwig  wrote:
>
> On Fri, Feb 26, 2021 at 12:17:50PM +0800, Claire Chang wrote:
> > Do you think I should fix this and rebase on the latest linux-next
> > now? I wonder if there are more factor and clean up coming and I
> > should wait after that.
>
> Here is my preferred plan:
>
>  1) wait for my series to support the min alignment in swiotlb to
> land in Linus tree
>  2) I'll resend my series with the further swiotlb cleanup and
> refactoring, which includes a slightly rebased version of your
> patch to add the io_tlb_mem structure
>  3) resend your series on top of that as a baseline
>
> This is my current WIP tree for 2:
>
>   
> http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/swiotlb-struct

Sounds good to me. Thanks!
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

RE: next/master bisection: baseline.login on r8a77960-ulcb

[Yoshihiro Shimoda]

Hi all,

> From: Heiko Thiery, Sent: Thursday, February 25, 2021 10:01 PM
> Am Do., 25. Feb. 2021 um 12:50 Uhr schrieb Thierry Reding:
> > On Thu, Feb 25, 2021 at 11:14:57AM +, Robin Murphy wrote:
> > > On 2021-02-25 11:09, Thierry Reding wrote:
> > > > On Wed, Feb 24, 2021 at 10:39:42PM +0100, Heiko Thiery wrote:
> > > > > Hi Christoph and all,
> > > > >
> > > > > On 23.02.21 10:56, Guillaume Tucker wrote:
> > > > > > Hi Christoph,
> > > > > >
> > > > > > Please see the bisection report below about a boot failure on
> > > > > > r8a77960-ulcb on next-20210222.
> > > > > >
> > > > > > Reports aren't automatically sent to the public while we're
> > > > > > trialing new bisection features on kernelci.org but this one
> > > > > > looks valid.
> > > > > >
> > > > > > The log shows a kernel panic, more details can be found here:

> >
> > Yep, changing max_slots from unsigned int to unsigned long fixes this as
> > well. Thanks for the pointer!
> 
> I also can confirm that changing that to unsigned long fixes the issue.

Thank you for the information! I also confirmed that changing the type of
max_slots fixed the issue on my environment (r8a77951-salvator-xs.dts with 
defconfig).

Best regards,
Yoshihiro Shimoda

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

Re: [PATCH 2/2] iommu/arm-smmu-qcom: Move the adreno smmu specific impl earlier

[Sai Prakash Ranjan]

On 2021-02-25 23:36, Jordan Crouse wrote:

On Thu, Feb 25, 2021 at 03:54:10PM +0530, Sai Prakash Ranjan wrote:

Adreno(GPU) SMMU and APSS(Application Processor SubSystem) SMMU
both implement "arm,mmu-500" in some QTI SoCs and to run through
adreno smmu specific implementation such as enabling split pagetables
support, we need to match the "qcom,adreno-smmu" compatible first
before apss smmu or else we will be running apps smmu implementation
for adreno smmu and the additional features for adreno smmu is never
set. For ex: we have "qcom,sc7280-smmu-500" compatible for both apps
and adreno smmu implementing "arm,mmu-500", so the adreno smmu
implementation is never reached because the current sequence checks
for apps smmu compatible(qcom,sc7280-smmu-500) first and runs that
specific impl and we never reach adreno smmu specific implementation.

Suggested-by: Akhil P Oommen 
Signed-off-by: Sai Prakash Ranjan 
---

Its either this or we add a new compatible for adreno smmu 
implementing

arm,mmu-500 like "qcom,sc7280-adreno-smmu-500".

---
 drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c 
b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c

index bea3ee0dabc2..7d0fc2c8e72f 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
@@ -345,11 +345,11 @@ struct arm_smmu_device 
*qcom_smmu_impl_init(struct arm_smmu_device *smmu)

 {
const struct device_node *np = smmu->dev->of_node;

-   if (of_match_node(qcom_smmu_impl_of_match, np))
-   return qcom_smmu_create(smmu, _smmu_impl);
-
if (of_device_is_compatible(np, "qcom,adreno-smmu"))
return qcom_smmu_create(smmu, _adreno_smmu_impl);

+   if (of_match_node(qcom_smmu_impl_of_match, np))
+   return qcom_smmu_create(smmu, _smmu_impl);
+


It would be good to add a comment here explaining the order here so we
don't accidentally reorganize ourselves back into a problem later.



Sure its better, will add it.

Thanks,
Sai

--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a 
member

of Code Aurora Forum, hosted by The Linux Foundation
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu