diff mbox

[RFC,v4,4/9] aer: impove pcie_aer_init to support vfio device

Message ID e2d0acee24691b02791c19efcd57f64251a770ba.1425280224.git.chen.fan.fnst@cn.fujitsu.com
State New
Headers show

Commit Message

chenfan March 2, 2015, 7:16 a.m. UTC 
extend pcie_aer_init arguments to adjust vfio device.

Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
---
 hw/pci-bridge/ioh3420.c            | 3 ++-
 hw/pci-bridge/xio3130_downstream.c | 3 ++-
 hw/pci-bridge/xio3130_upstream.c   | 3 ++-
 hw/pci/pcie_aer.c                  | 7 ++++---
 include/hw/pci/pcie_aer.h          | 4 +++-
 5 files changed, 13 insertions(+), 7 deletions(-)

Comments

Alex Williamson March 9, 2015, 8:29 p.m. UTC | #1 
On Mon, 2015-03-02 at 15:16 +0800, Chen Fan wrote:
> extend pcie_aer_init arguments to adjust vfio device.

Some discussion of why vfio wants this would be useful.


> Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
> ---
>  hw/pci-bridge/ioh3420.c            | 3 ++-
>  hw/pci-bridge/xio3130_downstream.c | 3 ++-
>  hw/pci-bridge/xio3130_upstream.c   | 3 ++-
>  hw/pci/pcie_aer.c                  | 7 ++++---
>  include/hw/pci/pcie_aer.h          | 4 +++-
>  5 files changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/pci-bridge/ioh3420.c b/hw/pci-bridge/ioh3420.c
> index cce2fdd..354574f 100644
> --- a/hw/pci-bridge/ioh3420.c
> +++ b/hw/pci-bridge/ioh3420.c
> @@ -129,7 +129,8 @@ static int ioh3420_initfn(PCIDevice *d)
>          goto err_pcie_cap;
>      }
>      pcie_cap_root_init(d);
> -    rc = pcie_aer_init(d, IOH_EP_AER_OFFSET);
> +    rc = pcie_aer_init(d, PCI_ERR_VER,
> +                       IOH_EP_AER_OFFSET, PCI_ERR_SIZEOF);
>      if (rc < 0) {
>          goto err;
>      }
> diff --git a/hw/pci-bridge/xio3130_downstream.c b/hw/pci-bridge/xio3130_downstream.c
> index b3a6479..407f75f 100644
> --- a/hw/pci-bridge/xio3130_downstream.c
> +++ b/hw/pci-bridge/xio3130_downstream.c
> @@ -92,7 +92,8 @@ static int xio3130_downstream_initfn(PCIDevice *d)
>          goto err_pcie_cap;
>      }
>      pcie_cap_arifwd_init(d);
> -    rc = pcie_aer_init(d, XIO3130_AER_OFFSET);
> +    rc = pcie_aer_init(d, PCI_ERR_VER,
> +                       XIO3130_AER_OFFSET, PCI_ERR_SIZEOF);
>      if (rc < 0) {
>          goto err;
>      }
> diff --git a/hw/pci-bridge/xio3130_upstream.c b/hw/pci-bridge/xio3130_upstream.c
> index eada582..52b130f 100644
> --- a/hw/pci-bridge/xio3130_upstream.c
> +++ b/hw/pci-bridge/xio3130_upstream.c
> @@ -81,7 +81,8 @@ static int xio3130_upstream_initfn(PCIDevice *d)
>      }
>      pcie_cap_flr_init(d);
>      pcie_cap_deverr_init(d);
> -    rc = pcie_aer_init(d, XIO3130_AER_OFFSET);
> +    rc = pcie_aer_init(d, PCI_ERR_VER,
> +                       XIO3130_AER_OFFSET, PCI_ERR_SIZEOF);
>      if (rc < 0) {
>          goto err;
>      }
> diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c
> index ece1487..a76a943 100644
> --- a/hw/pci/pcie_aer.c
> +++ b/hw/pci/pcie_aer.c
> @@ -94,12 +94,13 @@ static void aer_log_clear_all_err(PCIEAERLog *aer_log)
>      aer_log->log_num = 0;
>  }
>  
> -int pcie_aer_init(PCIDevice *dev, uint16_t offset)
> +int pcie_aer_init(PCIDevice *dev, uint8_t cap_ver,
> +                  uint16_t offset, uint16_t size)
>  {
>      PCIExpressDevice *exp;
>  
> -    pcie_add_capability(dev, PCI_EXT_CAP_ID_ERR, PCI_ERR_VER,
> -                        offset, PCI_ERR_SIZEOF);
> +    pcie_add_capability(dev, PCI_EXT_CAP_ID_ERR,
> +                        cap_ver, offset, size);
>      exp = &dev->exp;
>      exp->aer_cap = offset;
>  
> diff --git a/include/hw/pci/pcie_aer.h b/include/hw/pci/pcie_aer.h
> index bcac80a..afa074e 100644
> --- a/include/hw/pci/pcie_aer.h
> +++ b/include/hw/pci/pcie_aer.h
> @@ -87,7 +87,9 @@ struct PCIEAERErr {
>  
>  extern const VMStateDescription vmstate_pcie_aer_log;
>  
> -int pcie_aer_init(PCIDevice *dev, uint16_t offset);
> +int pcie_aer_init(PCIDevice *dev, uint8_t cap_ver,
> +                  uint16_t offset, uint16_t size);
> +
>  void pcie_aer_exit(PCIDevice *dev);
>  void pcie_aer_write_config(PCIDevice *dev,
>                             uint32_t addr, uint32_t val, int len);
chenfan March 11, 2015, 2:37 a.m. UTC | #2 
On 03/10/2015 04:29 AM, Alex Williamson wrote:
> On Mon, 2015-03-02 at 15:16 +0800, Chen Fan wrote:
>> extend pcie_aer_init arguments to adjust vfio device.
> Some discussion of why vfio wants this would be useful.
qemu treats vfio device as an emulated device.
and these attributes of aer can be emulated too.

so here I would use pcie_aer_init to add an aer capability
directly. and get rid of this patch from series.


Thanks,
Chen
>
>
>> Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
>> ---
>>   hw/pci-bridge/ioh3420.c            | 3 ++-
>>   hw/pci-bridge/xio3130_downstream.c | 3 ++-
>>   hw/pci-bridge/xio3130_upstream.c   | 3 ++-
>>   hw/pci/pcie_aer.c                  | 7 ++++---
>>   include/hw/pci/pcie_aer.h          | 4 +++-
>>   5 files changed, 13 insertions(+), 7 deletions(-)
>>
>> diff --git a/hw/pci-bridge/ioh3420.c b/hw/pci-bridge/ioh3420.c
>> index cce2fdd..354574f 100644
>> --- a/hw/pci-bridge/ioh3420.c
>> +++ b/hw/pci-bridge/ioh3420.c
>> @@ -129,7 +129,8 @@ static int ioh3420_initfn(PCIDevice *d)
>>           goto err_pcie_cap;
>>       }
>>       pcie_cap_root_init(d);
>> -    rc = pcie_aer_init(d, IOH_EP_AER_OFFSET);
>> +    rc = pcie_aer_init(d, PCI_ERR_VER,
>> +                       IOH_EP_AER_OFFSET, PCI_ERR_SIZEOF);
>>       if (rc < 0) {
>>           goto err;
>>       }
>> diff --git a/hw/pci-bridge/xio3130_downstream.c b/hw/pci-bridge/xio3130_downstream.c
>> index b3a6479..407f75f 100644
>> --- a/hw/pci-bridge/xio3130_downstream.c
>> +++ b/hw/pci-bridge/xio3130_downstream.c
>> @@ -92,7 +92,8 @@ static int xio3130_downstream_initfn(PCIDevice *d)
>>           goto err_pcie_cap;
>>       }
>>       pcie_cap_arifwd_init(d);
>> -    rc = pcie_aer_init(d, XIO3130_AER_OFFSET);
>> +    rc = pcie_aer_init(d, PCI_ERR_VER,
>> +                       XIO3130_AER_OFFSET, PCI_ERR_SIZEOF);
>>       if (rc < 0) {
>>           goto err;
>>       }
>> diff --git a/hw/pci-bridge/xio3130_upstream.c b/hw/pci-bridge/xio3130_upstream.c
>> index eada582..52b130f 100644
>> --- a/hw/pci-bridge/xio3130_upstream.c
>> +++ b/hw/pci-bridge/xio3130_upstream.c
>> @@ -81,7 +81,8 @@ static int xio3130_upstream_initfn(PCIDevice *d)
>>       }
>>       pcie_cap_flr_init(d);
>>       pcie_cap_deverr_init(d);
>> -    rc = pcie_aer_init(d, XIO3130_AER_OFFSET);
>> +    rc = pcie_aer_init(d, PCI_ERR_VER,
>> +                       XIO3130_AER_OFFSET, PCI_ERR_SIZEOF);
>>       if (rc < 0) {
>>           goto err;
>>       }
>> diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c
>> index ece1487..a76a943 100644
>> --- a/hw/pci/pcie_aer.c
>> +++ b/hw/pci/pcie_aer.c
>> @@ -94,12 +94,13 @@ static void aer_log_clear_all_err(PCIEAERLog *aer_log)
>>       aer_log->log_num = 0;
>>   }
>>   
>> -int pcie_aer_init(PCIDevice *dev, uint16_t offset)
>> +int pcie_aer_init(PCIDevice *dev, uint8_t cap_ver,
>> +                  uint16_t offset, uint16_t size)
>>   {
>>       PCIExpressDevice *exp;
>>   
>> -    pcie_add_capability(dev, PCI_EXT_CAP_ID_ERR, PCI_ERR_VER,
>> -                        offset, PCI_ERR_SIZEOF);
>> +    pcie_add_capability(dev, PCI_EXT_CAP_ID_ERR,
>> +                        cap_ver, offset, size);
>>       exp = &dev->exp;
>>       exp->aer_cap = offset;
>>   
>> diff --git a/include/hw/pci/pcie_aer.h b/include/hw/pci/pcie_aer.h
>> index bcac80a..afa074e 100644
>> --- a/include/hw/pci/pcie_aer.h
>> +++ b/include/hw/pci/pcie_aer.h
>> @@ -87,7 +87,9 @@ struct PCIEAERErr {
>>   
>>   extern const VMStateDescription vmstate_pcie_aer_log;
>>   
>> -int pcie_aer_init(PCIDevice *dev, uint16_t offset);
>> +int pcie_aer_init(PCIDevice *dev, uint8_t cap_ver,
>> +                  uint16_t offset, uint16_t size);
>> +
>>   void pcie_aer_exit(PCIDevice *dev);
>>   void pcie_aer_write_config(PCIDevice *dev,
>>                              uint32_t addr, uint32_t val, int len);
>
>
> .
>
chenfan March 12, 2015, 10:29 a.m. UTC | #3 
On 03/10/2015 04:29 AM, Alex Williamson wrote:
> On Mon, 2015-03-02 at 15:16 +0800, Chen Fan wrote:
>> extend pcie_aer_init arguments to adjust vfio device.
> Some discussion of why vfio wants this would be useful.
I remain this patch at the latest v5. and add the 'size' argument,
because, the aer config space is not fixed.  the TLP Prefix Log Register
is not always implemented. for instance, my local network device:
Ethernet controller: Realtek Semiconductor Co., Ltd. RTL8111/8168B PCI 
Express Gigabit Ethernet controller,
     Capabilities: [100] Advanced Error Reporting
     Capabilities: [140] Virtual Channel

the size is 0x40.
So I think we need a size argument indeed.

Thanks,
Chen




>
>
>> Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
>> ---
>>   hw/pci-bridge/ioh3420.c            | 3 ++-
>>   hw/pci-bridge/xio3130_downstream.c | 3 ++-
>>   hw/pci-bridge/xio3130_upstream.c   | 3 ++-
>>   hw/pci/pcie_aer.c                  | 7 ++++---
>>   include/hw/pci/pcie_aer.h          | 4 +++-
>>   5 files changed, 13 insertions(+), 7 deletions(-)
>>
>> diff --git a/hw/pci-bridge/ioh3420.c b/hw/pci-bridge/ioh3420.c
>> index cce2fdd..354574f 100644
>> --- a/hw/pci-bridge/ioh3420.c
>> +++ b/hw/pci-bridge/ioh3420.c
>> @@ -129,7 +129,8 @@ static int ioh3420_initfn(PCIDevice *d)
>>           goto err_pcie_cap;
>>       }
>>       pcie_cap_root_init(d);
>> -    rc = pcie_aer_init(d, IOH_EP_AER_OFFSET);
>> +    rc = pcie_aer_init(d, PCI_ERR_VER,
>> +                       IOH_EP_AER_OFFSET, PCI_ERR_SIZEOF);
>>       if (rc < 0) {
>>           goto err;
>>       }
>> diff --git a/hw/pci-bridge/xio3130_downstream.c b/hw/pci-bridge/xio3130_downstream.c
>> index b3a6479..407f75f 100644
>> --- a/hw/pci-bridge/xio3130_downstream.c
>> +++ b/hw/pci-bridge/xio3130_downstream.c
>> @@ -92,7 +92,8 @@ static int xio3130_downstream_initfn(PCIDevice *d)
>>           goto err_pcie_cap;
>>       }
>>       pcie_cap_arifwd_init(d);
>> -    rc = pcie_aer_init(d, XIO3130_AER_OFFSET);
>> +    rc = pcie_aer_init(d, PCI_ERR_VER,
>> +                       XIO3130_AER_OFFSET, PCI_ERR_SIZEOF);
>>       if (rc < 0) {
>>           goto err;
>>       }
>> diff --git a/hw/pci-bridge/xio3130_upstream.c b/hw/pci-bridge/xio3130_upstream.c
>> index eada582..52b130f 100644
>> --- a/hw/pci-bridge/xio3130_upstream.c
>> +++ b/hw/pci-bridge/xio3130_upstream.c
>> @@ -81,7 +81,8 @@ static int xio3130_upstream_initfn(PCIDevice *d)
>>       }
>>       pcie_cap_flr_init(d);
>>       pcie_cap_deverr_init(d);
>> -    rc = pcie_aer_init(d, XIO3130_AER_OFFSET);
>> +    rc = pcie_aer_init(d, PCI_ERR_VER,
>> +                       XIO3130_AER_OFFSET, PCI_ERR_SIZEOF);
>>       if (rc < 0) {
>>           goto err;
>>       }
>> diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c
>> index ece1487..a76a943 100644
>> --- a/hw/pci/pcie_aer.c
>> +++ b/hw/pci/pcie_aer.c
>> @@ -94,12 +94,13 @@ static void aer_log_clear_all_err(PCIEAERLog *aer_log)
>>       aer_log->log_num = 0;
>>   }
>>   
>> -int pcie_aer_init(PCIDevice *dev, uint16_t offset)
>> +int pcie_aer_init(PCIDevice *dev, uint8_t cap_ver,
>> +                  uint16_t offset, uint16_t size)
>>   {
>>       PCIExpressDevice *exp;
>>   
>> -    pcie_add_capability(dev, PCI_EXT_CAP_ID_ERR, PCI_ERR_VER,
>> -                        offset, PCI_ERR_SIZEOF);
>> +    pcie_add_capability(dev, PCI_EXT_CAP_ID_ERR,
>> +                        cap_ver, offset, size);
>>       exp = &dev->exp;
>>       exp->aer_cap = offset;
>>   
>> diff --git a/include/hw/pci/pcie_aer.h b/include/hw/pci/pcie_aer.h
>> index bcac80a..afa074e 100644
>> --- a/include/hw/pci/pcie_aer.h
>> +++ b/include/hw/pci/pcie_aer.h
>> @@ -87,7 +87,9 @@ struct PCIEAERErr {
>>   
>>   extern const VMStateDescription vmstate_pcie_aer_log;
>>   
>> -int pcie_aer_init(PCIDevice *dev, uint16_t offset);
>> +int pcie_aer_init(PCIDevice *dev, uint8_t cap_ver,
>> +                  uint16_t offset, uint16_t size);
>> +
>>   void pcie_aer_exit(PCIDevice *dev);
>>   void pcie_aer_write_config(PCIDevice *dev,
>>                              uint32_t addr, uint32_t val, int len);
>
>
> .
>
diff mbox

Patch

diff --git a/hw/pci-bridge/ioh3420.c b/hw/pci-bridge/ioh3420.c
index cce2fdd..354574f 100644
--- a/hw/pci-bridge/ioh3420.c
+++ b/hw/pci-bridge/ioh3420.c
@@ -129,7 +129,8 @@  static int ioh3420_initfn(PCIDevice *d)
         goto err_pcie_cap;
     }
     pcie_cap_root_init(d);
-    rc = pcie_aer_init(d, IOH_EP_AER_OFFSET);
+    rc = pcie_aer_init(d, PCI_ERR_VER,
+                       IOH_EP_AER_OFFSET, PCI_ERR_SIZEOF);
     if (rc < 0) {
         goto err;
     }
diff --git a/hw/pci-bridge/xio3130_downstream.c b/hw/pci-bridge/xio3130_downstream.c
index b3a6479..407f75f 100644
--- a/hw/pci-bridge/xio3130_downstream.c
+++ b/hw/pci-bridge/xio3130_downstream.c
@@ -92,7 +92,8 @@  static int xio3130_downstream_initfn(PCIDevice *d)
         goto err_pcie_cap;
     }
     pcie_cap_arifwd_init(d);
-    rc = pcie_aer_init(d, XIO3130_AER_OFFSET);
+    rc = pcie_aer_init(d, PCI_ERR_VER,
+                       XIO3130_AER_OFFSET, PCI_ERR_SIZEOF);
     if (rc < 0) {
         goto err;
     }
diff --git a/hw/pci-bridge/xio3130_upstream.c b/hw/pci-bridge/xio3130_upstream.c
index eada582..52b130f 100644
--- a/hw/pci-bridge/xio3130_upstream.c
+++ b/hw/pci-bridge/xio3130_upstream.c
@@ -81,7 +81,8 @@  static int xio3130_upstream_initfn(PCIDevice *d)
     }
     pcie_cap_flr_init(d);
     pcie_cap_deverr_init(d);
-    rc = pcie_aer_init(d, XIO3130_AER_OFFSET);
+    rc = pcie_aer_init(d, PCI_ERR_VER,
+                       XIO3130_AER_OFFSET, PCI_ERR_SIZEOF);
     if (rc < 0) {
         goto err;
     }
diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c
index ece1487..a76a943 100644
--- a/hw/pci/pcie_aer.c
+++ b/hw/pci/pcie_aer.c
@@ -94,12 +94,13 @@  static void aer_log_clear_all_err(PCIEAERLog *aer_log)
     aer_log->log_num = 0;
 }
 
-int pcie_aer_init(PCIDevice *dev, uint16_t offset)
+int pcie_aer_init(PCIDevice *dev, uint8_t cap_ver,
+                  uint16_t offset, uint16_t size)
 {
     PCIExpressDevice *exp;
 
-    pcie_add_capability(dev, PCI_EXT_CAP_ID_ERR, PCI_ERR_VER,
-                        offset, PCI_ERR_SIZEOF);
+    pcie_add_capability(dev, PCI_EXT_CAP_ID_ERR,
+                        cap_ver, offset, size);
     exp = &dev->exp;
     exp->aer_cap = offset;
 
diff --git a/include/hw/pci/pcie_aer.h b/include/hw/pci/pcie_aer.h
index bcac80a..afa074e 100644
--- a/include/hw/pci/pcie_aer.h
+++ b/include/hw/pci/pcie_aer.h
@@ -87,7 +87,9 @@  struct PCIEAERErr {
 
 extern const VMStateDescription vmstate_pcie_aer_log;
 
-int pcie_aer_init(PCIDevice *dev, uint16_t offset);
+int pcie_aer_init(PCIDevice *dev, uint8_t cap_ver,
+                  uint16_t offset, uint16_t size);
+
 void pcie_aer_exit(PCIDevice *dev);
 void pcie_aer_write_config(PCIDevice *dev,
                            uint32_t addr, uint32_t val, int len);