* [PATCH v3 0/2] hw/virtio-pci: AER capability @ 2020-10-05 11:55 andrew 2020-10-05 11:56 ` [PATCH v3 1/2] hw/virtio-pci Added counter for pcie capabilities offsets andrew 2020-10-05 11:56 ` [PATCH v3 2/2] hw/virtio-pci Added AER capability andrew 0 siblings, 2 replies; 6+ messages in thread From: andrew @ 2020-10-05 11:55 UTC (permalink / raw) To: qemu-devel; +Cc: mst From: Andrew Melnychenko <andrew@daynix.com> Now, AER capability for virtio-pci is disabled by default. AER capability is only for PCI with PCIe interface on PCIe bus. During migration - device "realize" should initialize AER if requested by device properties. Also fixed commit message and added proper link to bugzilla. Andrew (2): hw/virtio-pci Added counter for pcie capabilities offsets. hw/virtio-pci Added AER capability. hw/virtio/virtio-pci.c | 20 +++++++++++++++++++- hw/virtio/virtio-pci.h | 4 ++++ 2 files changed, 23 insertions(+), 1 deletion(-) -- 2.28.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v3 1/2] hw/virtio-pci Added counter for pcie capabilities offsets. 2020-10-05 11:55 [PATCH v3 0/2] hw/virtio-pci: AER capability andrew @ 2020-10-05 11:56 ` andrew 2020-10-05 11:56 ` [PATCH v3 2/2] hw/virtio-pci Added AER capability andrew 1 sibling, 0 replies; 6+ messages in thread From: andrew @ 2020-10-05 11:56 UTC (permalink / raw) To: qemu-devel; +Cc: mst From: Andrew <andrew@daynix.com> Removed hardcoded offset for ats. Added cap offset counter for future capabilities like AER. Signed-off-by: Andrew Melnychenko <andrew@daynix.com> --- hw/virtio/virtio-pci.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c index 5bc769f685..ae60c1e249 100644 --- a/hw/virtio/virtio-pci.c +++ b/hw/virtio/virtio-pci.c @@ -1788,6 +1788,7 @@ static void virtio_pci_realize(PCIDevice *pci_dev, Error **errp) if (pcie_port && pci_is_express(pci_dev)) { int pos; + uint16_t last_pcie_cap_offset = PCI_CONFIG_SPACE_SIZE; pos = pcie_endpoint_cap_init(pci_dev, 0); assert(pos > 0); @@ -1823,7 +1824,8 @@ static void virtio_pci_realize(PCIDevice *pci_dev, Error **errp) } if (proxy->flags & VIRTIO_PCI_FLAG_ATS) { - pcie_ats_init(pci_dev, 256); + pcie_ats_init(pci_dev, last_pcie_cap_offset); + last_pcie_cap_offset += PCI_EXT_CAP_ATS_SIZEOF; } if (proxy->flags & VIRTIO_PCI_FLAG_INIT_FLR) { -- 2.28.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v3 2/2] hw/virtio-pci Added AER capability. 2020-10-05 11:55 [PATCH v3 0/2] hw/virtio-pci: AER capability andrew 2020-10-05 11:56 ` [PATCH v3 1/2] hw/virtio-pci Added counter for pcie capabilities offsets andrew @ 2020-10-05 11:56 ` andrew 2020-10-05 17:46 ` Michael S. Tsirkin 1 sibling, 1 reply; 6+ messages in thread From: andrew @ 2020-10-05 11:56 UTC (permalink / raw) To: qemu-devel; +Cc: mst From: Andrew <andrew@daynix.com> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1878465 Added AER capability for virtio-pci devices. Also added property for devices, by default AER is disabled. Signed-off-by: Andrew Melnychenko <andrew@daynix.com> --- hw/virtio/virtio-pci.c | 16 ++++++++++++++++ hw/virtio/virtio-pci.h | 4 ++++ 2 files changed, 20 insertions(+) diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c index ae60c1e249..e0a7936f9c 100644 --- a/hw/virtio/virtio-pci.c +++ b/hw/virtio/virtio-pci.c @@ -1807,6 +1807,12 @@ static void virtio_pci_realize(PCIDevice *pci_dev, Error **errp) */ pci_set_word(pci_dev->config + pos + PCI_PM_PMC, 0x3); + if (proxy->flags & VIRTIO_PCI_FLAG_AER) { + pcie_aer_init(pci_dev, PCI_ERR_VER, last_pcie_cap_offset, + PCI_ERR_SIZEOF, NULL); + last_pcie_cap_offset += PCI_ERR_SIZEOF; + } + if (proxy->flags & VIRTIO_PCI_FLAG_INIT_DEVERR) { /* Init error enabling flags */ pcie_cap_deverr_init(pci_dev); @@ -1848,7 +1854,15 @@ static void virtio_pci_realize(PCIDevice *pci_dev, Error **errp) static void virtio_pci_exit(PCIDevice *pci_dev) { + VirtIOPCIProxy *proxy = VIRTIO_PCI(pci_dev); + bool pcie_port = pci_bus_is_express(pci_get_bus(pci_dev)) && + !pci_bus_is_root(pci_get_bus(pci_dev)); + msix_uninit_exclusive_bar(pci_dev); + if (proxy->flags & VIRTIO_PCI_FLAG_AER && pcie_port && + pci_is_express(pci_dev)) { + pcie_aer_exit(pci_dev); + } } static void virtio_pci_reset(DeviceState *qdev) @@ -1901,6 +1915,8 @@ static Property virtio_pci_properties[] = { VIRTIO_PCI_FLAG_INIT_PM_BIT, true), DEFINE_PROP_BIT("x-pcie-flr-init", VirtIOPCIProxy, flags, VIRTIO_PCI_FLAG_INIT_FLR_BIT, true), + DEFINE_PROP_BIT("aer", VirtIOPCIProxy, flags, + VIRTIO_PCI_FLAG_AER_BIT, false), DEFINE_PROP_END_OF_LIST(), }; diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h index 91096f0291..3986b4f0e3 100644 --- a/hw/virtio/virtio-pci.h +++ b/hw/virtio/virtio-pci.h @@ -45,6 +45,7 @@ enum { VIRTIO_PCI_FLAG_INIT_LNKCTL_BIT, VIRTIO_PCI_FLAG_INIT_PM_BIT, VIRTIO_PCI_FLAG_INIT_FLR_BIT, + VIRTIO_PCI_FLAG_AER_BIT, }; /* Need to activate work-arounds for buggy guests at vmstate load. */ @@ -84,6 +85,9 @@ enum { /* Init Function Level Reset capability */ #define VIRTIO_PCI_FLAG_INIT_FLR (1 << VIRTIO_PCI_FLAG_INIT_FLR_BIT) +/* Advanced Error Reporting capability */ +#define VIRTIO_PCI_FLAG_AER (1 << VIRTIO_PCI_FLAG_AER_BIT) + typedef struct { MSIMessage msg; int virq; -- 2.28.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3 2/2] hw/virtio-pci Added AER capability. 2020-10-05 11:56 ` [PATCH v3 2/2] hw/virtio-pci Added AER capability andrew @ 2020-10-05 17:46 ` Michael S. Tsirkin 2020-10-07 6:41 ` Yan Vugenfirer 2020-10-07 7:13 ` Andrew Melnichenko 0 siblings, 2 replies; 6+ messages in thread From: Michael S. Tsirkin @ 2020-10-05 17:46 UTC (permalink / raw) To: andrew; +Cc: qemu-devel On Mon, Oct 05, 2020 at 02:56:01PM +0300, andrew@daynix.com wrote: > From: Andrew <andrew@daynix.com> > > Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1878465 That's a private bug - what information can you share about the motivation for the patch? > Added AER capability for virtio-pci devices. > Also added property for devices, by default AER is disabled. > > Signed-off-by: Andrew Melnychenko <andrew@daynix.com> > --- > hw/virtio/virtio-pci.c | 16 ++++++++++++++++ > hw/virtio/virtio-pci.h | 4 ++++ > 2 files changed, 20 insertions(+) > > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c > index ae60c1e249..e0a7936f9c 100644 > --- a/hw/virtio/virtio-pci.c > +++ b/hw/virtio/virtio-pci.c > @@ -1807,6 +1807,12 @@ static void virtio_pci_realize(PCIDevice *pci_dev, Error **errp) > */ > pci_set_word(pci_dev->config + pos + PCI_PM_PMC, 0x3); > > + if (proxy->flags & VIRTIO_PCI_FLAG_AER) { > + pcie_aer_init(pci_dev, PCI_ERR_VER, last_pcie_cap_offset, > + PCI_ERR_SIZEOF, NULL); > + last_pcie_cap_offset += PCI_ERR_SIZEOF; > + } > + > if (proxy->flags & VIRTIO_PCI_FLAG_INIT_DEVERR) { > /* Init error enabling flags */ > pcie_cap_deverr_init(pci_dev); > @@ -1848,7 +1854,15 @@ static void virtio_pci_realize(PCIDevice *pci_dev, Error **errp) > > static void virtio_pci_exit(PCIDevice *pci_dev) > { > + VirtIOPCIProxy *proxy = VIRTIO_PCI(pci_dev); > + bool pcie_port = pci_bus_is_express(pci_get_bus(pci_dev)) && > + !pci_bus_is_root(pci_get_bus(pci_dev)); > + > msix_uninit_exclusive_bar(pci_dev); > + if (proxy->flags & VIRTIO_PCI_FLAG_AER && pcie_port && > + pci_is_express(pci_dev)) { > + pcie_aer_exit(pci_dev); > + } > } > > static void virtio_pci_reset(DeviceState *qdev) > @@ -1901,6 +1915,8 @@ static Property virtio_pci_properties[] = { > VIRTIO_PCI_FLAG_INIT_PM_BIT, true), > DEFINE_PROP_BIT("x-pcie-flr-init", VirtIOPCIProxy, flags, > VIRTIO_PCI_FLAG_INIT_FLR_BIT, true), > + DEFINE_PROP_BIT("aer", VirtIOPCIProxy, flags, > + VIRTIO_PCI_FLAG_AER_BIT, false), > DEFINE_PROP_END_OF_LIST(), > }; > Does management need ability to enable this capability? If yes let's cc them. If no let's rename to x-aer so we don't commit to a stable interface ... > diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h > index 91096f0291..3986b4f0e3 100644 > --- a/hw/virtio/virtio-pci.h > +++ b/hw/virtio/virtio-pci.h > @@ -45,6 +45,7 @@ enum { > VIRTIO_PCI_FLAG_INIT_LNKCTL_BIT, > VIRTIO_PCI_FLAG_INIT_PM_BIT, > VIRTIO_PCI_FLAG_INIT_FLR_BIT, > + VIRTIO_PCI_FLAG_AER_BIT, > }; > > /* Need to activate work-arounds for buggy guests at vmstate load. */ > @@ -84,6 +85,9 @@ enum { > /* Init Function Level Reset capability */ > #define VIRTIO_PCI_FLAG_INIT_FLR (1 << VIRTIO_PCI_FLAG_INIT_FLR_BIT) > > +/* Advanced Error Reporting capability */ > +#define VIRTIO_PCI_FLAG_AER (1 << VIRTIO_PCI_FLAG_AER_BIT) > + > typedef struct { > MSIMessage msg; > int virq; > -- > 2.28.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 2/2] hw/virtio-pci Added AER capability. 2020-10-05 17:46 ` Michael S. Tsirkin @ 2020-10-07 6:41 ` Yan Vugenfirer 2020-10-07 7:13 ` Andrew Melnichenko 1 sibling, 0 replies; 6+ messages in thread From: Yan Vugenfirer @ 2020-10-07 6:41 UTC (permalink / raw) To: Michael S. Tsirkin; +Cc: Andrew Melnichenko, qemu-devel Hi Michael, > On 5 Oct 2020, at 8:46 PM, Michael S. Tsirkin <mst@redhat.com> wrote: > > On Mon, Oct 05, 2020 at 02:56:01PM +0300, andrew@daynix.com wrote: >> From: Andrew <andrew@daynix.com> >> >> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1878465 > > That's a private bug - what information can you share about > the motivation for the patch? We need AER feature in order to pass MS WHQL tests for the future Windows Server versions. According to Microsoft driver\device certification requirements for next version of Window Server, PCIe devices must support AER. The exact quote from Microsoft certification requirements: "Windows Server PCI Express devices are required to support Advanced Error Reporting [AER] as defined in PCI Express Base Specification version 2.1.” > >> Added AER capability for virtio-pci devices. >> Also added property for devices, by default AER is disabled. >> >> Signed-off-by: Andrew Melnychenko <andrew@daynix.com> >> --- >> hw/virtio/virtio-pci.c | 16 ++++++++++++++++ >> hw/virtio/virtio-pci.h | 4 ++++ >> 2 files changed, 20 insertions(+) >> >> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c >> index ae60c1e249..e0a7936f9c 100644 >> --- a/hw/virtio/virtio-pci.c >> +++ b/hw/virtio/virtio-pci.c >> @@ -1807,6 +1807,12 @@ static void virtio_pci_realize(PCIDevice *pci_dev, Error **errp) >> */ >> pci_set_word(pci_dev->config + pos + PCI_PM_PMC, 0x3); >> >> + if (proxy->flags & VIRTIO_PCI_FLAG_AER) { >> + pcie_aer_init(pci_dev, PCI_ERR_VER, last_pcie_cap_offset, >> + PCI_ERR_SIZEOF, NULL); >> + last_pcie_cap_offset += PCI_ERR_SIZEOF; >> + } >> + >> if (proxy->flags & VIRTIO_PCI_FLAG_INIT_DEVERR) { >> /* Init error enabling flags */ >> pcie_cap_deverr_init(pci_dev); >> @@ -1848,7 +1854,15 @@ static void virtio_pci_realize(PCIDevice *pci_dev, Error **errp) >> >> static void virtio_pci_exit(PCIDevice *pci_dev) >> { >> + VirtIOPCIProxy *proxy = VIRTIO_PCI(pci_dev); >> + bool pcie_port = pci_bus_is_express(pci_get_bus(pci_dev)) && >> + !pci_bus_is_root(pci_get_bus(pci_dev)); >> + >> msix_uninit_exclusive_bar(pci_dev); >> + if (proxy->flags & VIRTIO_PCI_FLAG_AER && pcie_port && >> + pci_is_express(pci_dev)) { >> + pcie_aer_exit(pci_dev); >> + } >> } >> >> static void virtio_pci_reset(DeviceState *qdev) >> @@ -1901,6 +1915,8 @@ static Property virtio_pci_properties[] = { >> VIRTIO_PCI_FLAG_INIT_PM_BIT, true), >> DEFINE_PROP_BIT("x-pcie-flr-init", VirtIOPCIProxy, flags, >> VIRTIO_PCI_FLAG_INIT_FLR_BIT, true), >> + DEFINE_PROP_BIT("aer", VirtIOPCIProxy, flags, >> + VIRTIO_PCI_FLAG_AER_BIT, false), >> DEFINE_PROP_END_OF_LIST(), >> }; >> > > Does management need ability to enable this capability? > If yes let's cc them. If no let's rename to x-aer so we don't > commit to a stable interface ... QE is using libvirt in order to spawn test setups. So I think it should be used by management as well. Whom should Andrew CC? Best regards, Yan. > > >> diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h >> index 91096f0291..3986b4f0e3 100644 >> --- a/hw/virtio/virtio-pci.h >> +++ b/hw/virtio/virtio-pci.h >> @@ -45,6 +45,7 @@ enum { >> VIRTIO_PCI_FLAG_INIT_LNKCTL_BIT, >> VIRTIO_PCI_FLAG_INIT_PM_BIT, >> VIRTIO_PCI_FLAG_INIT_FLR_BIT, >> + VIRTIO_PCI_FLAG_AER_BIT, >> }; >> >> /* Need to activate work-arounds for buggy guests at vmstate load. */ >> @@ -84,6 +85,9 @@ enum { >> /* Init Function Level Reset capability */ >> #define VIRTIO_PCI_FLAG_INIT_FLR (1 << VIRTIO_PCI_FLAG_INIT_FLR_BIT) >> >> +/* Advanced Error Reporting capability */ >> +#define VIRTIO_PCI_FLAG_AER (1 << VIRTIO_PCI_FLAG_AER_BIT) >> + >> typedef struct { >> MSIMessage msg; >> int virq; >> -- >> 2.28.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 2/2] hw/virtio-pci Added AER capability. 2020-10-05 17:46 ` Michael S. Tsirkin 2020-10-07 6:41 ` Yan Vugenfirer @ 2020-10-07 7:13 ` Andrew Melnichenko 1 sibling, 0 replies; 6+ messages in thread From: Andrew Melnichenko @ 2020-10-07 7:13 UTC (permalink / raw) To: Michael S. Tsirkin; +Cc: Yan Vugenfirer, qemu-devel [-- Attachment #1: Type: text/plain, Size: 4103 bytes --] Ok, Main motivation: > According to Microsoft driver\device certification requirements for next > version of Window Server, PCIe device must support AER. > The exact quote of Microsoft certification requirements: > "Windows Server PCI Express devices are required to support Advanced > Error Reporting [AER] as defined in PCI Express Base Specification version > 2.1.*”* > and > Does management need ability to enable this capability? > Actually, yes. Can you provide their email address? I'll prepare a new patch and I'll remove bugzilla link. On Mon, Oct 5, 2020 at 8:46 PM Michael S. Tsirkin <mst@redhat.com> wrote: > On Mon, Oct 05, 2020 at 02:56:01PM +0300, andrew@daynix.com wrote: > > From: Andrew <andrew@daynix.com> > > > > Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1878465 > > That's a private bug - what information can you share about > the motivation for the patch? > > > Added AER capability for virtio-pci devices. > > Also added property for devices, by default AER is disabled. > > > > Signed-off-by: Andrew Melnychenko <andrew@daynix.com> > > --- > > hw/virtio/virtio-pci.c | 16 ++++++++++++++++ > > hw/virtio/virtio-pci.h | 4 ++++ > > 2 files changed, 20 insertions(+) > > > > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c > > index ae60c1e249..e0a7936f9c 100644 > > --- a/hw/virtio/virtio-pci.c > > +++ b/hw/virtio/virtio-pci.c > > @@ -1807,6 +1807,12 @@ static void virtio_pci_realize(PCIDevice > *pci_dev, Error **errp) > > */ > > pci_set_word(pci_dev->config + pos + PCI_PM_PMC, 0x3); > > > > + if (proxy->flags & VIRTIO_PCI_FLAG_AER) { > > + pcie_aer_init(pci_dev, PCI_ERR_VER, last_pcie_cap_offset, > > + PCI_ERR_SIZEOF, NULL); > > + last_pcie_cap_offset += PCI_ERR_SIZEOF; > > + } > > + > > if (proxy->flags & VIRTIO_PCI_FLAG_INIT_DEVERR) { > > /* Init error enabling flags */ > > pcie_cap_deverr_init(pci_dev); > > @@ -1848,7 +1854,15 @@ static void virtio_pci_realize(PCIDevice > *pci_dev, Error **errp) > > > > static void virtio_pci_exit(PCIDevice *pci_dev) > > { > > + VirtIOPCIProxy *proxy = VIRTIO_PCI(pci_dev); > > + bool pcie_port = pci_bus_is_express(pci_get_bus(pci_dev)) && > > + !pci_bus_is_root(pci_get_bus(pci_dev)); > > + > > msix_uninit_exclusive_bar(pci_dev); > > + if (proxy->flags & VIRTIO_PCI_FLAG_AER && pcie_port && > > + pci_is_express(pci_dev)) { > > + pcie_aer_exit(pci_dev); > > + } > > } > > > > static void virtio_pci_reset(DeviceState *qdev) > > @@ -1901,6 +1915,8 @@ static Property virtio_pci_properties[] = { > > VIRTIO_PCI_FLAG_INIT_PM_BIT, true), > > DEFINE_PROP_BIT("x-pcie-flr-init", VirtIOPCIProxy, flags, > > VIRTIO_PCI_FLAG_INIT_FLR_BIT, true), > > + DEFINE_PROP_BIT("aer", VirtIOPCIProxy, flags, > > + VIRTIO_PCI_FLAG_AER_BIT, false), > > DEFINE_PROP_END_OF_LIST(), > > }; > > > > Does management need ability to enable this capability? > If yes let's cc them. If no let's rename to x-aer so we don't > commit to a stable interface ... > > > > diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h > > index 91096f0291..3986b4f0e3 100644 > > --- a/hw/virtio/virtio-pci.h > > +++ b/hw/virtio/virtio-pci.h > > @@ -45,6 +45,7 @@ enum { > > VIRTIO_PCI_FLAG_INIT_LNKCTL_BIT, > > VIRTIO_PCI_FLAG_INIT_PM_BIT, > > VIRTIO_PCI_FLAG_INIT_FLR_BIT, > > + VIRTIO_PCI_FLAG_AER_BIT, > > }; > > > > /* Need to activate work-arounds for buggy guests at vmstate load. */ > > @@ -84,6 +85,9 @@ enum { > > /* Init Function Level Reset capability */ > > #define VIRTIO_PCI_FLAG_INIT_FLR (1 << VIRTIO_PCI_FLAG_INIT_FLR_BIT) > > > > +/* Advanced Error Reporting capability */ > > +#define VIRTIO_PCI_FLAG_AER (1 << VIRTIO_PCI_FLAG_AER_BIT) > > + > > typedef struct { > > MSIMessage msg; > > int virq; > > -- > > 2.28.0 > > [-- Attachment #2: Type: text/html, Size: 5891 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-10-07 6:46 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-10-05 11:55 [PATCH v3 0/2] hw/virtio-pci: AER capability andrew 2020-10-05 11:56 ` [PATCH v3 1/2] hw/virtio-pci Added counter for pcie capabilities offsets andrew 2020-10-05 11:56 ` [PATCH v3 2/2] hw/virtio-pci Added AER capability andrew 2020-10-05 17:46 ` Michael S. Tsirkin 2020-10-07 6:41 ` Yan Vugenfirer 2020-10-07 7:13 ` Andrew Melnichenko
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).