xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: "Marek Marczykowski-Górecki" <marmarek@invisiblethingslab.com>
To: xen-devel@lists.xenproject.org
Cc: "Marek Marczykowski-Górecki" <marmarek@invisiblethingslab.com>,
	"Jan Beulich" <jbeulich@suse.com>,
	"Andrew Cooper" <andrew.cooper3@citrix.com>,
	"Roger Pau Monné" <roger.pau@citrix.com>, "Wei Liu" <wl@xen.org>
Subject: [PATCH 2/2] x86/msi: Allow writes to registers on the same page as MSI-X table
Date: Mon, 14 Nov 2022 20:21:00 +0100	[thread overview]
Message-ID: <20221114192100.1539267-2-marmarek@invisiblethingslab.com> (raw)
In-Reply-To: <20221114192100.1539267-1-marmarek@invisiblethingslab.com>

Some devices (notably Intel Wifi 6 AX210 card) keep auxiliary registers
on the same page as MSI-X table. Device model (especially one in
stubdomain) cannot really handle those, as direct writes to that page is
refused (page is on mmio_ro_ranges list). Instead, add internal ioreq
server that handle those writes.

This may be also used to read Pending Bit Array, if it lives on the same
page, making QEMU not needing /dev/mem access at all (especially helpful
with lockdown enabled in dom0). If PBA lives on another page, it can be
(and will be) mapped to the guest directly.
If PBA lives on the same page, forbid writes. Technically, writes outside
of PBA could be allowed, but at this moment the precise location of PBA
isn't saved.

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
 xen/arch/x86/hvm/vmsi.c        | 135 +++++++++++++++++++++++++++++++++
 xen/arch/x86/include/asm/msi.h |   1 +
 xen/arch/x86/msi.c             |  21 +++++
 3 files changed, 157 insertions(+)

diff --git a/xen/arch/x86/hvm/vmsi.c b/xen/arch/x86/hvm/vmsi.c
index ba4cf46dfe91..57cfcf70741e 100644
--- a/xen/arch/x86/hvm/vmsi.c
+++ b/xen/arch/x86/hvm/vmsi.c
@@ -428,6 +428,133 @@ static const struct hvm_io_ops msixtbl_mmio_ops = {
     .write = _msixtbl_write,
 };
 
+static void __iomem *msixtbl_page_handler_get_hwaddr(
+        const struct vcpu *v,
+        uint64_t address,
+        bool write)
+{
+    struct domain *d = v->domain;
+    struct pci_dev *pdev = NULL;
+    struct msixtbl_entry *entry;
+    void __iomem *ret = NULL;
+    uint64_t table_end_addr;
+
+    rcu_read_lock(&msixtbl_rcu_lock);
+    /*
+     * Check if it's on the same page as the end of the MSI-X table, but
+     * outside of the table itself.
+     */
+    list_for_each_entry( entry, &d->arch.hvm.msixtbl_list, list )
+        if ( PFN_DOWN(address) == PFN_DOWN(entry->gtable + entry->table_len) &&
+             address >= entry->gtable + entry->table_len )
+        {
+            pdev = entry->pdev;
+            break;
+        }
+    rcu_read_unlock(&msixtbl_rcu_lock);
+
+    if ( !pdev )
+        return NULL;
+
+    ASSERT( pdev->msix );
+
+    table_end_addr = (pdev->msix->table.first << PAGE_SHIFT) +
+        pdev->msix->nr_entries * PCI_MSIX_ENTRY_SIZE;
+    ASSERT( PFN_DOWN(table_end_addr) == pdev->msix->table.last );
+
+    /* If PBA lives on the same page too, forbid writes. */
+    if ( write && pdev->msix->table.last == pdev->msix->pba.first )
+        return NULL;
+
+    if ( pdev->msix->last_table_page )
+        ret = pdev->msix->last_table_page + (address & (PAGE_SIZE - 1));
+    else
+        gdprintk(XENLOG_WARNING,
+                 "MSI-X last_table_page not initialized for %04x:%02x:%02x.%u\n",
+                 pdev->seg,
+                 pdev->bus,
+                 PCI_SLOT(pdev->devfn),
+                 PCI_FUNC(pdev->devfn));
+
+    return ret;
+}
+
+static bool cf_check msixtbl_page_accept(
+        const struct hvm_io_handler *handler, const ioreq_t *r)
+{
+    unsigned long addr = r->addr;
+
+    ASSERT( r->type == IOREQ_TYPE_COPY );
+
+    return msixtbl_page_handler_get_hwaddr(
+            current, addr, r->dir == IOREQ_WRITE);
+}
+
+static int cf_check msixtbl_page_read(
+        const struct hvm_io_handler *handler,
+        uint64_t address, uint32_t len, uint64_t *pval)
+{
+    void __iomem *hwaddr = msixtbl_page_handler_get_hwaddr(
+            current, address, false);
+
+    if ( !hwaddr )
+        return X86EMUL_UNHANDLEABLE;
+
+    switch ( len ) {
+        case 1:
+            *pval = readb(hwaddr);
+            break;
+        case 2:
+            *pval = readw(hwaddr);
+            break;
+        case 4:
+            *pval = readl(hwaddr);
+            break;
+        case 8:
+            *pval = readq(hwaddr);
+            break;
+        default:
+            return X86EMUL_UNHANDLEABLE;
+    }
+    return X86EMUL_OKAY;
+}
+
+static int cf_check msixtbl_page_write(
+        const struct hvm_io_handler *handler,
+        uint64_t address, uint32_t len, uint64_t val)
+{
+    void __iomem *hwaddr = msixtbl_page_handler_get_hwaddr(
+            current, address, true);
+
+    if ( !hwaddr )
+        return X86EMUL_UNHANDLEABLE;
+
+    switch ( len ) {
+        case 1:
+            writeb(val, hwaddr);
+            break;
+        case 2:
+            writew(val, hwaddr);
+            break;
+        case 4:
+            writel(val, hwaddr);
+            break;
+        case 8:
+            writeq(val, hwaddr);
+            break;
+        default:
+            return X86EMUL_UNHANDLEABLE;
+    }
+    return X86EMUL_OKAY;
+
+}
+
+static const struct hvm_io_ops msixtbl_mmio_page_ops = {
+    .accept = msixtbl_page_accept,
+    .read = msixtbl_page_read,
+    .write = msixtbl_page_write,
+};
+
 static void add_msixtbl_entry(struct domain *d,
                               struct pci_dev *pdev,
                               uint64_t gtable,
@@ -583,6 +710,14 @@ void msixtbl_init(struct domain *d)
         handler->type = IOREQ_TYPE_COPY;
         handler->ops = &msixtbl_mmio_ops;
     }
+
+    /* passthrough access to other registers on the same page */
+    handler = hvm_next_io_handler(d);
+    if ( handler )
+    {
+        handler->type = IOREQ_TYPE_COPY;
+        handler->ops = &msixtbl_mmio_page_ops;
+    }
 }
 
 void msixtbl_pt_cleanup(struct domain *d)
diff --git a/xen/arch/x86/include/asm/msi.h b/xen/arch/x86/include/asm/msi.h
index fe670895eed2..d4287140f04c 100644
--- a/xen/arch/x86/include/asm/msi.h
+++ b/xen/arch/x86/include/asm/msi.h
@@ -236,6 +236,7 @@ struct arch_msix {
     } table, pba;
     int table_refcnt[MAX_MSIX_TABLE_PAGES];
     int table_idx[MAX_MSIX_TABLE_PAGES];
+    void __iomem *last_table_page;
     spinlock_t table_lock;
     bool host_maskall, guest_maskall;
     domid_t warned;
diff --git a/xen/arch/x86/msi.c b/xen/arch/x86/msi.c
index d0bf63df1def..e7fe41f424d8 100644
--- a/xen/arch/x86/msi.c
+++ b/xen/arch/x86/msi.c
@@ -961,6 +961,21 @@ static int msix_capability_init(struct pci_dev *dev,
                 domain_crash(d);
             /* XXX How to deal with existing mappings? */
         }
+
+        /*
+         * If the MSI-X table doesn't span full page(s), map the last page for
+         * passthrough accesses.
+         */
+        if ( (msix->nr_entries * PCI_MSIX_ENTRY_SIZE) & (PAGE_SIZE - 1) )
+        {
+            uint64_t entry_paddr = table_paddr + msix->nr_entries * PCI_MSIX_ENTRY_SIZE;
+            int idx = msix_get_fixmap(msix, table_paddr, entry_paddr);
+
+            if ( idx >= 0 )
+                msix->last_table_page = fix_to_virt(idx);
+            else
+                gprintk(XENLOG_ERR, "Failed to map last MSI-X table page: %d\n", idx);
+        }
     }
     WARN_ON(msix->table.first != (table_paddr >> PAGE_SHIFT));
     ++msix->used_entries;
@@ -1090,6 +1105,12 @@ static void _pci_cleanup_msix(struct arch_msix *msix)
             WARN();
         msix->table.first = 0;
         msix->table.last = 0;
+        if ( msix->last_table_page )
+        {
+            msix_put_fixmap(msix,
+                            virt_to_fix((unsigned long)msix->last_table_page));
+            msix->last_table_page = 0;
+        }
 
         if ( rangeset_remove_range(mmio_ro_ranges, msix->pba.first,
                                    msix->pba.last) )
-- 
2.37.3



  reply	other threads:[~2022-11-14 19:21 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-14 19:20 [PATCH 1/2] x86/msi: passthrough all MSI-X vector ctrl writes to device model Marek Marczykowski-Górecki
2022-11-14 19:21 ` Marek Marczykowski-Górecki [this message]
2022-11-17 16:34   ` [PATCH 2/2] x86/msi: Allow writes to registers on the same page as MSI-X table Jan Beulich
2022-11-17 17:31     ` Marek Marczykowski-Górecki
2022-11-18  7:20       ` Jan Beulich
2022-11-18 12:19         ` Marek Marczykowski-Górecki
2022-11-18 12:33           ` Jan Beulich
2022-11-18 13:00             ` Marek Marczykowski-Górecki
2022-11-18 13:07               ` Jan Beulich
2022-11-15  9:36 ` [PATCH 1/2] x86/msi: passthrough all MSI-X vector ctrl writes to device model Jan Beulich
2022-11-15 11:37   ` Marek Marczykowski-Górecki
2022-11-15 13:54     ` Jan Beulich

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=20221114192100.1539267-2-marmarek@invisiblethingslab.com \
    --to=marmarek@invisiblethingslab.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=jbeulich@suse.com \
    --cc=roger.pau@citrix.com \
    --cc=wl@xen.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 NNTP newsgroup(s).