[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 6/9] x86_iommu/amd: Add interrupt remap suppo
From: |
Michael S. Tsirkin |
Subject: |
Re: [Qemu-devel] [PATCH v3 6/9] x86_iommu/amd: Add interrupt remap support when VAPIC is not enabled |
Date: |
Fri, 28 Sep 2018 10:08:36 -0400 |
On Fri, Sep 28, 2018 at 11:12:37AM +0800, Peter Xu wrote:
> On Thu, Sep 27, 2018 at 12:28:42PM +0000, Singh, Brijesh wrote:
> > >> +static bool amdvi_validate_int_remap(AMDVIState *s, uint64_t *dte)
> > >> +{
> > >> + /* Check if IR is enabled in DTE */
> > >> + if (!(dte[2] & AMDVI_IR_REMAP_ENABLE)) {
> > >> + return false;
> > >> + }
> > >> +
> > >> + /* validate that we are configure with intremap=on */
> > >> + if (!X86_IOMMU_DEVICE(s)->intr_supported) {
> > >> + error_report("Interrupt remapping is enabled in the guest but "
> > >> + "not in the host. Use intremap=on to enable
> > >> interrupt "
> > >> + "remapping in amd-iommu.");
> > > Just to make sure: we should never get this with Linux, right? Since
> > > Linux should try to detect IOAPIC device first before enabling IR.
> >
> > Yes, this should *never* happen with Linux.
>
> Thanks for the clarification. Then I think this is still a good
> candidate for error_report_once() otherwise we might face DOS attack
> from the guest (or use trace if you prefer).
Trace sounds better.
> > >> /* Interrupt remapping for MSI/MSI-X entry */
> > >> static int amdvi_int_remap_msi(AMDVIState *iommu,
> > >> MSIMessage *origin,
> > >> MSIMessage *translated,
> > >> uint16_t sid)
> > >> {
> > >> + int ret = 0;
> > >> + uint64_t pass = 0;
> > >> + uint64_t dte[4] = { 0 };
> > >> + X86IOMMUIrq irq = { 0 };
> > >> + uint8_t dest_mode, delivery_mode;
> > >> +
> > >> assert(origin && translated);
> > >>
> > >> + /*
> > >> + * When IOMMU is enabled, interrupt remap request will come either
> > >> from
> > >> + * IO-APIC or PCI device. If interrupt is from PCI device then it
> > >> will
> > >> + * have a valid requester id but if the interrupt is from IO-APIC
> > >> + * then requester id will be invalid.
> > >> + */
> > >> + if (sid == X86_IOMMU_SID_INVALID) {
> > >> + sid = AMDVI_IOAPIC_SB_DEVID;
> > >> + }
> > >> +
> > >> trace_amdvi_ir_remap_msi_req(origin->address, origin->data, sid);
> > >>
> > >> - if (!iommu || !X86_IOMMU_DEVICE(iommu)->intr_supported) {
> > >> + /* verify that interrupt remapping is enabled before going further.
> > >> */
> > >> + if (!iommu ||
> > >> + !amdvi_get_dte(iommu, sid, dte) ||
> > >> + !amdvi_validate_int_remap(iommu, dte)) {
> > > I'll have similar question as I left in patch 3 - IMHO we should have
> > > three paths rather than two here:
> > >
> > > - IR enabled
> > > - passthrough
> > > - error
> > >
> > > The "error" path seems missing, and we're treating all the error path
> > > as "passthrough" as well. Is this really what you want?
> >
> > I will see what I can do to not "passthrough" messages in error case.
> > There are only two error cases here:
> >
> > 1) if reserved bits are set in DTE
> > 2) if guest OS has enabled IR when intremap=off in amd-iommu.
> >
> > Other than the above two cases everything should be pass through.
>
> For interrupts, I _guess_ we should just drop the interrupt and
> report. For the reporting part, for sure you can still use either
> trace or error_report_once() on QEMU side, meanwhile reporting this to
> the guest kernel if AMD has such a feature (I didn't dig).
>
> Regards,
>
> --
> Peter Xu
- Re: [Qemu-devel] [PATCH v3 3/9] x86_iommu/amd: remove V=1 check from amdvi_validate_dte(), (continued)
- [Qemu-devel] [PATCH v3 2/9] x86_iommu: move vtd_generate_msi_message in common file, Singh, Brijesh, 2018/09/21
- [Qemu-devel] [PATCH v3 4/9] x86_iommu/amd: make the address space naming consistent with intel-iommu, Singh, Brijesh, 2018/09/21
- [Qemu-devel] [PATCH v3 1/9] x86_iommu: move the kernel-irqchip check in common code, Singh, Brijesh, 2018/09/21
- [Qemu-devel] [PATCH v3 5/9] x86_iommu/amd: Prepare for interrupt remap support, Singh, Brijesh, 2018/09/21
- [Qemu-devel] [PATCH v3 6/9] x86_iommu/amd: Add interrupt remap support when VAPIC is not enabled, Singh, Brijesh, 2018/09/21
[Qemu-devel] [PATCH v3 7/9] i386: acpi: add IVHD device entry for IOAPIC, Singh, Brijesh, 2018/09/21
[Qemu-devel] [PATCH v3 9/9] x86_iommu/amd: Enable Guest virtual APIC support, Singh, Brijesh, 2018/09/21
Message not available