* [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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.