Re: [PATCH] ACRN: DM: Fix the MSI mask and unmask bugs.


Yu Wang
 

On Wed, Jun 08, 2022 at 04:35:07PM +0800, Liu Long wrote:
The patch fix the msi clear pending bit issue, add new struct element
in the msicap struct, we will use this element by address pointer.

Signed-off-by: Liu Long <long.liu@...>
---
devicemodel/hw/pci/core.c | 13 ++++++-------
devicemodel/include/pci_core.h | 5 ++++-
2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/devicemodel/hw/pci/core.c b/devicemodel/hw/pci/core.c
index 4f52a76a4..e44045b12 100644
--- a/devicemodel/hw/pci/core.c
+++ b/devicemodel/hw/pci/core.c
@@ -961,7 +961,7 @@ pci_emul_add_capability(struct pci_vdev *dev, u_char *capdata, int caplen)
int
pci_emul_find_capability(struct pci_vdev *dev, uint8_t capid, int *p_capoff)
{
- int coff;
+ int coff = 0;
uint16_t sts;

sts = pci_get_cfgdata16(dev, PCIR_STATUS);
@@ -1069,7 +1069,7 @@ static int
pci_access_msi(struct pci_vdev *dev, int msi_cap, uint32_t *val, bool is_write)
{
uint16_t msgctrl;
- int rc, offset;
+ int rc, offset = 0;

if (msi_cap > PCIR_MSI_PENDING) {
pr_err("%s: Msi capability length is out of msi length!\n", __func__);
@@ -1079,7 +1079,7 @@ pci_access_msi(struct pci_vdev *dev, int msi_cap, uint32_t *val, bool is_write)
if (rc)
return -1;

- msgctrl = pci_get_cfgdata16(dev, offset);
+ msgctrl = pci_get_cfgdata16(dev, offset + 2);
offset + PCIR_MSI_CTRL

if (msgctrl & PCIM_MSICTRL_64BIT)
offset = offset + msi_cap;
else
@@ -1088,8 +1088,7 @@ pci_access_msi(struct pci_vdev *dev, int msi_cap, uint32_t *val, bool is_write)
if (is_write)
pci_set_cfgdata32(dev, offset, *val);
else
- *val = pci_get_cfgdata16(dev, offset);
-
+ *val = pci_get_cfgdata32(dev, offset);
return 0;
}

@@ -1133,7 +1132,7 @@ pci_set_msi_pending(struct pci_vdev *dev, uint32_t index, bool set)
if (set)
val = (1 << index) | val;
else
- val = (~(1 << index)) | val;
+ val = (~(1 << index)) & val;
pci_access_msi(dev, PCIR_MSI_PENDING, &val, true);
}

@@ -1160,7 +1159,7 @@ pci_populate_msicap(struct msicap *msicap, int msgnum, int nextptr)
int
pci_emul_add_msicap(struct pci_vdev *dev, int msgnum)
{
- struct msicap msicap;
+ struct msicap msicap = {0};

return pci_populate_msicap(&msicap, msgnum, 0) ||
pci_emul_add_capability(dev, (u_char *)&msicap, sizeof(msicap));
diff --git a/devicemodel/include/pci_core.h b/devicemodel/include/pci_core.h
index 5aa562c3b..f24bb33bf 100644
--- a/devicemodel/include/pci_core.h
+++ b/devicemodel/include/pci_core.h
@@ -200,8 +200,11 @@ struct msicap {
uint32_t addrlo;
uint32_t addrhi;
uint16_t msgdata;
+ uint16_t reserve;
+ uint32_t maskbit;
+ uint32_t pendbit;
If you add such fields into the msicap, then we should use them to check
mask and pending? Otherwise, what're their meaning?

} __attribute__((packed));
-static_assert(sizeof(struct msicap) == 14, "compile-time assertion failed");
+static_assert(sizeof(struct msicap) == 24, "compile-time assertion failed");

struct msixcap {
uint8_t capid;
--
2.25.1

Join acrn-dev@lists.projectacrn.org to automatically receive all group messages.