From: "Jürgen Groß" <jgross@suse.com>
To: "Chen, Jiqian" <Jiqian.Chen@amd.com>,
Stefano Stabellini <sstabellini@kernel.org>
Cc: "Bjorn Helgaas" <bhelgaas@google.com>,
"Rafael J . Wysocki" <rafael@kernel.org>,
"Roger Pau Monné" <roger.pau@citrix.com>,
"xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
"linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"linux-acpi@vger.kernel.org" <linux-acpi@vger.kernel.org>,
"Huang, Ray" <Ray.Huang@amd.com>
Subject: Re: [RFC KERNEL PATCH v6 3/3] xen/privcmd: Add new syscall to get gsi from irq
Date: Mon, 13 May 2024 09:59:34 +0200 [thread overview]
Message-ID: <55bbd23b-d851-4274-9511-c5a7af938a07@suse.com> (raw)
In-Reply-To: <BL1PR12MB5849496A5B3148D162787961E7E22@BL1PR12MB5849.namprd12.prod.outlook.com>
On 13.05.24 09:47, Chen, Jiqian wrote:
> Hi,
> On 2024/5/10 17:06, Chen, Jiqian wrote:
>> Hi,
>>
>> On 2024/5/10 14:46, Jürgen Groß wrote:
>>> On 19.04.24 05:36, Jiqian Chen wrote:
>>>> In PVH dom0, it uses the linux local interrupt mechanism,
>>>> when it allocs irq for a gsi, it is dynamic, and follow
>>>> the principle of applying first, distributing first. And
>>>> the irq number is alloced from small to large, but the
>>>> applying gsi number is not, may gsi 38 comes before gsi 28,
>>>> it causes the irq number is not equal with the gsi number.
>>>> And when passthrough a device, QEMU will use device's gsi
>>>> number to do pirq mapping, but the gsi number is got from
>>>> file /sys/bus/pci/devices/<sbdf>/irq, irq!= gsi, so it will
>>>> fail when mapping.
>>>> And in current linux codes, there is no method to translate
>>>> irq to gsi for userspace.
>>>>
>>>> For above purpose, record the relationship of gsi and irq
>>>> when PVH dom0 do acpi_register_gsi_ioapic for devices and
>>>> adds a new syscall into privcmd to let userspace can get
>>>> that translation when they have a need.
>>>>
>>>> Co-developed-by: Huang Rui <ray.huang@amd.com>
>>>> Signed-off-by: Jiqian Chen <Jiqian.Chen@amd.com>
>>>> ---
>>>> arch/x86/include/asm/apic.h | 8 +++++++
>>>> arch/x86/include/asm/xen/pci.h | 5 ++++
>>>> arch/x86/kernel/acpi/boot.c | 2 +-
>>>> arch/x86/pci/xen.c | 21 +++++++++++++++++
>>>> drivers/xen/events/events_base.c | 39 ++++++++++++++++++++++++++++++++
>>>> drivers/xen/privcmd.c | 19 ++++++++++++++++
>>>> include/uapi/xen/privcmd.h | 7 ++++++
>>>> include/xen/events.h | 5 ++++
>>>> 8 files changed, 105 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
>>>> index 9d159b771dc8..dd4139250895 100644
>>>> --- a/arch/x86/include/asm/apic.h
>>>> +++ b/arch/x86/include/asm/apic.h
>>>> @@ -169,6 +169,9 @@ extern bool apic_needs_pit(void);
>>>> extern void apic_send_IPI_allbutself(unsigned int vector);
>>>> +extern int acpi_register_gsi_ioapic(struct device *dev, u32 gsi,
>>>> + int trigger, int polarity);
>>>> +
>>>> #else /* !CONFIG_X86_LOCAL_APIC */
>>>> static inline void lapic_shutdown(void) { }
>>>> #define local_apic_timer_c2_ok 1
>>>> @@ -183,6 +186,11 @@ static inline void apic_intr_mode_init(void) { }
>>>> static inline void lapic_assign_system_vectors(void) { }
>>>> static inline void lapic_assign_legacy_vector(unsigned int i, bool r) { }
>>>> static inline bool apic_needs_pit(void) { return true; }
>>>> +static inline int acpi_register_gsi_ioapic(struct device *dev, u32 gsi,
>>>> + int trigger, int polarity)
>>>> +{
>>>> + return (int)gsi;
>>>> +}
>>>> #endif /* !CONFIG_X86_LOCAL_APIC */
>>>> #ifdef CONFIG_X86_X2APIC
>>>> diff --git a/arch/x86/include/asm/xen/pci.h b/arch/x86/include/asm/xen/pci.h
>>>> index 9015b888edd6..aa8ded61fc2d 100644
>>>> --- a/arch/x86/include/asm/xen/pci.h
>>>> +++ b/arch/x86/include/asm/xen/pci.h
>>>> @@ -5,6 +5,7 @@
>>>> #if defined(CONFIG_PCI_XEN)
>>>> extern int __init pci_xen_init(void);
>>>> extern int __init pci_xen_hvm_init(void);
>>>> +extern int __init pci_xen_pvh_init(void);
>>>> #define pci_xen 1
>>>> #else
>>>> #define pci_xen 0
>>>> @@ -13,6 +14,10 @@ static inline int pci_xen_hvm_init(void)
>>>> {
>>>> return -1;
>>>> }
>>>> +static inline int pci_xen_pvh_init(void)
>>>> +{
>>>> + return -1;
>>>> +}
>>>> #endif
>>>> #ifdef CONFIG_XEN_PV_DOM0
>>>> int __init pci_xen_initial_domain(void);
>>>> diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
>>>> index 85a3ce2a3666..72c73458c083 100644
>>>> --- a/arch/x86/kernel/acpi/boot.c
>>>> +++ b/arch/x86/kernel/acpi/boot.c
>>>> @@ -749,7 +749,7 @@ static int acpi_register_gsi_pic(struct device *dev, u32 gsi,
>>>> }
>>>> #ifdef CONFIG_X86_LOCAL_APIC
>>>> -static int acpi_register_gsi_ioapic(struct device *dev, u32 gsi,
>>>> +int acpi_register_gsi_ioapic(struct device *dev, u32 gsi,
>>>> int trigger, int polarity)
>>>> {
>>>> int irq = gsi;
>>>> diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c
>>>> index 652cd53e77f6..f056ab5c0a06 100644
>>>> --- a/arch/x86/pci/xen.c
>>>> +++ b/arch/x86/pci/xen.c
>>>> @@ -114,6 +114,21 @@ static int acpi_register_gsi_xen_hvm(struct device *dev, u32 gsi,
>>>> false /* no mapping of GSI to PIRQ */);
>>>> }
>>>> +static int acpi_register_gsi_xen_pvh(struct device *dev, u32 gsi,
>>>> + int trigger, int polarity)
>>>> +{
>>>> + int irq;
>>>> +
>>>> + irq = acpi_register_gsi_ioapic(dev, gsi, trigger, polarity);
>>>> + if (irq < 0)
>>>> + return irq;
>>>> +
>>>> + if (xen_pvh_add_gsi_irq_map(gsi, irq) == -EEXIST)
>>>> + printk(KERN_INFO "Already map the GSI :%u and IRQ: %d\n", gsi, irq);
>>>> +
>>>> + return irq;
>>>> +}
>>>> +
>>>> #ifdef CONFIG_XEN_PV_DOM0
>>>> static int xen_register_gsi(u32 gsi, int triggering, int polarity)
>>>> {
>>>> @@ -558,6 +573,12 @@ int __init pci_xen_hvm_init(void)
>>>> return 0;
>>>> }
>>>> +int __init pci_xen_pvh_init(void)
>>>> +{
>>>> + __acpi_register_gsi = acpi_register_gsi_xen_pvh;
>>>
>>> No support for unregistering the gsi again?
>> __acpi_unregister_gsi is set in function acpi_set_irq_model_ioapic.
>> Maybe I need to use a new function to call acpi_unregister_gsi_ioapic and remove the mapping of irq and gsi from xen_irq_list_head ?
> When I tried to support unregistering the gsi and removing the mapping during disable device,
> I encountered that after running "xl pci-assignable-add 03:00.0", callstack pcistub_init_device->xen_pcibk_reset_device->pci_disable_device->pcibios_disable_device->acpi_pci_irq_disable->__acpi_unregister_gsi
> removed the mapping, after that when user space called xen_gsi_from_irq to get gsi, it failed.
>
> To cover above case, I want to change the implementation of xen_gsi_from_irq to pass sbdf to get the gsi instead of passing irq,
> Because the sbdf and gsi of a device is unique and wiil not be changed even device is disabled or re-enabled.
>
> Do you think this kind of change is acceptable?
Yes, I think so.
Juergen
prev parent reply other threads:[~2024-05-13 7:59 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-19 3:36 [RFC KERNEL PATCH v6 0/3] Support device passthrough when dom0 is PVH on Xen Jiqian Chen
2024-04-19 3:36 ` [KERNEL PATCH v6 1/3] xen/pci: Add xen_reset_device_state function Jiqian Chen
2024-04-19 3:36 ` [RFC KERNEL PATCH v6 2/3] xen/pvh: Setup gsi for passthrough device Jiqian Chen
2024-05-10 7:48 ` Juergen Gross
2024-05-10 8:42 ` Chen, Jiqian
2024-04-19 3:36 ` [RFC KERNEL PATCH v6 3/3] xen/privcmd: Add new syscall to get gsi from irq Jiqian Chen
2024-05-10 6:46 ` Jürgen Groß
2024-05-10 9:06 ` Chen, Jiqian
2024-05-10 9:53 ` Jürgen Groß
2024-05-10 10:13 ` Chen, Jiqian
2024-05-10 10:21 ` Jürgen Groß
2024-05-10 10:32 ` Chen, Jiqian
2024-05-10 11:27 ` Jürgen Groß
2024-05-11 2:16 ` Chen, Jiqian
2024-05-13 7:47 ` Chen, Jiqian
2024-05-13 7:59 ` Jürgen Groß [this message]
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:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=55bbd23b-d851-4274-9511-c5a7af938a07@suse.com \
--to=jgross@suse.com \
--cc=Jiqian.Chen@amd.com \
--cc=Ray.Huang@amd.com \
--cc=bhelgaas@google.com \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=rafael@kernel.org \
--cc=roger.pau@citrix.com \
--cc=sstabellini@kernel.org \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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 read-only IMAP folder(s) and NNTP newsgroup(s).