[Qemu-devel] [PATCH] hw/gen_pcie_root_port: properly set

[Qemu-devel] [PATCH] hw/gen_pcie_root_port: properly set

[Marcel Apfelbaum]

IO_LIMIT and IO_BASE registers should not be writable if
gen_pcie_root_port's io-reserve property is set to 0.
The COMMAND register should have the IO flag read only.


Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
---
 hw/pci-bridge/gen_pcie_root_port.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/hw/pci-bridge/gen_pcie_root_port.c b/hw/pci-bridge/gen_pcie_root_port.c
index ed03ffc764..ad4e6aa7ff 100644
--- a/hw/pci-bridge/gen_pcie_root_port.c
+++ b/hw/pci-bridge/gen_pcie_root_port.c
@@ -85,6 +85,13 @@ static void gen_rp_realize(DeviceState *dev, Error **errp)
         rpc->parent_class.exit(d);
         return;
     }
+
+    if (!grp->io_reserve) {
+        pci_word_test_and_clear_mask(d->wmask + PCI_COMMAND,
+                                     PCI_COMMAND_IO);
+        d->wmask[PCI_IO_BASE] = 0;
+        d->wmask[PCI_IO_LIMIT] = 0;
+    }
 }
 
 static const VMStateDescription vmstate_rp_dev = {
-- 
2.13.5

Re: [Qemu-devel] [PATCH] hw/gen_pcie_root_port: properly set

[Laszlo Ersek]

On 10/02/17 12:31, Marcel Apfelbaum wrote:
> IO_LIMIT and IO_BASE registers should not be writable if
> gen_pcie_root_port's io-reserve property is set to 0.
> The COMMAND register should have the IO flag read only.
> 
> 
> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
> ---
>  hw/pci-bridge/gen_pcie_root_port.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/hw/pci-bridge/gen_pcie_root_port.c b/hw/pci-bridge/gen_pcie_root_port.c
> index ed03ffc764..ad4e6aa7ff 100644
> --- a/hw/pci-bridge/gen_pcie_root_port.c
> +++ b/hw/pci-bridge/gen_pcie_root_port.c
> @@ -85,6 +85,13 @@ static void gen_rp_realize(DeviceState *dev, Error **errp)
>          rpc->parent_class.exit(d);
>          return;
>      }
> +
> +    if (!grp->io_reserve) {
> +        pci_word_test_and_clear_mask(d->wmask + PCI_COMMAND,
> +                                     PCI_COMMAND_IO);
> +        d->wmask[PCI_IO_BASE] = 0;
> +        d->wmask[PCI_IO_LIMIT] = 0;
> +    }
>  }
>  
>  static const VMStateDescription vmstate_rp_dev = {
> 

The patch looks good to me (which doesn't mean much, honestly :) ), but
I think the subject line is truncated. You left off what exactly should
be set properly. (IO base, IO limit, and command registers, presumably.)

Thanks!
Laszlo

Re: [Qemu-devel] [PATCH] hw/gen_pcie_root_port: properly set

[Marcel Apfelbaum]

On 03/10/2017 0:49, Laszlo Ersek wrote:
> On 10/02/17 12:31, Marcel Apfelbaum wrote:
>> IO_LIMIT and IO_BASE registers should not be writable if
>> gen_pcie_root_port's io-reserve property is set to 0.
>> The COMMAND register should have the IO flag read only.
>>
>>
>> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
>> ---
>>   hw/pci-bridge/gen_pcie_root_port.c | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/hw/pci-bridge/gen_pcie_root_port.c b/hw/pci-bridge/gen_pcie_root_port.c
>> index ed03ffc764..ad4e6aa7ff 100644
>> --- a/hw/pci-bridge/gen_pcie_root_port.c
>> +++ b/hw/pci-bridge/gen_pcie_root_port.c
>> @@ -85,6 +85,13 @@ static void gen_rp_realize(DeviceState *dev, Error **errp)
>>           rpc->parent_class.exit(d);
>>           return;
>>       }
>> +
>> +    if (!grp->io_reserve) {
>> +        pci_word_test_and_clear_mask(d->wmask + PCI_COMMAND,
>> +                                     PCI_COMMAND_IO);
>> +        d->wmask[PCI_IO_BASE] = 0;
>> +        d->wmask[PCI_IO_LIMIT] = 0;
>> +    }
>>   }
>>   
>>   static const VMStateDescription vmstate_rp_dev = {
>>
> 
> The patch looks good to me (which doesn't mean much, honestly :) ), but
> I think the subject line is truncated. You left off what exactly should
> be set properly. (IO base, IO limit, and command registers, presumably.)
> 

Strange, thanks for pointing it out.
I'll send a V2.

Thanks,
Marcel

> Thanks!
> Laszlo
>