-----Original Message-----
From: acrn-dev@... <acrn-dev@...> On Behalf Of Yu Wang
Sent: Wednesday, June 8, 2022 7:50 PM
To: Liu Long <long.liu@...>
Cc: acrn-dev@...; Li, Fei1 <fei1.li@...>
Subject: Re: [acrn-dev] [PATCH] ACRN: DM: Fix the MSI mask and unmask bugs.

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?

[Long:] We will use these fields by address pointer, just like the base + offset.

} __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