[PATCH] pci-assign: Do not expose MSI/MSI-X if the kernel does not support it

[PATCH] pci-assign: Do not expose MSI/MSI-X if the kernel does not support it

[Jan Kiszka]

Add checks for KVM_CAP_ASSIGN_DEV_IRQ (MSI) and KVM_CAP_DEVICE_MSIX, do
not set up MSI/MSI-X if the required kernel features are missing.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/device-assignment.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/device-assignment.c b/hw/device-assignment.c
index 4cc7b1a..7e965cb 100644
--- a/hw/device-assignment.c
+++ b/hw/device-assignment.c
@@ -1201,7 +1201,8 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev)
 
     /* Expose MSI capability
      * MSI capability is the 1st capability in capability config */
-    if ((pos = pci_find_cap_offset(pci_dev, PCI_CAP_ID_MSI, 0))) {
+    pos = pci_find_cap_offset(pci_dev, PCI_CAP_ID_MSI, 0);
+    if (pos != 0 && kvm_check_extension(kvm_state, KVM_CAP_ASSIGN_DEV_IRQ)) {
         dev->cap.available |= ASSIGNED_DEVICE_CAP_MSI;
         /* Only 32-bit/no-mask currently supported */
         if ((ret = pci_add_capability(pci_dev, PCI_CAP_ID_MSI, pos, 10)) < 0) {
@@ -1222,7 +1223,8 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev)
         pci_set_word(pci_dev->wmask + pos + PCI_MSI_DATA_32, 0xffff);
     }
     /* Expose MSI-X capability */
-    if ((pos = pci_find_cap_offset(pci_dev, PCI_CAP_ID_MSIX, 0))) {
+    pos = pci_find_cap_offset(pci_dev, PCI_CAP_ID_MSIX, 0);
+    if (pos != 0 && kvm_check_extension(kvm_state, KVM_CAP_DEVICE_MSIX)) {
         int bar_nr;
         uint32_t msix_table_entry;
 
-- 
1.7.3.4

Re: [PATCH] pci-assign: Do not expose MSI/MSI-X if the kernel does not support it

[Alex Williamson]

On Tue, 2011-08-02 at 14:27 +0200, Jan Kiszka wrote:
> Add checks for KVM_CAP_ASSIGN_DEV_IRQ (MSI) and KVM_CAP_DEVICE_MSIX, do
> not set up MSI/MSI-X if the required kernel features are missing.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  hw/device-assignment.c |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/device-assignment.c b/hw/device-assignment.c
> index 4cc7b1a..7e965cb 100644
> --- a/hw/device-assignment.c
> +++ b/hw/device-assignment.c
> @@ -1201,7 +1201,8 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev)
>  
>      /* Expose MSI capability
>       * MSI capability is the 1st capability in capability config */
> -    if ((pos = pci_find_cap_offset(pci_dev, PCI_CAP_ID_MSI, 0))) {
> +    pos = pci_find_cap_offset(pci_dev, PCI_CAP_ID_MSI, 0);
> +    if (pos != 0 && kvm_check_extension(kvm_state, KVM_CAP_ASSIGN_DEV_IRQ)) {

Is it even useful to have a device assigned w/o KVM_CAP_ASSIGN_DEV_IRQ?
Thanks,

Alex

>          dev->cap.available |= ASSIGNED_DEVICE_CAP_MSI;
>          /* Only 32-bit/no-mask currently supported */
>          if ((ret = pci_add_capability(pci_dev, PCI_CAP_ID_MSI, pos, 10)) < 0) {
> @@ -1222,7 +1223,8 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev)
>          pci_set_word(pci_dev->wmask + pos + PCI_MSI_DATA_32, 0xffff);
>      }
>      /* Expose MSI-X capability */
> -    if ((pos = pci_find_cap_offset(pci_dev, PCI_CAP_ID_MSIX, 0))) {
> +    pos = pci_find_cap_offset(pci_dev, PCI_CAP_ID_MSIX, 0);
> +    if (pos != 0 && kvm_check_extension(kvm_state, KVM_CAP_DEVICE_MSIX)) {
>          int bar_nr;
>          uint32_t msix_table_entry;
>  

Re: [PATCH] pci-assign: Do not expose MSI/MSI-X if the kernel does not support it

[Jan Kiszka]

On 2011-08-02 19:02, Alex Williamson wrote:
> On Tue, 2011-08-02 at 14:27 +0200, Jan Kiszka wrote:
>> Add checks for KVM_CAP_ASSIGN_DEV_IRQ (MSI) and KVM_CAP_DEVICE_MSIX, do
>> not set up MSI/MSI-X if the required kernel features are missing.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>>  hw/device-assignment.c |    6 ++++--
>>  1 files changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/device-assignment.c b/hw/device-assignment.c
>> index 4cc7b1a..7e965cb 100644
>> --- a/hw/device-assignment.c
>> +++ b/hw/device-assignment.c
>> @@ -1201,7 +1201,8 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev)
>>  
>>      /* Expose MSI capability
>>       * MSI capability is the 1st capability in capability config */
>> -    if ((pos = pci_find_cap_offset(pci_dev, PCI_CAP_ID_MSI, 0))) {
>> +    pos = pci_find_cap_offset(pci_dev, PCI_CAP_ID_MSI, 0);
>> +    if (pos != 0 && kvm_check_extension(kvm_state, KVM_CAP_ASSIGN_DEV_IRQ)) {
> 
> Is it even useful to have a device assigned w/o KVM_CAP_ASSIGN_DEV_IRQ?

If that feature is lacking, we fall back to the old KVM_ASSIGN_IRQ
interface. I guess that used to work, but I bet no one tested it
recently. However, the code is there, also in the kvm core.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [PATCH] pci-assign: Do not expose MSI/MSI-X if the kernel does not support it

[Alex Williamson]

On Tue, 2011-08-02 at 19:32 +0200, Jan Kiszka wrote:
> On 2011-08-02 19:02, Alex Williamson wrote:
> > On Tue, 2011-08-02 at 14:27 +0200, Jan Kiszka wrote:
> >> Add checks for KVM_CAP_ASSIGN_DEV_IRQ (MSI) and KVM_CAP_DEVICE_MSIX, do
> >> not set up MSI/MSI-X if the required kernel features are missing.
> >>
> >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> >> ---
> >>  hw/device-assignment.c |    6 ++++--
> >>  1 files changed, 4 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/hw/device-assignment.c b/hw/device-assignment.c
> >> index 4cc7b1a..7e965cb 100644
> >> --- a/hw/device-assignment.c
> >> +++ b/hw/device-assignment.c
> >> @@ -1201,7 +1201,8 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev)
> >>  
> >>      /* Expose MSI capability
> >>       * MSI capability is the 1st capability in capability config */
> >> -    if ((pos = pci_find_cap_offset(pci_dev, PCI_CAP_ID_MSI, 0))) {
> >> +    pos = pci_find_cap_offset(pci_dev, PCI_CAP_ID_MSI, 0);
> >> +    if (pos != 0 && kvm_check_extension(kvm_state, KVM_CAP_ASSIGN_DEV_IRQ)) {
> > 
> > Is it even useful to have a device assigned w/o KVM_CAP_ASSIGN_DEV_IRQ?
> 
> If that feature is lacking, we fall back to the old KVM_ASSIGN_IRQ
> interface. I guess that used to work, but I bet no one tested it
> recently. However, the code is there, also in the kvm core.

Ah right.  Not sure how much I trust that, but seems obviously correct
to not expose capabilities we can't support.

Acked-by: Alex Williamson <alex.williamson@redhat.com>

Re: [PATCH] pci-assign: Do not expose MSI/MSI-X if the kernel does not support it

[Marcelo Tosatti]

On Tue, Aug 02, 2011 at 02:27:26PM +0200, Jan Kiszka wrote:
> Add checks for KVM_CAP_ASSIGN_DEV_IRQ (MSI) and KVM_CAP_DEVICE_MSIX, do
> not set up MSI/MSI-X if the required kernel features are missing.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  hw/device-assignment.c |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)

Applied, thanks.