On 06/13/2016 06:16 PM, Marcel Apfelbaum wrote:
On 06/10/2016 12:54 PM, Cao jin wrote:
diff --git a/hw/net/e1000e.c b/hw/net/e1000e.c
index 692283f..a06d184 100644
--- a/hw/net/e1000e.c
+++ b/hw/net/e1000e.c
@@ -268,13 +268,9 @@ e1000e_init_msi(E1000EState *s)
{
int res;
- res = msi_init(PCI_DEVICE(s),
- 0xD0, /* MSI capability offset */
- 1, /* MAC MSI interrupts */
- true, /* 64-bit message addresses supported */
- false); /* Per vector mask supported */
+ res = msi_init(PCI_DEVICE(s), 0xD0, 1, true, false, NULL);
Why did you chose to remove author's comments?
Because msi_init() has its function comments now, which is the same is
the author`s comments, I guess author add these comments because
function has no comment before, remove comments also is to save screen
space:)
some macros of some devices is also unnecessary I think, because we have
comments of msi_init().
+
+#define VMXNET3_USE_64BIT (true)
+#define VMXNET3_PER_VECTOR_MASK (false)
+
like these macros, but it does't take too much space as above one I
feel, so I didn't touch them.
@@ -56,14 +57,17 @@ static int xio3130_upstream_initfn(PCIDevice *d)
{
PCIEPort *p = PCIE_PORT(d);
int rc;
+ Error *err = NULL;
pci_bridge_initfn(d, TYPE_PCIE_BUS);
pcie_port_init_reg(d);
rc = msi_init(d, XIO3130_MSI_OFFSET, XIO3130_MSI_NR_VECTOR,
XIO3130_MSI_SUPPORTED_FLAGS & PCI_MSI_FLAGS_64BIT,
- XIO3130_MSI_SUPPORTED_FLAGS & PCI_MSI_FLAGS_MASKBIT);
+ XIO3130_MSI_SUPPORTED_FLAGS &
PCI_MSI_FLAGS_MASKBIT, &err);
if (rc < 0) {
+ assert(rc == -ENOTSUP);
+ error_report_err(err);
Hi Jin, Markus
It looks a little weird to me to check for negative error code
and then assert for a specific error as the only "valid error".
Maybe we should assert inside msi_init to leave the callers "clean"?
Hi Marcel,
I think it is because: except assigned device(vfio), the emulated pci
devices won`t have config space overlapped(-EINVAL), unless programming
error.
If implemented as you said, I guess there need a judgement (if..else..)
to check if current device is assigned in msi_init(), or else assert the
error
I am well aware a lot of time was spent for this series, but I just
spotted it and I want to be sure we are doing it right.
Thanks,
Marcel
--
Yours Sincerely,
Cao jin