From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1032465AbdAFE1f (ORCPT ); Thu, 5 Jan 2017 23:27:35 -0500 Received: from mail-he1eur01on0040.outbound.protection.outlook.com ([104.47.0.40]:13792 "EHLO EUR01-HE1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S939482AbdAFE1S (ORCPT ); Thu, 5 Jan 2017 23:27:18 -0500 From: Bharat Bhushan To: Auger Eric , Marc Zyngier , "eric.auger.pro@gmail.com" , "christoffer.dall@linaro.org" , "robin.murphy@arm.com" , "alex.williamson@redhat.com" , "will.deacon@arm.com" , "joro@8bytes.org" , "tglx@linutronix.de" , "jason@lakedaemon.net" , "linux-arm-kernel@lists.infradead.org" CC: "drjones@redhat.com" , "kvm@vger.kernel.org" , "punit.agrawal@arm.com" , "linux-kernel@vger.kernel.org" , "geethasowjanya.akula@gmail.com" , "Diana Madalina Craciun" , "iommu@lists.linux-foundation.org" , "pranav.sawargaonkar@gmail.com" , "shankerd@codeaurora.org" , "gpkulkarni@gmail.com" Subject: RE: [PATCH v5 13/17] irqdomain: irq_domain_check_msi_remap Thread-Topic: [PATCH v5 13/17] irqdomain: irq_domain_check_msi_remap Thread-Index: AQHSZo8oGuvUagl5lEmNKWeg1UFzEKEoVL8AgAAHJQCAABT8AIABQ46AgAALSACAAAE1gIAAB9WAgAADCACAAQ+i4A== Date: Fri, 6 Jan 2017 04:27:03 +0000 Message-ID: References: <1483536746-2725-1-git-send-email-eric.auger@redhat.com> <1483536746-2725-14-git-send-email-eric.auger@redhat.com> <92fcb464-3364-c432-29d8-1c2ce4a5f110@redhat.com> <1e881510-bc18-ae97-ec2f-feadcba54150@redhat.com> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=bharat.bhushan@nxp.com; x-originating-ip: [192.88.169.1] x-ms-office365-filtering-correlation-id: c8bb027a-27e0-4c36-669e-08d435ec4433 x-microsoft-antispam: UriScan:;BCL:0;PCL:0;RULEID:(22001);SRVR:VI1PR04MB1325; x-microsoft-exchange-diagnostics: 1;VI1PR04MB1325;7:kvAFsLIbzrMpriwwVNlg6tAdKRgelvPDyxztWxVxmvd6G024ezPiHHZ3L9hqGH7l66qjN6wT6HYkVs+8oMLXGbmxMVpo1tZAwHTFBdbiJuBVcQ3J5cIiJuojK+oyogeB8mKsDCcFJBnwxrgMJYJZ0rM4k/2hRRHNcdqqvoes3e3JEedns0tmwv7maefv2LYVFODbsINXT8EAxyTgRsd1OWiP0KXYawdVi+kxzDvh4oA7bObB0E4flDPSNdj/ZZXUTVtFJ9zRDN3Mn7we/6+pTR8D36HYmNtmSe8BIketR24IDazZRBs1mrM7Jdai3FpyecYMxeiduH9/Z67t4NOvNNq1SunfiERX2ABZ+Vj8NbWB1WmbJvWSCaAyWSSqp1BGg+tFLWVLSZs/WEvwTbjKQy2eFJAMVZAvK2+AmLvdpwwa+q+O5wl4LjqMofoz0uTuJ3gX5SaB93RtSDjN+Bl0Gw== x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(180628864354917)(9452136761055)(185117386973197)(258649278758335); x-exchange-antispam-report-cfa-test: BCL:0;PCL:0;RULEID:(6040375)(601004)(2401047)(5005006)(8121501046)(3002001)(10201501046)(6055026)(6041248)(20161123562025)(20161123555025)(20161123564025)(20161123560025)(6072148)(6047074);SRVR:VI1PR04MB1325;BCL:0;PCL:0;RULEID:;SRVR:VI1PR04MB1325; x-forefront-prvs: 01792087B6 x-forefront-antispam-report: SFV:NSPM;SFS:(10009020)(6009001)(7916002)(39410400002)(39850400002)(39860400002)(39450400003)(39840400002)(199003)(24454002)(377454003)(189002)(13464003)(101416001)(105586002)(97736004)(106356001)(4326007)(93886004)(8936002)(2950100002)(102836003)(6116002)(3846002)(2906002)(575784001)(86362001)(2201001)(92566002)(345774005)(2900100001)(5001770100001)(106116001)(76176999)(33656002)(5660300001)(55016002)(99286003)(25786008)(6506006)(39060400001)(38730400001)(7696004)(7416002)(229853002)(77096006)(54906002)(305945005)(66066001)(81156014)(8676002)(7736002)(81166006)(9686002)(68736007)(122556002)(2501003)(189998001)(50986999)(54356999)(3280700002)(3660700001)(6436002)(74316002)(429304003)(921003)(1121003);DIR:OUT;SFP:1101;SCL:1;SRVR:VI1PR04MB1325;H:AM5PR0401MB2545.eurprd04.prod.outlook.com;FPR:;SPF:None;PTR:InfoNoRecords;A:1;MX:1;LANG:en; spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 X-OriginatorOrg: nxp.com X-MS-Exchange-CrossTenant-originalarrivaltime: 06 Jan 2017 04:27:03.6854 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635 X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR04MB1325 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by mail.home.local id v064RrDo030457 Hi Mark, > -----Original Message----- > From: Auger Eric [mailto:eric.auger@redhat.com] > Sent: Thursday, January 05, 2017 5:39 PM > To: Marc Zyngier ; eric.auger.pro@gmail.com; > christoffer.dall@linaro.org; robin.murphy@arm.com; > alex.williamson@redhat.com; will.deacon@arm.com; joro@8bytes.org; > tglx@linutronix.de; jason@lakedaemon.net; linux-arm- > kernel@lists.infradead.org > Cc: drjones@redhat.com; kvm@vger.kernel.org; punit.agrawal@arm.com; > linux-kernel@vger.kernel.org; geethasowjanya.akula@gmail.com; Diana > Madalina Craciun ; iommu@lists.linux- > foundation.org; pranav.sawargaonkar@gmail.com; Bharat Bhushan > ; shankerd@codeaurora.org; > gpkulkarni@gmail.com > Subject: Re: [PATCH v5 13/17] irqdomain: irq_domain_check_msi_remap > > Hi Marc, > > On 05/01/2017 12:57, Marc Zyngier wrote: > > On 05/01/17 11:29, Auger Eric wrote: > >> Hi Marc, > >> > >> On 05/01/2017 12:25, Marc Zyngier wrote: > >>> On 05/01/17 10:45, Auger Eric wrote: > >>>> Hi Marc, > >>>> > >>>> On 04/01/2017 16:27, Marc Zyngier wrote: > >>>>> On 04/01/17 14:11, Auger Eric wrote: > >>>>>> Hi Marc, > >>>>>> > >>>>>> On 04/01/2017 14:46, Marc Zyngier wrote: > >>>>>>> Hi Eric, > >>>>>>> > >>>>>>> On 04/01/17 13:32, Eric Auger wrote: > >>>>>>>> This new function checks whether all platform and PCI MSI > >>>>>>>> domains implement IRQ remapping. This is useful to understand > >>>>>>>> whether VFIO passthrough is safe with respect to interrupts. > >>>>>>>> > >>>>>>>> On ARM typically an MSI controller can sit downstream to the > >>>>>>>> IOMMU without preventing VFIO passthrough. > >>>>>>>> As such any assigned device can write into the MSI doorbell. > >>>>>>>> In case the MSI controller implements IRQ remapping, assigned > >>>>>>>> devices will not be able to trigger interrupts towards the > >>>>>>>> host. On the contrary, the assignment must be emphasized as > >>>>>>>> unsafe with respect to interrupts. > >>>>>>>> > >>>>>>>> Signed-off-by: Eric Auger > >>>>>>>> > >>>>>>>> --- > >>>>>>>> > >>>>>>>> v4 -> v5: > >>>>>>>> - Handle DOMAIN_BUS_FSL_MC_MSI domains > >>>>>>>> - Check parents > >>>>>>>> --- > >>>>>>>> include/linux/irqdomain.h | 1 + > >>>>>>>> kernel/irq/irqdomain.c | 41 > +++++++++++++++++++++++++++++++++++++++++ > >>>>>>>> 2 files changed, 42 insertions(+) > >>>>>>>> > >>>>>>>> diff --git a/include/linux/irqdomain.h > >>>>>>>> b/include/linux/irqdomain.h index ab017b2..281a40f 100644 > >>>>>>>> --- a/include/linux/irqdomain.h > >>>>>>>> +++ b/include/linux/irqdomain.h > >>>>>>>> @@ -219,6 +219,7 @@ struct irq_domain > *irq_domain_add_legacy(struct device_node *of_node, > >>>>>>>> void *host_data); > >>>>>>>> extern struct irq_domain *irq_find_matching_fwspec(struct > irq_fwspec *fwspec, > >>>>>>>> enum > irq_domain_bus_token bus_token); > >>>>>>>> +extern bool irq_domain_check_msi_remap(void); > >>>>>>>> extern void irq_set_default_host(struct irq_domain *host); > >>>>>>>> extern int irq_domain_alloc_descs(int virq, unsigned int nr_irqs, > >>>>>>>> irq_hw_number_t hwirq, int node, > diff --git > >>>>>>>> a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c index > >>>>>>>> 8c0a0ae..700caea 100644 > >>>>>>>> --- a/kernel/irq/irqdomain.c > >>>>>>>> +++ b/kernel/irq/irqdomain.c > >>>>>>>> @@ -278,6 +278,47 @@ struct irq_domain > >>>>>>>> *irq_find_matching_fwspec(struct irq_fwspec *fwspec, > >>>>>>>> EXPORT_SYMBOL_GPL(irq_find_matching_fwspec); > >>>>>>>> > >>>>>>>> /** > >>>>>>>> + * irq_domain_is_msi_remap - Check if @domain or any parent > >>>>>>>> + * has MSI remapping support > >>>>>>>> + * @domain: domain pointer > >>>>>>>> + */ > >>>>>>>> +static bool irq_domain_is_msi_remap(struct irq_domain > *domain) > >>>>>>>> +{ > >>>>>>>> + struct irq_domain *h = domain; > >>>>>>>> + > >>>>>>>> + for (; h; h = h->parent) { > >>>>>>>> + if (h->flags & IRQ_DOMAIN_FLAG_MSI_REMAP) > >>>>>>>> + return true; > >>>>>>>> + } > >>>>>>>> + return false; > >>>>>>>> +} > >>>>>>>> + > >>>>>>>> +/** > >>>>>>>> + * irq_domain_check_msi_remap() - Checks whether all MSI > >>>>>>>> + * irq domains implement IRQ remapping */ bool > >>>>>>>> +irq_domain_check_msi_remap(void) { > >>>>>>>> + struct irq_domain *h; > >>>>>>>> + bool ret = true; > >>>>>>>> + > >>>>>>>> + mutex_lock(&irq_domain_mutex); > >>>>>>>> + list_for_each_entry(h, &irq_domain_list, link) { > >>>>>>>> + if (((h->bus_token & DOMAIN_BUS_PCI_MSI) || > >>>>>>>> + (h->bus_token & DOMAIN_BUS_PLATFORM_MSI) > || > >>>>>>>> + (h->bus_token & DOMAIN_BUS_FSL_MC_MSI)) > && > >>>>>>>> + !irq_domain_is_msi_remap(h)) { > >>>>>>> > >>>>>>> (h->bus_token & DOMAIN_BUS_PCI_MSI) and co looks quite > wrong. > >>>>>>> bus_token is not a bitmap, and DOMAIN_BUS_* not a single bit > >>>>>>> value (see enum irq_domain_bus_token). Surely this should read > >>>>>>> (h->bus_token == DOMAIN_BUS_PCI_MSI). > >>>>>> Oh I did not notice that. Thanks. > >>>>>> > >>>>>> Any other comments on the irqdomain side? Do you think the > >>>>>> current approach consisting in looking at those bus tokens and > >>>>>> their parents looks good? > >>>>> > >>>>> To be completely honest, I don't like it much, as having to > >>>>> enumerate all the bus types can come up with could become quite a > >>>>> burden in the long run. I'd rather be able to identify MSI capable > >>>>> domains by construction. I came up with the following approach (fully > untested): > >>>>> > >>>>> diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h > >>>>> index 281a40f..7779796 100644 > >>>>> --- a/include/linux/irqdomain.h > >>>>> +++ b/include/linux/irqdomain.h > >>>>> @@ -183,8 +183,11 @@ enum { > >>>>> /* Irq domain is an IPI domain with single virq */ > >>>>> IRQ_DOMAIN_FLAG_IPI_SINGLE = (1 << 3), > >>>>> > >>>>> + /* Irq domain implements MSIs */ > >>>>> + IRQ_DOMAIN_FLAG_MSI = (1 << 4), > >>>>> + > >>>>> /* Irq domain is MSI remapping capable */ > >>>>> - IRQ_DOMAIN_FLAG_MSI_REMAP = (1 << 4), > >>>>> + IRQ_DOMAIN_FLAG_MSI_REMAP = (1 << 5), > >>>>> > >>>>> /* > >>>>> * Flags starting from IRQ_DOMAIN_FLAG_NONCORE are reserved > @@ > >>>>> -450,6 +453,11 @@ static inline bool > >>>>> irq_domain_is_ipi_single(struct irq_domain *domain) { > >>>>> return domain->flags & IRQ_DOMAIN_FLAG_IPI_SINGLE; } > >>>>> + > >>>>> +static inline bool irq_domain_is_msi(struct irq_domain *domain) { > >>>>> + return domain->flags & IRQ_DOMAIN_FLAG_MSI; } > >>>>> #else /* CONFIG_IRQ_DOMAIN_HIERARCHY */ > >>>>> static inline void irq_domain_activate_irq(struct irq_data *data) > >>>>> { } static inline void irq_domain_deactivate_irq(struct irq_data > >>>>> *data) { } @@ -481,6 +489,11 @@ static inline bool > >>>>> irq_domain_is_ipi_single(struct irq_domain *domain) { > >>>>> return false; > >>>>> } > >>>>> + > >>>>> +static inline bool irq_domain_is_msi(struct irq_domain *domain) { > >>>>> + return false; > >>>>> +} > >>>>> #endif /* CONFIG_IRQ_DOMAIN_HIERARCHY */ > >>>>> > >>>>> #else /* CONFIG_IRQ_DOMAIN */ > >>>>> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c index > >>>>> 700caea..33b6921 100644 > >>>>> --- a/kernel/irq/irqdomain.c > >>>>> +++ b/kernel/irq/irqdomain.c > >>>>> @@ -304,10 +304,7 @@ bool irq_domain_check_msi_remap(void) > >>>>> > >>>>> mutex_lock(&irq_domain_mutex); > >>>>> list_for_each_entry(h, &irq_domain_list, link) { > >>>>> - if (((h->bus_token & DOMAIN_BUS_PCI_MSI) || > >>>>> - (h->bus_token & DOMAIN_BUS_PLATFORM_MSI) > || > >>>>> - (h->bus_token & DOMAIN_BUS_FSL_MC_MSI)) > && > >>>>> - !irq_domain_is_msi_remap(h)) { > >>>>> + if (irq_domain_is_msi(h) && > !irq_domain_is_msi_remap(h)) { > >>>>> ret = false; > >>>>> goto out; > >>>>> } > >>>>> diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c index > >>>>> ee23006..b637263 100644 > >>>>> --- a/kernel/irq/msi.c > >>>>> +++ b/kernel/irq/msi.c > >>>>> @@ -270,7 +270,7 @@ struct irq_domain > *msi_create_irq_domain(struct fwnode_handle *fwnode, > >>>>> if (info->flags & MSI_FLAG_USE_DEF_CHIP_OPS) > >>>>> msi_domain_update_chip_ops(info); > >>>>> > >>>>> - return irq_domain_create_hierarchy(parent, 0, 0, fwnode, > >>>>> + return irq_domain_create_hierarchy(parent, > IRQ_DOMAIN_FLAG_MSI, > >>>>> +0, fwnode, > >>>>> &msi_domain_ops, info); > >>>>> } > >>>>> > >>>>> > >>>>> > >>>>> Thoughts? > >>>> > >>>> Don't we need to set the IRQ_DOMAIN_FLAG_MSI flag in > >>>> platform_msi_create_device_domain too (drivers/base/platform- > msi.c)? > >> was mentioning platform_msi_create_*device*_domain. > >> it calls irq_domain_create_hierarchy and looks to be MSI irq domain > >> related. But I don't have a full understanding of the whole irq > >> domain hierarchy. > > > > Ah, sorry - I blame the ARM coffee. > > > > This function builds a domain for a single device on top of the MSI > > domain that has been already created (see the dev->msi_domain passed > > to irq_domain_create_hierarchy). The structure looks like this: > > > > device-domain -> platform MSI domain -> HW MSI domain -> whatever > > > > So what we're *really* interested in is the platform MSI domain, which > > is going to carry the IRQ_DOMAIN_FLAG_MSI flag. The device-domain only > > describes a portion of it, and can safely be ignored. > > > > In the end, what matters for this patch is that we can prove that from > > any domain carrying the IRQ_DOMAIN_FLAG_MSI flag, we can find a > domain > > carrying the IRQ_DOMAIN_FLAG_MSI_REMAP flag. If that property holds, > > we're safe. Otherwise, we disable the Guest MSI feature. > > > > Does it make sense? > Yes it makes sense. Thank you for the explanation! If I understood correctly then the domain hierarchy is -> "gic-irq-domain" -> "gic-its-irq-domain" -> "platform-msi-domain" -> "pci-msi-domain" -> "fsl-mc-msi-domain" "gic-its-irq-domain" carries IRQ_DOMAIN_FLAG_MSI_REMAP So we need to look for the IRQ_DOMAIN_FLAG_MSI_REMAP flag in "gic-its-irq-domain" when doing safety check for "platform/pci/fsl-mc"-msi-irqdomain, is this what you mentioned? Can we can pass this flags from "gic-its-irq-domain" to "platform/pci/fsl-mc"-msi-irqdomain during domain creation? Thanks -Bharat > > Eric > > > > Thanks, > > > > M. > > From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bharat Bhushan Subject: RE: [PATCH v5 13/17] irqdomain: irq_domain_check_msi_remap Date: Fri, 6 Jan 2017 04:27:03 +0000 Message-ID: References: <1483536746-2725-1-git-send-email-eric.auger@redhat.com> <1483536746-2725-14-git-send-email-eric.auger@redhat.com> <92fcb464-3364-c432-29d8-1c2ce4a5f110@redhat.com> <1e881510-bc18-ae97-ec2f-feadcba54150@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: "drjones-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org" , "kvm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , "punit.agrawal-5wv7dgnIgG8@public.gmane.org" , "linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , "geethasowjanya.akula-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org" , "iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org" , "pranav.sawargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org" , "shankerd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org" , "gpkulkarni-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org" To: Auger Eric , Marc Zyngier , "eric.auger.pro-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org" , "christoffer.dall-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org" , "robin.murphy-5wv7dgnIgG8@public.gmane.org" , "alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org" , "will.deacon-5wv7dgnIgG8@public.gmane.org" , "joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org" , "tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org" , "jason-NLaQJdtUoK4Be96aLqz0jA@public.gmane.org" , "linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org" Return-path: In-Reply-To: Content-Language: en-US List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org Errors-To: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org List-Id: kvm.vger.kernel.org Hi Mark, > -----Original Message----- > From: Auger Eric [mailto:eric.auger-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org] > Sent: Thursday, January 05, 2017 5:39 PM > To: Marc Zyngier ; eric.auger.pro-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org; > christoffer.dall-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org; robin.murphy-5wv7dgnIgG8@public.gmane.org; > alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org; will.deacon-5wv7dgnIgG8@public.gmane.org; joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org; > tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org; jason-NLaQJdtUoK4Be96aLqz0jA@public.gmane.org; linux-arm- > kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org > Cc: drjones-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org; kvm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; punit.agrawal-5wv7dgnIgG8@public.gmane.org; > linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; geethasowjanya.akula-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org; Diana > Madalina Craciun ; iommu-cunTk1MwBs/ROKNJybVBZg@public.gmane.org > foundation.org; pranav.sawargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org; Bharat Bhushan > ; shankerd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org; > gpkulkarni-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org > Subject: Re: [PATCH v5 13/17] irqdomain: irq_domain_check_msi_remap > > Hi Marc, > > On 05/01/2017 12:57, Marc Zyngier wrote: > > On 05/01/17 11:29, Auger Eric wrote: > >> Hi Marc, > >> > >> On 05/01/2017 12:25, Marc Zyngier wrote: > >>> On 05/01/17 10:45, Auger Eric wrote: > >>>> Hi Marc, > >>>> > >>>> On 04/01/2017 16:27, Marc Zyngier wrote: > >>>>> On 04/01/17 14:11, Auger Eric wrote: > >>>>>> Hi Marc, > >>>>>> > >>>>>> On 04/01/2017 14:46, Marc Zyngier wrote: > >>>>>>> Hi Eric, > >>>>>>> > >>>>>>> On 04/01/17 13:32, Eric Auger wrote: > >>>>>>>> This new function checks whether all platform and PCI MSI > >>>>>>>> domains implement IRQ remapping. This is useful to understand > >>>>>>>> whether VFIO passthrough is safe with respect to interrupts. > >>>>>>>> > >>>>>>>> On ARM typically an MSI controller can sit downstream to the > >>>>>>>> IOMMU without preventing VFIO passthrough. > >>>>>>>> As such any assigned device can write into the MSI doorbell. > >>>>>>>> In case the MSI controller implements IRQ remapping, assigned > >>>>>>>> devices will not be able to trigger interrupts towards the > >>>>>>>> host. On the contrary, the assignment must be emphasized as > >>>>>>>> unsafe with respect to interrupts. > >>>>>>>> > >>>>>>>> Signed-off-by: Eric Auger > >>>>>>>> > >>>>>>>> --- > >>>>>>>> > >>>>>>>> v4 -> v5: > >>>>>>>> - Handle DOMAIN_BUS_FSL_MC_MSI domains > >>>>>>>> - Check parents > >>>>>>>> --- > >>>>>>>> include/linux/irqdomain.h | 1 + > >>>>>>>> kernel/irq/irqdomain.c | 41 > +++++++++++++++++++++++++++++++++++++++++ > >>>>>>>> 2 files changed, 42 insertions(+) > >>>>>>>> > >>>>>>>> diff --git a/include/linux/irqdomain.h > >>>>>>>> b/include/linux/irqdomain.h index ab017b2..281a40f 100644 > >>>>>>>> --- a/include/linux/irqdomain.h > >>>>>>>> +++ b/include/linux/irqdomain.h > >>>>>>>> @@ -219,6 +219,7 @@ struct irq_domain > *irq_domain_add_legacy(struct device_node *of_node, > >>>>>>>> void *host_data); > >>>>>>>> extern struct irq_domain *irq_find_matching_fwspec(struct > irq_fwspec *fwspec, > >>>>>>>> enum > irq_domain_bus_token bus_token); > >>>>>>>> +extern bool irq_domain_check_msi_remap(void); > >>>>>>>> extern void irq_set_default_host(struct irq_domain *host); > >>>>>>>> extern int irq_domain_alloc_descs(int virq, unsigned int nr_irqs, > >>>>>>>> irq_hw_number_t hwirq, int node, > diff --git > >>>>>>>> a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c index > >>>>>>>> 8c0a0ae..700caea 100644 > >>>>>>>> --- a/kernel/irq/irqdomain.c > >>>>>>>> +++ b/kernel/irq/irqdomain.c > >>>>>>>> @@ -278,6 +278,47 @@ struct irq_domain > >>>>>>>> *irq_find_matching_fwspec(struct irq_fwspec *fwspec, > >>>>>>>> EXPORT_SYMBOL_GPL(irq_find_matching_fwspec); > >>>>>>>> > >>>>>>>> /** > >>>>>>>> + * irq_domain_is_msi_remap - Check if @domain or any parent > >>>>>>>> + * has MSI remapping support > >>>>>>>> + * @domain: domain pointer > >>>>>>>> + */ > >>>>>>>> +static bool irq_domain_is_msi_remap(struct irq_domain > *domain) > >>>>>>>> +{ > >>>>>>>> + struct irq_domain *h = domain; > >>>>>>>> + > >>>>>>>> + for (; h; h = h->parent) { > >>>>>>>> + if (h->flags & IRQ_DOMAIN_FLAG_MSI_REMAP) > >>>>>>>> + return true; > >>>>>>>> + } > >>>>>>>> + return false; > >>>>>>>> +} > >>>>>>>> + > >>>>>>>> +/** > >>>>>>>> + * irq_domain_check_msi_remap() - Checks whether all MSI > >>>>>>>> + * irq domains implement IRQ remapping */ bool > >>>>>>>> +irq_domain_check_msi_remap(void) { > >>>>>>>> + struct irq_domain *h; > >>>>>>>> + bool ret = true; > >>>>>>>> + > >>>>>>>> + mutex_lock(&irq_domain_mutex); > >>>>>>>> + list_for_each_entry(h, &irq_domain_list, link) { > >>>>>>>> + if (((h->bus_token & DOMAIN_BUS_PCI_MSI) || > >>>>>>>> + (h->bus_token & DOMAIN_BUS_PLATFORM_MSI) > || > >>>>>>>> + (h->bus_token & DOMAIN_BUS_FSL_MC_MSI)) > && > >>>>>>>> + !irq_domain_is_msi_remap(h)) { > >>>>>>> > >>>>>>> (h->bus_token & DOMAIN_BUS_PCI_MSI) and co looks quite > wrong. > >>>>>>> bus_token is not a bitmap, and DOMAIN_BUS_* not a single bit > >>>>>>> value (see enum irq_domain_bus_token). Surely this should read > >>>>>>> (h->bus_token == DOMAIN_BUS_PCI_MSI). > >>>>>> Oh I did not notice that. Thanks. > >>>>>> > >>>>>> Any other comments on the irqdomain side? Do you think the > >>>>>> current approach consisting in looking at those bus tokens and > >>>>>> their parents looks good? > >>>>> > >>>>> To be completely honest, I don't like it much, as having to > >>>>> enumerate all the bus types can come up with could become quite a > >>>>> burden in the long run. I'd rather be able to identify MSI capable > >>>>> domains by construction. I came up with the following approach (fully > untested): > >>>>> > >>>>> diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h > >>>>> index 281a40f..7779796 100644 > >>>>> --- a/include/linux/irqdomain.h > >>>>> +++ b/include/linux/irqdomain.h > >>>>> @@ -183,8 +183,11 @@ enum { > >>>>> /* Irq domain is an IPI domain with single virq */ > >>>>> IRQ_DOMAIN_FLAG_IPI_SINGLE = (1 << 3), > >>>>> > >>>>> + /* Irq domain implements MSIs */ > >>>>> + IRQ_DOMAIN_FLAG_MSI = (1 << 4), > >>>>> + > >>>>> /* Irq domain is MSI remapping capable */ > >>>>> - IRQ_DOMAIN_FLAG_MSI_REMAP = (1 << 4), > >>>>> + IRQ_DOMAIN_FLAG_MSI_REMAP = (1 << 5), > >>>>> > >>>>> /* > >>>>> * Flags starting from IRQ_DOMAIN_FLAG_NONCORE are reserved > @@ > >>>>> -450,6 +453,11 @@ static inline bool > >>>>> irq_domain_is_ipi_single(struct irq_domain *domain) { > >>>>> return domain->flags & IRQ_DOMAIN_FLAG_IPI_SINGLE; } > >>>>> + > >>>>> +static inline bool irq_domain_is_msi(struct irq_domain *domain) { > >>>>> + return domain->flags & IRQ_DOMAIN_FLAG_MSI; } > >>>>> #else /* CONFIG_IRQ_DOMAIN_HIERARCHY */ > >>>>> static inline void irq_domain_activate_irq(struct irq_data *data) > >>>>> { } static inline void irq_domain_deactivate_irq(struct irq_data > >>>>> *data) { } @@ -481,6 +489,11 @@ static inline bool > >>>>> irq_domain_is_ipi_single(struct irq_domain *domain) { > >>>>> return false; > >>>>> } > >>>>> + > >>>>> +static inline bool irq_domain_is_msi(struct irq_domain *domain) { > >>>>> + return false; > >>>>> +} > >>>>> #endif /* CONFIG_IRQ_DOMAIN_HIERARCHY */ > >>>>> > >>>>> #else /* CONFIG_IRQ_DOMAIN */ > >>>>> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c index > >>>>> 700caea..33b6921 100644 > >>>>> --- a/kernel/irq/irqdomain.c > >>>>> +++ b/kernel/irq/irqdomain.c > >>>>> @@ -304,10 +304,7 @@ bool irq_domain_check_msi_remap(void) > >>>>> > >>>>> mutex_lock(&irq_domain_mutex); > >>>>> list_for_each_entry(h, &irq_domain_list, link) { > >>>>> - if (((h->bus_token & DOMAIN_BUS_PCI_MSI) || > >>>>> - (h->bus_token & DOMAIN_BUS_PLATFORM_MSI) > || > >>>>> - (h->bus_token & DOMAIN_BUS_FSL_MC_MSI)) > && > >>>>> - !irq_domain_is_msi_remap(h)) { > >>>>> + if (irq_domain_is_msi(h) && > !irq_domain_is_msi_remap(h)) { > >>>>> ret = false; > >>>>> goto out; > >>>>> } > >>>>> diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c index > >>>>> ee23006..b637263 100644 > >>>>> --- a/kernel/irq/msi.c > >>>>> +++ b/kernel/irq/msi.c > >>>>> @@ -270,7 +270,7 @@ struct irq_domain > *msi_create_irq_domain(struct fwnode_handle *fwnode, > >>>>> if (info->flags & MSI_FLAG_USE_DEF_CHIP_OPS) > >>>>> msi_domain_update_chip_ops(info); > >>>>> > >>>>> - return irq_domain_create_hierarchy(parent, 0, 0, fwnode, > >>>>> + return irq_domain_create_hierarchy(parent, > IRQ_DOMAIN_FLAG_MSI, > >>>>> +0, fwnode, > >>>>> &msi_domain_ops, info); > >>>>> } > >>>>> > >>>>> > >>>>> > >>>>> Thoughts? > >>>> > >>>> Don't we need to set the IRQ_DOMAIN_FLAG_MSI flag in > >>>> platform_msi_create_device_domain too (drivers/base/platform- > msi.c)? > >> was mentioning platform_msi_create_*device*_domain. > >> it calls irq_domain_create_hierarchy and looks to be MSI irq domain > >> related. But I don't have a full understanding of the whole irq > >> domain hierarchy. > > > > Ah, sorry - I blame the ARM coffee. > > > > This function builds a domain for a single device on top of the MSI > > domain that has been already created (see the dev->msi_domain passed > > to irq_domain_create_hierarchy). The structure looks like this: > > > > device-domain -> platform MSI domain -> HW MSI domain -> whatever > > > > So what we're *really* interested in is the platform MSI domain, which > > is going to carry the IRQ_DOMAIN_FLAG_MSI flag. The device-domain only > > describes a portion of it, and can safely be ignored. > > > > In the end, what matters for this patch is that we can prove that from > > any domain carrying the IRQ_DOMAIN_FLAG_MSI flag, we can find a > domain > > carrying the IRQ_DOMAIN_FLAG_MSI_REMAP flag. If that property holds, > > we're safe. Otherwise, we disable the Guest MSI feature. > > > > Does it make sense? > Yes it makes sense. Thank you for the explanation! If I understood correctly then the domain hierarchy is -> "gic-irq-domain" -> "gic-its-irq-domain" -> "platform-msi-domain" -> "pci-msi-domain" -> "fsl-mc-msi-domain" "gic-its-irq-domain" carries IRQ_DOMAIN_FLAG_MSI_REMAP So we need to look for the IRQ_DOMAIN_FLAG_MSI_REMAP flag in "gic-its-irq-domain" when doing safety check for "platform/pci/fsl-mc"-msi-irqdomain, is this what you mentioned? Can we can pass this flags from "gic-its-irq-domain" to "platform/pci/fsl-mc"-msi-irqdomain during domain creation? Thanks -Bharat > > Eric > > > > Thanks, > > > > M. > > From mboxrd@z Thu Jan 1 00:00:00 1970 From: bharat.bhushan@nxp.com (Bharat Bhushan) Date: Fri, 6 Jan 2017 04:27:03 +0000 Subject: [PATCH v5 13/17] irqdomain: irq_domain_check_msi_remap In-Reply-To: References: <1483536746-2725-1-git-send-email-eric.auger@redhat.com> <1483536746-2725-14-git-send-email-eric.auger@redhat.com> <92fcb464-3364-c432-29d8-1c2ce4a5f110@redhat.com> <1e881510-bc18-ae97-ec2f-feadcba54150@redhat.com> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi Mark, > -----Original Message----- > From: Auger Eric [mailto:eric.auger at redhat.com] > Sent: Thursday, January 05, 2017 5:39 PM > To: Marc Zyngier ; eric.auger.pro at gmail.com; > christoffer.dall at linaro.org; robin.murphy at arm.com; > alex.williamson at redhat.com; will.deacon at arm.com; joro at 8bytes.org; > tglx at linutronix.de; jason at lakedaemon.net; linux-arm- > kernel at lists.infradead.org > Cc: drjones at redhat.com; kvm at vger.kernel.org; punit.agrawal at arm.com; > linux-kernel at vger.kernel.org; geethasowjanya.akula at gmail.com; Diana > Madalina Craciun ; iommu at lists.linux- > foundation.org; pranav.sawargaonkar at gmail.com; Bharat Bhushan > ; shankerd at codeaurora.org; > gpkulkarni at gmail.com > Subject: Re: [PATCH v5 13/17] irqdomain: irq_domain_check_msi_remap > > Hi Marc, > > On 05/01/2017 12:57, Marc Zyngier wrote: > > On 05/01/17 11:29, Auger Eric wrote: > >> Hi Marc, > >> > >> On 05/01/2017 12:25, Marc Zyngier wrote: > >>> On 05/01/17 10:45, Auger Eric wrote: > >>>> Hi Marc, > >>>> > >>>> On 04/01/2017 16:27, Marc Zyngier wrote: > >>>>> On 04/01/17 14:11, Auger Eric wrote: > >>>>>> Hi Marc, > >>>>>> > >>>>>> On 04/01/2017 14:46, Marc Zyngier wrote: > >>>>>>> Hi Eric, > >>>>>>> > >>>>>>> On 04/01/17 13:32, Eric Auger wrote: > >>>>>>>> This new function checks whether all platform and PCI MSI > >>>>>>>> domains implement IRQ remapping. This is useful to understand > >>>>>>>> whether VFIO passthrough is safe with respect to interrupts. > >>>>>>>> > >>>>>>>> On ARM typically an MSI controller can sit downstream to the > >>>>>>>> IOMMU without preventing VFIO passthrough. > >>>>>>>> As such any assigned device can write into the MSI doorbell. > >>>>>>>> In case the MSI controller implements IRQ remapping, assigned > >>>>>>>> devices will not be able to trigger interrupts towards the > >>>>>>>> host. On the contrary, the assignment must be emphasized as > >>>>>>>> unsafe with respect to interrupts. > >>>>>>>> > >>>>>>>> Signed-off-by: Eric Auger > >>>>>>>> > >>>>>>>> --- > >>>>>>>> > >>>>>>>> v4 -> v5: > >>>>>>>> - Handle DOMAIN_BUS_FSL_MC_MSI domains > >>>>>>>> - Check parents > >>>>>>>> --- > >>>>>>>> include/linux/irqdomain.h | 1 + > >>>>>>>> kernel/irq/irqdomain.c | 41 > +++++++++++++++++++++++++++++++++++++++++ > >>>>>>>> 2 files changed, 42 insertions(+) > >>>>>>>> > >>>>>>>> diff --git a/include/linux/irqdomain.h > >>>>>>>> b/include/linux/irqdomain.h index ab017b2..281a40f 100644 > >>>>>>>> --- a/include/linux/irqdomain.h > >>>>>>>> +++ b/include/linux/irqdomain.h > >>>>>>>> @@ -219,6 +219,7 @@ struct irq_domain > *irq_domain_add_legacy(struct device_node *of_node, > >>>>>>>> void *host_data); > >>>>>>>> extern struct irq_domain *irq_find_matching_fwspec(struct > irq_fwspec *fwspec, > >>>>>>>> enum > irq_domain_bus_token bus_token); > >>>>>>>> +extern bool irq_domain_check_msi_remap(void); > >>>>>>>> extern void irq_set_default_host(struct irq_domain *host); > >>>>>>>> extern int irq_domain_alloc_descs(int virq, unsigned int nr_irqs, > >>>>>>>> irq_hw_number_t hwirq, int node, > diff --git > >>>>>>>> a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c index > >>>>>>>> 8c0a0ae..700caea 100644 > >>>>>>>> --- a/kernel/irq/irqdomain.c > >>>>>>>> +++ b/kernel/irq/irqdomain.c > >>>>>>>> @@ -278,6 +278,47 @@ struct irq_domain > >>>>>>>> *irq_find_matching_fwspec(struct irq_fwspec *fwspec, > >>>>>>>> EXPORT_SYMBOL_GPL(irq_find_matching_fwspec); > >>>>>>>> > >>>>>>>> /** > >>>>>>>> + * irq_domain_is_msi_remap - Check if @domain or any parent > >>>>>>>> + * has MSI remapping support > >>>>>>>> + * @domain: domain pointer > >>>>>>>> + */ > >>>>>>>> +static bool irq_domain_is_msi_remap(struct irq_domain > *domain) > >>>>>>>> +{ > >>>>>>>> + struct irq_domain *h = domain; > >>>>>>>> + > >>>>>>>> + for (; h; h = h->parent) { > >>>>>>>> + if (h->flags & IRQ_DOMAIN_FLAG_MSI_REMAP) > >>>>>>>> + return true; > >>>>>>>> + } > >>>>>>>> + return false; > >>>>>>>> +} > >>>>>>>> + > >>>>>>>> +/** > >>>>>>>> + * irq_domain_check_msi_remap() - Checks whether all MSI > >>>>>>>> + * irq domains implement IRQ remapping */ bool > >>>>>>>> +irq_domain_check_msi_remap(void) { > >>>>>>>> + struct irq_domain *h; > >>>>>>>> + bool ret = true; > >>>>>>>> + > >>>>>>>> + mutex_lock(&irq_domain_mutex); > >>>>>>>> + list_for_each_entry(h, &irq_domain_list, link) { > >>>>>>>> + if (((h->bus_token & DOMAIN_BUS_PCI_MSI) || > >>>>>>>> + (h->bus_token & DOMAIN_BUS_PLATFORM_MSI) > || > >>>>>>>> + (h->bus_token & DOMAIN_BUS_FSL_MC_MSI)) > && > >>>>>>>> + !irq_domain_is_msi_remap(h)) { > >>>>>>> > >>>>>>> (h->bus_token & DOMAIN_BUS_PCI_MSI) and co looks quite > wrong. > >>>>>>> bus_token is not a bitmap, and DOMAIN_BUS_* not a single bit > >>>>>>> value (see enum irq_domain_bus_token). Surely this should read > >>>>>>> (h->bus_token == DOMAIN_BUS_PCI_MSI). > >>>>>> Oh I did not notice that. Thanks. > >>>>>> > >>>>>> Any other comments on the irqdomain side? Do you think the > >>>>>> current approach consisting in looking at those bus tokens and > >>>>>> their parents looks good? > >>>>> > >>>>> To be completely honest, I don't like it much, as having to > >>>>> enumerate all the bus types can come up with could become quite a > >>>>> burden in the long run. I'd rather be able to identify MSI capable > >>>>> domains by construction. I came up with the following approach (fully > untested): > >>>>> > >>>>> diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h > >>>>> index 281a40f..7779796 100644 > >>>>> --- a/include/linux/irqdomain.h > >>>>> +++ b/include/linux/irqdomain.h > >>>>> @@ -183,8 +183,11 @@ enum { > >>>>> /* Irq domain is an IPI domain with single virq */ > >>>>> IRQ_DOMAIN_FLAG_IPI_SINGLE = (1 << 3), > >>>>> > >>>>> + /* Irq domain implements MSIs */ > >>>>> + IRQ_DOMAIN_FLAG_MSI = (1 << 4), > >>>>> + > >>>>> /* Irq domain is MSI remapping capable */ > >>>>> - IRQ_DOMAIN_FLAG_MSI_REMAP = (1 << 4), > >>>>> + IRQ_DOMAIN_FLAG_MSI_REMAP = (1 << 5), > >>>>> > >>>>> /* > >>>>> * Flags starting from IRQ_DOMAIN_FLAG_NONCORE are reserved > @@ > >>>>> -450,6 +453,11 @@ static inline bool > >>>>> irq_domain_is_ipi_single(struct irq_domain *domain) { > >>>>> return domain->flags & IRQ_DOMAIN_FLAG_IPI_SINGLE; } > >>>>> + > >>>>> +static inline bool irq_domain_is_msi(struct irq_domain *domain) { > >>>>> + return domain->flags & IRQ_DOMAIN_FLAG_MSI; } > >>>>> #else /* CONFIG_IRQ_DOMAIN_HIERARCHY */ > >>>>> static inline void irq_domain_activate_irq(struct irq_data *data) > >>>>> { } static inline void irq_domain_deactivate_irq(struct irq_data > >>>>> *data) { } @@ -481,6 +489,11 @@ static inline bool > >>>>> irq_domain_is_ipi_single(struct irq_domain *domain) { > >>>>> return false; > >>>>> } > >>>>> + > >>>>> +static inline bool irq_domain_is_msi(struct irq_domain *domain) { > >>>>> + return false; > >>>>> +} > >>>>> #endif /* CONFIG_IRQ_DOMAIN_HIERARCHY */ > >>>>> > >>>>> #else /* CONFIG_IRQ_DOMAIN */ > >>>>> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c index > >>>>> 700caea..33b6921 100644 > >>>>> --- a/kernel/irq/irqdomain.c > >>>>> +++ b/kernel/irq/irqdomain.c > >>>>> @@ -304,10 +304,7 @@ bool irq_domain_check_msi_remap(void) > >>>>> > >>>>> mutex_lock(&irq_domain_mutex); > >>>>> list_for_each_entry(h, &irq_domain_list, link) { > >>>>> - if (((h->bus_token & DOMAIN_BUS_PCI_MSI) || > >>>>> - (h->bus_token & DOMAIN_BUS_PLATFORM_MSI) > || > >>>>> - (h->bus_token & DOMAIN_BUS_FSL_MC_MSI)) > && > >>>>> - !irq_domain_is_msi_remap(h)) { > >>>>> + if (irq_domain_is_msi(h) && > !irq_domain_is_msi_remap(h)) { > >>>>> ret = false; > >>>>> goto out; > >>>>> } > >>>>> diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c index > >>>>> ee23006..b637263 100644 > >>>>> --- a/kernel/irq/msi.c > >>>>> +++ b/kernel/irq/msi.c > >>>>> @@ -270,7 +270,7 @@ struct irq_domain > *msi_create_irq_domain(struct fwnode_handle *fwnode, > >>>>> if (info->flags & MSI_FLAG_USE_DEF_CHIP_OPS) > >>>>> msi_domain_update_chip_ops(info); > >>>>> > >>>>> - return irq_domain_create_hierarchy(parent, 0, 0, fwnode, > >>>>> + return irq_domain_create_hierarchy(parent, > IRQ_DOMAIN_FLAG_MSI, > >>>>> +0, fwnode, > >>>>> &msi_domain_ops, info); > >>>>> } > >>>>> > >>>>> > >>>>> > >>>>> Thoughts? > >>>> > >>>> Don't we need to set the IRQ_DOMAIN_FLAG_MSI flag in > >>>> platform_msi_create_device_domain too (drivers/base/platform- > msi.c)? > >> was mentioning platform_msi_create_*device*_domain. > >> it calls irq_domain_create_hierarchy and looks to be MSI irq domain > >> related. But I don't have a full understanding of the whole irq > >> domain hierarchy. > > > > Ah, sorry - I blame the ARM coffee. > > > > This function builds a domain for a single device on top of the MSI > > domain that has been already created (see the dev->msi_domain passed > > to irq_domain_create_hierarchy). The structure looks like this: > > > > device-domain -> platform MSI domain -> HW MSI domain -> whatever > > > > So what we're *really* interested in is the platform MSI domain, which > > is going to carry the IRQ_DOMAIN_FLAG_MSI flag. The device-domain only > > describes a portion of it, and can safely be ignored. > > > > In the end, what matters for this patch is that we can prove that from > > any domain carrying the IRQ_DOMAIN_FLAG_MSI flag, we can find a > domain > > carrying the IRQ_DOMAIN_FLAG_MSI_REMAP flag. If that property holds, > > we're safe. Otherwise, we disable the Guest MSI feature. > > > > Does it make sense? > Yes it makes sense. Thank you for the explanation! If I understood correctly then the domain hierarchy is -> "gic-irq-domain" -> "gic-its-irq-domain" -> "platform-msi-domain" -> "pci-msi-domain" -> "fsl-mc-msi-domain" "gic-its-irq-domain" carries IRQ_DOMAIN_FLAG_MSI_REMAP So we need to look for the IRQ_DOMAIN_FLAG_MSI_REMAP flag in "gic-its-irq-domain" when doing safety check for "platform/pci/fsl-mc"-msi-irqdomain, is this what you mentioned? Can we can pass this flags from "gic-its-irq-domain" to "platform/pci/fsl-mc"-msi-irqdomain during domain creation? Thanks -Bharat > > Eric > > > > Thanks, > > > > M. > >