diff mbox series

[XEN,v11,3/8] x86/pvh: Add PHYSDEVOP_setup_gsi for PVH dom0

Message ID 20240630123344.20623-4-Jiqian.Chen@amd.com (mailing list archive)
State Superseded
Headers show
Series Support device passthrough when dom0 is PVH on Xen | expand

Commit Message

Chen, Jiqian June 30, 2024, 12:33 p.m. UTC
The gsi of a passthrough device must be configured for it to be
able to be mapped into a hvm domU.
But When dom0 is PVH, the gsis don't get registered, it causes
the info of apic, pin and irq not be added into irq_2_pin list,
and the handler of irq_desc is not set, then when passthrough a
device, setting ioapic affinity and vector will fail.

To fix above problem, on Linux kernel side, a new code will
need to call PHYSDEVOP_setup_gsi for passthrough devices to
register gsi when dom0 is PVH.

So, add PHYSDEVOP_setup_gsi into hvm_physdev_op for above
purpose.

Clarify two questions:
First, why the gsi of devices belong to PVH dom0 can work?
Because when probe a driver to a normal device, it calls(on linux
kernel side) pci_device_probe-> request_threaded_irq->
irq_startup-> __unmask_ioapic-> io_apic_write, then trap into xen
side hvmemul_do_io-> hvm_io_intercept-> hvm_process_io_intercept->
vioapic_write_indirect-> vioapic_hwdom_map_gsi-> mp_register_gsi.
So that the gsi can be registered.

Second, why the gsi of passthrough device can't work when dom0
is PVH?
Because when assign a device to passthrough, it uses pciback to
probe the device, and it calls pcistub_probe->pcistub_seize->
pcistub_init_device-> xen_pcibk_reset_device->
xen_pcibk_control_isr->isr_on, but isr_on is not set, so that the
fake IRQ handler is not installed, then the gsi isn't unmasked.
What's more, we can see on Xen side, the function
vioapic_hwdom_map_gsi-> mp_register_gsi will be called only when
the gsi is unmasked, so that the gsi can't work for passthrough
device.

Signed-off-by: Jiqian Chen <Jiqian.Chen@amd.com>
Signed-off-by: Huang Rui <ray.huang@amd.com>
Signed-off-by: Jiqian Chen <Jiqian.Chen@amd.com>
---
 xen/arch/x86/hvm/hypercall.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Jan Beulich July 1, 2024, 7:52 a.m. UTC | #1
On 30.06.2024 14:33, Jiqian Chen wrote:
> The gsi of a passthrough device must be configured for it to be
> able to be mapped into a hvm domU.
> But When dom0 is PVH, the gsis don't get registered, it causes

As per below, it's not "don't" but "may not". As the details don't
follow right away, you may also want to add something like "(see
below)".

> the info of apic, pin and irq not be added into irq_2_pin list,
> and the handler of irq_desc is not set, then when passthrough a
> device, setting ioapic affinity and vector will fail.
> 
> To fix above problem, on Linux kernel side, a new code will
> need to call PHYSDEVOP_setup_gsi for passthrough devices to
> register gsi when dom0 is PVH.
> 
> So, add PHYSDEVOP_setup_gsi into hvm_physdev_op for above
> purpose.
> 
> Clarify two questions:
> First, why the gsi of devices belong to PVH dom0 can work?
> Because when probe a driver to a normal device, it calls(on linux
> kernel side) pci_device_probe-> request_threaded_irq->
> irq_startup-> __unmask_ioapic-> io_apic_write, then trap into xen
> side hvmemul_do_io-> hvm_io_intercept-> hvm_process_io_intercept->
> vioapic_write_indirect-> vioapic_hwdom_map_gsi-> mp_register_gsi.
> So that the gsi can be registered.
> 
> Second, why the gsi of passthrough device can't work when dom0
> is PVH?
> Because when assign a device to passthrough, it uses pciback to
> probe the device, and it calls pcistub_probe->pcistub_seize->
> pcistub_init_device-> xen_pcibk_reset_device->
> xen_pcibk_control_isr->isr_on, but isr_on is not set, so that the
> fake IRQ handler is not installed, then the gsi isn't unmasked.
> What's more, we can see on Xen side, the function
> vioapic_hwdom_map_gsi-> mp_register_gsi will be called only when
> the gsi is unmasked, so that the gsi can't work for passthrough
> device.

While this provides the requested detail (thanks), personally I find
this pretty hard to follow. It would likely be easier if it was
written to a larger part in English, rather than in call chain
terminology. But I'm not going to insist, unless others would agree
with that view of mine.

Jan
Chen, Jiqian July 2, 2024, 3:21 a.m. UTC | #2
On 2024/7/1 15:52, Jan Beulich wrote:
> On 30.06.2024 14:33, Jiqian Chen wrote:
>> The gsi of a passthrough device must be configured for it to be
>> able to be mapped into a hvm domU.
>> But When dom0 is PVH, the gsis don't get registered, it causes
> 
> As per below, it's not "don't" but "may not". As the details don't
> follow right away, you may also want to add something like "(see
> below)".
OK, will change in next version.

> 
>> the info of apic, pin and irq not be added into irq_2_pin list,
>> and the handler of irq_desc is not set, then when passthrough a
>> device, setting ioapic affinity and vector will fail.
>>
>> To fix above problem, on Linux kernel side, a new code will
>> need to call PHYSDEVOP_setup_gsi for passthrough devices to
>> register gsi when dom0 is PVH.
>>
>> So, add PHYSDEVOP_setup_gsi into hvm_physdev_op for above
>> purpose.
>>
>> Clarify two questions:
>> First, why the gsi of devices belong to PVH dom0 can work?
>> Because when probe a driver to a normal device, it calls(on linux
>> kernel side) pci_device_probe-> request_threaded_irq->
>> irq_startup-> __unmask_ioapic-> io_apic_write, then trap into xen
>> side hvmemul_do_io-> hvm_io_intercept-> hvm_process_io_intercept->
>> vioapic_write_indirect-> vioapic_hwdom_map_gsi-> mp_register_gsi.
>> So that the gsi can be registered.
>>
>> Second, why the gsi of passthrough device can't work when dom0
>> is PVH?
>> Because when assign a device to passthrough, it uses pciback to
>> probe the device, and it calls pcistub_probe->pcistub_seize->
>> pcistub_init_device-> xen_pcibk_reset_device->
>> xen_pcibk_control_isr->isr_on, but isr_on is not set, so that the
>> fake IRQ handler is not installed, then the gsi isn't unmasked.
>> What's more, we can see on Xen side, the function
>> vioapic_hwdom_map_gsi-> mp_register_gsi will be called only when
>> the gsi is unmasked, so that the gsi can't work for passthrough
>> device.
> 
> While this provides the requested detail (thanks), personally I find
> this pretty hard to follow. It would likely be easier if it was
> written to a larger part in English, rather than in call chain
> terminology. But I'm not going to insist, unless others would agree
> with that view of mine.
I will add the language description in next version, and also keep the call stack if not necessary to remove.

> 
> Jan
diff mbox series

Patch

diff --git a/xen/arch/x86/hvm/hypercall.c b/xen/arch/x86/hvm/hypercall.c
index 03ada3c880bd..cfe82d0f96ed 100644
--- a/xen/arch/x86/hvm/hypercall.c
+++ b/xen/arch/x86/hvm/hypercall.c
@@ -86,6 +86,7 @@  long hvm_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
             return -ENOSYS;
         break;
 
+    case PHYSDEVOP_setup_gsi:
     case PHYSDEVOP_pci_mmcfg_reserved:
     case PHYSDEVOP_pci_device_add:
     case PHYSDEVOP_pci_device_remove: