[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] hw/pci-bridge/pcie_pci_bridge: properly handle
From: |
Aleksandr Bezzubikov |
Subject: |
Re: [Qemu-devel] [PATCH] hw/pci-bridge/pcie_pci_bridge: properly handle MSI unavailability case |
Date: |
Thu, 21 Sep 2017 21:12:21 +0300 |
2017-09-21 13:16 GMT+03:00 Marcel Apfelbaum <address@hidden>:
> Hi Aleksandr,
>
> On 21/09/2017 0:21, Aleksandr Bezzubikov wrote:
>>
>> Signed-off-by: Aleksandr Bezzubikov <address@hidden>
>> ---
>> hw/pci-bridge/pcie_pci_bridge.c | 24 ++++++++++++++++++------
>> 1 file changed, 18 insertions(+), 6 deletions(-)
>>
>
> Please add Eduardo's Reported-by tag and the
> actual failure in the commit message.
> You might even explain in the commit message that you
> moved msi prop to ON_OFF_AUTO_AUTO.
Should I send a new one, or resend, or just reply to this post?
>
>
>> diff --git a/hw/pci-bridge/pcie_pci_bridge.c
>> b/hw/pci-bridge/pcie_pci_bridge.c
>> index 9aa5cc3..da562fe 100644
>> --- a/hw/pci-bridge/pcie_pci_bridge.c
>> +++ b/hw/pci-bridge/pcie_pci_bridge.c
>> @@ -65,10 +65,18 @@ static void pcie_pci_bridge_realize(PCIDevice *d,
>> Error **errp)
>> goto aer_error;
>> }
>> + Error *local_err = NULL;
>> if (pcie_br->msi != ON_OFF_AUTO_OFF) {
>> - rc = msi_init(d, 0, 1, true, true, errp);
>> + rc = msi_init(d, 0, 1, true, true, &local_err);
>> if (rc < 0) {
>> - goto msi_error;
>> + assert(rc == -ENOTSUP);
>> + if (pcie_br->msi != ON_OFF_AUTO_ON) {
>> + error_free(local_err);
>
>
> In that case it will fallback to legacy INTx, right?
Exactly. Maybe I should add this to the commit message.
>
> Thanks,
> Marcel
>
>
>> + } else {
>> + /* failed to satisfy user's explicit request for MSI */
>> + error_propagate(errp, local_err);
>> + goto msi_error;
>> + }
>> }
>> }
>> pci_register_bar(d, 0, PCI_BASE_ADDRESS_SPACE_MEMORY |
>> @@ -81,7 +89,7 @@ aer_error:
>> pm_error:
>> pcie_cap_exit(d);
>> cap_error:
>> - shpc_free(d);
>> + shpc_cleanup(d, &pcie_br->shpc_bar);
>> error:
>> pci_bridge_exitfn(d);
>> }
>> @@ -98,7 +106,9 @@ static void pcie_pci_bridge_reset(DeviceState *qdev)
>> {
>> PCIDevice *d = PCI_DEVICE(qdev);
>> pci_bridge_reset(qdev);
>> - msi_reset(d);
>> + if (msi_present(d)) {
>> + msi_reset(d);
>> + }
>> shpc_reset(d);
>> }
>> @@ -106,12 +116,14 @@ static void pcie_pci_bridge_write_config(PCIDevice
>> *d,
>> uint32_t address, uint32_t val, int len)
>> {
>> pci_bridge_write_config(d, address, val, len);
>> - msi_write_config(d, address, val, len);
>> + if (msi_present(d)) {
>> + msi_write_config(d, address, val, len);
>> + }
>> shpc_cap_write_config(d, address, val, len);
>> }
>> static Property pcie_pci_bridge_dev_properties[] = {
>> - DEFINE_PROP_ON_OFF_AUTO("msi", PCIEPCIBridge, msi,
>> ON_OFF_AUTO_ON),
>> + DEFINE_PROP_ON_OFF_AUTO("msi", PCIEPCIBridge, msi,
>> ON_OFF_AUTO_AUTO),
>> DEFINE_PROP_END_OF_LIST(),
>> };
>>
>
>
--
Aleksandr Bezzubikov