Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1590965 > unrolled thread
| Started by | Auger Eric <eric.auger@redhat.com> |
|---|---|
| First post | 2017-03-02 11:30 +0100 |
| Last post | 2017-03-02 17:00 +0100 |
| Articles | 4 — 2 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
Re: [PATCH 2/2] vfio: type1: conditionally check MSI remapping at irq domain level Auger Eric <eric.auger@redhat.com> - 2017-03-02 11:30 +0100
Re: [PATCH 2/2] vfio: type1: conditionally check MSI remapping at irq domain level Mian Yousaf Kaukab <yousaf.kaukab@suse.com> - 2017-03-02 13:50 +0100
Re: [PATCH 2/2] vfio: type1: conditionally check MSI remapping at irq domain level Auger Eric <eric.auger@redhat.com> - 2017-03-02 14:50 +0100
Re: [PATCH 2/2] vfio: type1: conditionally check MSI remapping at irq domain level Mian Yousaf Kaukab <yousaf.kaukab@suse.com> - 2017-03-02 17:00 +0100
| From | Auger Eric <eric.auger@redhat.com> |
|---|---|
| Date | 2017-03-02 11:30 +0100 |
| Subject | Re: [PATCH 2/2] vfio: type1: conditionally check MSI remapping at irq domain level |
| Message-ID | <tgw9H-Sh-7@gated-at.bofh.it> |
Hi,
On 02/03/2017 11:01, Mian Yousaf Kaukab wrote:
> Check only if irq domains are available.
>
> Signed-off-by: Mian Yousaf Kaukab <yousaf.kaukab@suse.com>
> ---
> drivers/vfio/vfio_iommu_type1.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index bd6f293c4ebd..e3ed50e40ead 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -1287,8 +1287,9 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
> INIT_LIST_HEAD(&domain->group_list);
> list_add(&group->next, &domain->group_list);
>
> - msi_remap = resv_msi ? irq_domain_check_msi_remap() :
> - iommu_capable(bus, IOMMU_CAP_INTR_REMAP);
> + msi_remap = resv_msi && IS_ENABLED(CONFIG_IRQ_DOMAIN) ?
> + irq_domain_check_msi_remap() :
> + iommu_capable(bus, IOMMU_CAP_INTR_REMAP);
Is that patch actually needed after [PATCH 1/2] irqdomain: add empty
irq_domain_check_msi_remap. irq_domain_check_msi_remap() should be
defined and if you follow my suggestion, would return false. Anyway in
your case resv_msi should be false.
Thanks
Eric
>
> if (!allow_unsafe_interrupts && !msi_remap) {
> pr_warn("%s: No interrupt remapping support. Use the module param \"allow_unsafe_interrupts\" to enable VFIO IOMMU support on this platform\n",
>
[toc] | [next] | [standalone]
| From | Mian Yousaf Kaukab <yousaf.kaukab@suse.com> |
|---|---|
| Date | 2017-03-02 13:50 +0100 |
| Message-ID | <tgylc-2jr-25@gated-at.bofh.it> |
| In reply to | #1590965 |
On 03/02/2017 11:24 AM, Auger Eric wrote: > Hi, > > On 02/03/2017 11:01, Mian Yousaf Kaukab wrote: >> Check only if irq domains are available. >> >> Signed-off-by: Mian Yousaf Kaukab <yousaf.kaukab@suse.com> >> --- >> drivers/vfio/vfio_iommu_type1.c | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c >> index bd6f293c4ebd..e3ed50e40ead 100644 >> --- a/drivers/vfio/vfio_iommu_type1.c >> +++ b/drivers/vfio/vfio_iommu_type1.c >> @@ -1287,8 +1287,9 @@ static int vfio_iommu_type1_attach_group(void *iommu_data, >> INIT_LIST_HEAD(&domain->group_list); >> list_add(&group->next, &domain->group_list); >> >> - msi_remap = resv_msi ? irq_domain_check_msi_remap() : >> - iommu_capable(bus, IOMMU_CAP_INTR_REMAP); >> + msi_remap = resv_msi && IS_ENABLED(CONFIG_IRQ_DOMAIN) ? >> + irq_domain_check_msi_remap() : >> + iommu_capable(bus, IOMMU_CAP_INTR_REMAP); > Is that patch actually needed after [PATCH 1/2] irqdomain: add empty > irq_domain_check_msi_remap. irq_domain_check_msi_remap() should be > defined and if you follow my suggestion, would return false. Anyway in > your case resv_msi should be false. I agree its an overkill if resv_msi is guaranteed to be false. What I am unsure about is that, if iommu have IOMMU_RESV_MSI regions that would mean that irq domains are selected in the build. If this is not guaranteed, then we need to add this check. > > Thanks > > Eric BR, Yousaf
[toc] | [prev] | [next] | [standalone]
| From | Auger Eric <eric.auger@redhat.com> |
|---|---|
| Date | 2017-03-02 14:50 +0100 |
| Message-ID | <tgzhg-2VJ-13@gated-at.bofh.it> |
| In reply to | #1591046 |
Hi, On 02/03/2017 13:38, Mian Yousaf Kaukab wrote: > On 03/02/2017 11:24 AM, Auger Eric wrote: >> Hi, >> >> On 02/03/2017 11:01, Mian Yousaf Kaukab wrote: >>> Check only if irq domains are available. >>> >>> Signed-off-by: Mian Yousaf Kaukab <yousaf.kaukab@suse.com> >>> --- >>> drivers/vfio/vfio_iommu_type1.c | 5 +++-- >>> 1 file changed, 3 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/vfio/vfio_iommu_type1.c >>> b/drivers/vfio/vfio_iommu_type1.c >>> index bd6f293c4ebd..e3ed50e40ead 100644 >>> --- a/drivers/vfio/vfio_iommu_type1.c >>> +++ b/drivers/vfio/vfio_iommu_type1.c >>> @@ -1287,8 +1287,9 @@ static int vfio_iommu_type1_attach_group(void >>> *iommu_data, >>> INIT_LIST_HEAD(&domain->group_list); >>> list_add(&group->next, &domain->group_list); >>> - msi_remap = resv_msi ? irq_domain_check_msi_remap() : >>> - iommu_capable(bus, IOMMU_CAP_INTR_REMAP); >>> + msi_remap = resv_msi && IS_ENABLED(CONFIG_IRQ_DOMAIN) ? >>> + irq_domain_check_msi_remap() : >>> + iommu_capable(bus, IOMMU_CAP_INTR_REMAP); >> Is that patch actually needed after [PATCH 1/2] irqdomain: add empty >> irq_domain_check_msi_remap. irq_domain_check_msi_remap() should be >> defined and if you follow my suggestion, would return false. Anyway in >> your case resv_msi should be false. > I agree its an overkill if resv_msi is guaranteed to be false. What I am > unsure about is that, if iommu have IOMMU_RESV_MSI regions that would > mean that irq domains are selected in the build. If this is not > guaranteed, then we need to add this check. Currently only ARM SMMUs advertise IOMMU_RESV_MSI regions. If attempting to do passthrough on an ARM platform not implementing IRQ_DOMAIN the unsafe IRQ assignment mode would need to be chosen (if irq_domain_check_msi_remap() returns false as discussed before). Anyway checking the interrupt remapping capability on IOMMU side would report false as well since the capability is not exposed by ARM SMMU anymore. Thanks Eric > >> >> Thanks >> >> Eric > > BR, > Yousaf
[toc] | [prev] | [next] | [standalone]
| From | Mian Yousaf Kaukab <yousaf.kaukab@suse.com> |
|---|---|
| Date | 2017-03-02 17:00 +0100 |
| Message-ID | <tgBj4-4tT-17@gated-at.bofh.it> |
| In reply to | #1591085 |
On 03/02/2017 02:46 PM, Auger Eric wrote: > Hi, > > On 02/03/2017 13:38, Mian Yousaf Kaukab wrote: >> On 03/02/2017 11:24 AM, Auger Eric wrote: >>> Hi, >>> >>> On 02/03/2017 11:01, Mian Yousaf Kaukab wrote: >>>> Check only if irq domains are available. >>>> >>>> Signed-off-by: Mian Yousaf Kaukab <yousaf.kaukab@suse.com> >>>> --- >>>> drivers/vfio/vfio_iommu_type1.c | 5 +++-- >>>> 1 file changed, 3 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/vfio/vfio_iommu_type1.c >>>> b/drivers/vfio/vfio_iommu_type1.c >>>> index bd6f293c4ebd..e3ed50e40ead 100644 >>>> --- a/drivers/vfio/vfio_iommu_type1.c >>>> +++ b/drivers/vfio/vfio_iommu_type1.c >>>> @@ -1287,8 +1287,9 @@ static int vfio_iommu_type1_attach_group(void >>>> *iommu_data, >>>> INIT_LIST_HEAD(&domain->group_list); >>>> list_add(&group->next, &domain->group_list); >>>> - msi_remap = resv_msi ? irq_domain_check_msi_remap() : >>>> - iommu_capable(bus, IOMMU_CAP_INTR_REMAP); >>>> + msi_remap = resv_msi && IS_ENABLED(CONFIG_IRQ_DOMAIN) ? >>>> + irq_domain_check_msi_remap() : >>>> + iommu_capable(bus, IOMMU_CAP_INTR_REMAP); >>> Is that patch actually needed after [PATCH 1/2] irqdomain: add empty >>> irq_domain_check_msi_remap. irq_domain_check_msi_remap() should be >>> defined and if you follow my suggestion, would return false. Anyway in >>> your case resv_msi should be false. >> I agree its an overkill if resv_msi is guaranteed to be false. What I am >> unsure about is that, if iommu have IOMMU_RESV_MSI regions that would >> mean that irq domains are selected in the build. If this is not >> guaranteed, then we need to add this check. > > Currently only ARM SMMUs advertise IOMMU_RESV_MSI regions. If attempting > to do passthrough on an ARM platform not implementing IRQ_DOMAIN the > unsafe IRQ assignment mode would need to be chosen (if > irq_domain_check_msi_remap() returns false as discussed before). Anyway > checking the interrupt remapping capability on IOMMU side would report > false as well since the capability is not exposed by ARM SMMU anymore.OK I will drop this patch. We get a warning anyway in case the capability is checked from the wrong place. BR, Yousaf
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web