xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Julien Grall <julien@xen.org>
To: "Roger Pau Monné" <roger.pau@citrix.com>
Cc: Rahul Singh <rahul.singh@arm.com>,
	xen-devel@lists.xenproject.org, bertrand.marquis@arm.com,
	Jan Beulich <jbeulich@suse.com>, Paul Durrant <paul@xen.org>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	George Dunlap <george.dunlap@citrix.com>,
	Ian Jackson <iwj@xenproject.org>,
	Stefano Stabellini <sstabellini@kernel.org>, Wei Liu <wl@xen.org>,
	Daniel De Graaf <dgdegra@tycho.nsa.gov>
Subject: Re: [PATCH 2/2] xen/pci: Gate all MSI code in common code with CONFIG_HAS_PCI_MSI
Date: Tue, 6 Apr 2021 16:09:34 +0100	[thread overview]
Message-ID: <88704bcf-a06b-cf89-5fa3-0db94428f9f8@xen.org> (raw)
In-Reply-To: <YGx3TsTlAuE9eQ7i@Air-de-Roger>

On 06/04/2021 15:59, Roger Pau Monné wrote:
> On Tue, Apr 06, 2021 at 03:30:01PM +0100, Julien Grall wrote:
>> Hi Roger,
>> On 06/04/2021 15:13, Roger Pau Monné wrote:
>>> On Tue, Apr 06, 2021 at 12:39:11PM +0100, Rahul Singh wrote:
>>>> MSI support is not implemented for ARM architecture but it is enabled
>>>> for x86 architecture and referenced in common passthrough/pci.c code.
>>>> Therefore introducing the new flag to gate the MSI code for ARM in
>>>> common code to avoid compilation error when HAS_PCI is enabled for ARM.
>>> Is such option really interesting long term?
>>> IIRC PCI Express mandates MSI support, at which point I don't see much
>>> value in being able to compile out the MSI support.
>> I am pretty sure there are board out with PCI support but no MSI support.
>> Anyway, even if the spec may mandate it...
>>> So while maybe helpful for Arm PCI efforts ATM, I'm not sure it
>>> warrants a Kconfig option, I would rather see Arm introduce dummy
>>> helpers for the missing functionality, even if unimplemented at the
>>> moment.
>> ... from my understanding, most of (if not all) the MSI code is not very
>> useful on Arm when using the GICv3 ITS.
>> The GICv3 ITS will do the isolation for you and therefore we should not need
>> to keep track of the state at the vPCI level.
> Right, but MSI has nothing to do with isolation, is just the
> capability to setup interrupts from PCI devices. What about systems
> without GICv3 ITS, is there an aim to support those also? (as from my
> reading of your reply those would require more auditing of the MSI
> accesses by the guests)

I am not aware of any plan for them so far.

>> So I think we want to be able to compile out the code if not used. That
>> said, I think providing stub would be better to avoid multiple #ifdef in the
>> same function.
> I think providing stubs is the way to go, that should allow to remove
> the unneeded code without having to explicitly drop MSI support. 
> As
> said before, I think it's fine to provide those unimplemented for Arm
> ATM, can be filled later if there's more pressing PCI work to do
> first.

We should remove unneeded and *avoid* allocation. Providing stub for 
existing functions will only address the first problem.

For the allocation (see alloc_pdev()) , we will need to move it in 
separate function and gate them to prevent the allocation.

It would be wrong to gate the code with #ifdef CONFIG_X86. So I think 
Rahul's idea to provide the new #ifdef is correct.


Julien Grall

  reply	other threads:[~2021-04-06 15:10 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-06 11:39 [PATCH 0/2] xen/pci: Make PCI passthrough code non-x86 specific Rahul Singh
2021-04-06 11:39 ` [PATCH 1/2] xen/pci: Move PCI ATS code to common directory Rahul Singh
2021-04-06 15:16   ` Jan Beulich
2021-04-06 11:39 ` [PATCH 2/2] xen/pci: Gate all MSI code in common code with CONFIG_HAS_PCI_MSI Rahul Singh
2021-04-06 14:13   ` Roger Pau Monné
2021-04-06 14:30     ` Julien Grall
2021-04-06 14:59       ` Roger Pau Monné
2021-04-06 15:09         ` Julien Grall [this message]
2021-04-06 15:58           ` Roger Pau Monné
2021-04-07 17:52             ` Rahul Singh
2021-04-06 15:25       ` Jan Beulich
2021-04-07 18:06         ` Julien Grall
2021-04-08  6:00           ` Jan Beulich
2021-04-08  8:45             ` Rahul Singh

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=88704bcf-a06b-cf89-5fa3-0db94428f9f8@xen.org \
    --to=julien@xen.org \
    --cc=andrew.cooper3@citrix.com \
    --cc=bertrand.marquis@arm.com \
    --cc=dgdegra@tycho.nsa.gov \
    --cc=george.dunlap@citrix.com \
    --cc=iwj@xenproject.org \
    --cc=jbeulich@suse.com \
    --cc=paul@xen.org \
    --cc=rahul.singh@arm.com \
    --cc=roger.pau@citrix.com \
    --cc=sstabellini@kernel.org \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    --subject='Re: [PATCH 2/2] xen/pci: Gate all MSI code in common code with CONFIG_HAS_PCI_MSI' \


* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).