mbox series

[v3,0/9] x86_iommu/amd: add interrupt remap support

Message ID 1537539920-30662-1-git-send-email-brijesh.singh@amd.com (mailing list archive)
Headers show
Series x86_iommu/amd: add interrupt remap support | expand

Message

Brijesh Singh Sept. 21, 2018, 2:25 p.m. UTC
This series adds the interrupt remapping support for amd-iommu device.

IOMMU spec is available at: https://support.amd.com/TechDocs/48882_IOMMU.pdf

To enable the interrupt remap use below qemu cli
# $QEMU \
  -device amd-iommu,intremap=on

I have tested FC-28 and Ubuntu 18.04 guest. 

Linux guest bootup log shows the interrupt remap supports:

[root@localhost ~]# dmesg | grep -i AMD-Vi
[    0.001761] AMD-Vi: Using IVHD type 0x10
[    0.003051] AMD-Vi: device: 00:03.0 cap: 0040 seg: 0 flags: d1 info 0000
[    0.004007] AMD-Vi:        mmio-addr: 00000000fed80000
[    0.004874] AMD-Vi:   DEV_ALL                        flags: 00
[    0.006236] AMD-Vi:   DEV_SPECIAL(IOAPIC[0])         devid: 00:14.0
[    0.667943] AMD-Vi: Found IOMMU at 0000:00:03.0 cap 0x40
[    0.668727] AMD-Vi: Extended features (0x29d3):
[    0.669874] AMD-Vi: Interrupt remapping enabled
[    0.671074] AMD-Vi: Lazy IO/TLB flushing enabled

cat /proc/interrupts confirms that its using IR

[root@localhost ~]# cat /proc/interrupts 
CPU0       
 0:         40  IR-IO-APIC    2-edge      timer
 1:          9  IR-IO-APIC    1-edge      i8042
 4:       1770  IR-IO-APIC    4-edge      ttyS0
 7:          0  IR-IO-APIC    7-edge      parport0
 8:          1  IR-IO-APIC    8-edge      rtc0
 9:          0  IR-IO-APIC    9-fasteoi   acpi
12:         15  IR-IO-APIC   12-edge      i8042
16:          0  IR-IO-APIC   16-fasteoi   i801_smbus
24:          0   PCI-MSI 49152-edge      AMD-Vi
25:      13070  IR-PCI-MSI 512000-edge      ahci[0000:00:1f.2]
26:         86  IR-PCI-MSI 32768-edge      enp0s2-rx-0
27:        139  IR-PCI-MSI 32769-edge      enp0s2-tx-0
28:          1  IR-PCI-MSI 32770-edge      enp0s2
NMI:          0   Non-maskable interrupts
LOC:      26686   Local timer interrupts
SPU:          0   Spurious interrupts
...
...

Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Cc: Tom Lendacky <Thomas.Lendacky@amd.com>
Cc: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
Cc: Peter Xu <peterx@redhat.com>

Changes since v2:
- make the address space rename a separate patch
- fix the V=1 check in patch 2
- add more comments in the patches
- use amdvi_target_abort trace point where applicable
- use error_report_once where applicable
- do not cause exit() when configuration mismatch is detected,
  but log the error so that user knows about it.

Changes since v1:
- move vtd_generate_msi_message to common code
- fix the dest_mode bit extraction
- add more comments explaining why we add the special device
- some minor cleanups based on Peter's feedbacks

Brijesh Singh (9):
  x86_iommu: move the kernel-irqchip check in common code
  x86_iommu: move vtd_generate_msi_message in common file
  x86_iommu/amd: remove V=1 check from amdvi_validate_dte()
  x86_iommu/amd: make the address space naming consistent with
    intel-iommu
  x86_iommu/amd: Prepare for interrupt remap support
  x86_iommu/amd: Add interrupt remap support when VAPIC is not enabled
  i386: acpi: add IVHD device entry for IOAPIC
  x86_iommu/amd: Add interrupt remap support when VAPIC is enabled
  x86_iommu/amd: Enable Guest virtual APIC support

 hw/i386/acpi-build.c          |  31 +++-
 hw/i386/amd_iommu.c           | 413 +++++++++++++++++++++++++++++++++++++++++-
 hw/i386/amd_iommu.h           |  96 +++++++++-
 hw/i386/intel_iommu.c         |  39 +---
 hw/i386/trace-events          |  14 ++
 hw/i386/x86-iommu.c           |  33 ++++
 include/hw/i386/intel_iommu.h |  59 ------
 include/hw/i386/x86-iommu.h   |  66 +++++++
 8 files changed, 642 insertions(+), 109 deletions(-)

Comments

Peter Xu Sept. 25, 2018, 6:57 a.m. UTC | #1
On Fri, Sep 21, 2018 at 02:25:42PM +0000, Singh, Brijesh wrote:

[...]

> +static int amdvi_int_remap_ga(AMDVIState *iommu,
> +                              MSIMessage *origin,
> +                              MSIMessage *translated,
> +                              uint64_t *dte,
> +                              X86IOMMUIrq *irq,
> +                              uint16_t sid)
> +{
> +    int ret;
> +    struct irte_ga irte;
> +
> +    /* get interrupt remapping table */
> +    ret = amdvi_get_irte_ga(iommu, origin, dte, &irte, sid);
> +    if (ret < 0) {
> +        return ret;
> +    }
> +
> +    if (!irte.lo.fields_remap.valid) {
> +        trace_amdvi_ir_target_abort("RemapEn is disabled");
> +        return -AMDVI_IR_TARGET_ABORT;
> +    }
> +
> +    if (irte.lo.fields_remap.guest_mode) {
> +        trace_amdvi_ir_err("guest mode is not zero");

Just to let you know that you mentioned about replacing some of the
traces with error_report_once() but I didn't see any in this latest
version.  Again that's optional so traces would be fine, but I'm still
pointing that out in case you wanted that and you're posting a wrong
tree.

Otherwise it looks good to me.

Thanks,
Brijesh Singh Sept. 27, 2018, 12:36 p.m. UTC | #2
On 9/25/18 1:57 AM, Peter Xu wrote:
> On Fri, Sep 21, 2018 at 02:25:42PM +0000, Singh, Brijesh wrote:
>
> [...]
>
>> +static int amdvi_int_remap_ga(AMDVIState *iommu,
>> +                              MSIMessage *origin,
>> +                              MSIMessage *translated,
>> +                              uint64_t *dte,
>> +                              X86IOMMUIrq *irq,
>> +                              uint16_t sid)
>> +{
>> +    int ret;
>> +    struct irte_ga irte;
>> +
>> +    /* get interrupt remapping table */
>> +    ret = amdvi_get_irte_ga(iommu, origin, dte, &irte, sid);
>> +    if (ret < 0) {
>> +        return ret;
>> +    }
>> +
>> +    if (!irte.lo.fields_remap.valid) {
>> +        trace_amdvi_ir_target_abort("RemapEn is disabled");
>> +        return -AMDVI_IR_TARGET_ABORT;
>> +    }
>> +
>> +    if (irte.lo.fields_remap.guest_mode) {
>> +        trace_amdvi_ir_err("guest mode is not zero");
> Just to let you know that you mentioned about replacing some of the
> traces with error_report_once() but I didn't see any in this latest
> version.  Again that's optional so traces would be fine, but I'm still
> pointing that out in case you wanted that and you're posting a wrong
> tree.

Hmm, let me go back to my patches again, i thought I used
error_report_once() in few places but it seems those are lost somewhere.
thanks for pointing this out.

> Otherwise it looks good to me.
>
> Thanks,
>