* [Qemu-devel] [PATCH] hw/pci-bridge/pcie_pci_bridge: properly handle MSI unavailability case
@ 2017-09-20 21:21 Aleksandr Bezzubikov
2017-09-21 10:16 ` Marcel Apfelbaum
0 siblings, 1 reply; 4+ messages in thread
From: Aleksandr Bezzubikov @ 2017-09-20 21:21 UTC (permalink / raw)
To: qemu-devel; +Cc: mst, marcel, Aleksandr Bezzubikov
Signed-off-by: Aleksandr Bezzubikov <zuban32s@gmail.com>
---
hw/pci-bridge/pcie_pci_bridge.c | 24 ++++++++++++++++++------
1 file changed, 18 insertions(+), 6 deletions(-)
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);
+ } 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(),
};
--
2.7.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/pci-bridge/pcie_pci_bridge: properly handle MSI unavailability case
2017-09-20 21:21 [Qemu-devel] [PATCH] hw/pci-bridge/pcie_pci_bridge: properly handle MSI unavailability case Aleksandr Bezzubikov
@ 2017-09-21 10:16 ` Marcel Apfelbaum
2017-09-21 18:12 ` Aleksandr Bezzubikov
0 siblings, 1 reply; 4+ messages in thread
From: Marcel Apfelbaum @ 2017-09-21 10:16 UTC (permalink / raw)
To: Aleksandr Bezzubikov, qemu-devel; +Cc: mst
Hi Aleksandr,
On 21/09/2017 0:21, Aleksandr Bezzubikov wrote:
> Signed-off-by: Aleksandr Bezzubikov <zuban32s@gmail.com>
> ---
> 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.
> 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?
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(),
> };
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/pci-bridge/pcie_pci_bridge: properly handle MSI unavailability case
2017-09-21 10:16 ` Marcel Apfelbaum
@ 2017-09-21 18:12 ` Aleksandr Bezzubikov
2017-09-22 6:42 ` Marcel Apfelbaum
0 siblings, 1 reply; 4+ messages in thread
From: Aleksandr Bezzubikov @ 2017-09-21 18:12 UTC (permalink / raw)
To: Marcel Apfelbaum; +Cc: QEMU Developers, Michael S. Tsirkin
2017-09-21 13:16 GMT+03:00 Marcel Apfelbaum <marcel@redhat.com>:
> Hi Aleksandr,
>
> On 21/09/2017 0:21, Aleksandr Bezzubikov wrote:
>>
>> Signed-off-by: Aleksandr Bezzubikov <zuban32s@gmail.com>
>> ---
>> 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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/pci-bridge/pcie_pci_bridge: properly handle MSI unavailability case
2017-09-21 18:12 ` Aleksandr Bezzubikov
@ 2017-09-22 6:42 ` Marcel Apfelbaum
0 siblings, 0 replies; 4+ messages in thread
From: Marcel Apfelbaum @ 2017-09-22 6:42 UTC (permalink / raw)
To: Aleksandr Bezzubikov; +Cc: QEMU Developers, Michael S. Tsirkin
On 21/09/2017 21:12, Aleksandr Bezzubikov wrote:
> 2017-09-21 13:16 GMT+03:00 Marcel Apfelbaum <marcel@redhat.com>:
>> Hi Aleksandr,
>>
>> On 21/09/2017 0:21, Aleksandr Bezzubikov wrote:
>>>
>>> Signed-off-by: Aleksandr Bezzubikov <zuban32s@gmail.com>
>>> ---
>>> 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?
>
A new version would be preferred.
>>
>>
>>> 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.
>
With the above comment:
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
Thanks,
Marcel
>>
>> 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(),
>>> };
>>>
>>
>>
>
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-09-22 6:42 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-20 21:21 [Qemu-devel] [PATCH] hw/pci-bridge/pcie_pci_bridge: properly handle MSI unavailability case Aleksandr Bezzubikov
2017-09-21 10:16 ` Marcel Apfelbaum
2017-09-21 18:12 ` Aleksandr Bezzubikov
2017-09-22 6:42 ` Marcel Apfelbaum
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.