* [PATCH 1/1] amd_iommu: fix wrong MMIO operations
@ 2021-04-26 8:21 Roman Kapl
2021-04-26 23:24 ` Michael S. Tsirkin
0 siblings, 1 reply; 3+ messages in thread
From: Roman Kapl @ 2021-04-26 8:21 UTC (permalink / raw)
Cc: Eduardo Habkost, Michael S. Tsirkin, Roman Kapl,
Richard Henderson, qemu-devel, Paolo Bonzini
Address was swapped with value when writing MMIO registers, so the user
saw garbage in lot of cases. The interrupt status was not correctly set.
Signed-off-by: Roman Kapl <rka@sysgo.com>
---
hw/i386/amd_iommu.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index 74a93a5d93..bb5ce8c04d 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -141,13 +141,13 @@ static bool amdvi_test_mask(AMDVIState *s, hwaddr addr, uint64_t val)
/* OR a 64-bit register with a 64-bit value storing result in the register */
static void amdvi_assign_orq(AMDVIState *s, hwaddr addr, uint64_t val)
{
- amdvi_writeq_raw(s, addr, amdvi_readq(s, addr) | val);
+ amdvi_writeq_raw(s, amdvi_readq(s, addr) | val, addr);
}
/* AND a 64-bit register with a 64-bit value storing result in the register */
static void amdvi_assign_andq(AMDVIState *s, hwaddr addr, uint64_t val)
{
- amdvi_writeq_raw(s, addr, amdvi_readq(s, addr) & val);
+ amdvi_writeq_raw(s, amdvi_readq(s, addr) & val, addr);
}
static void amdvi_generate_msi_interrupt(AMDVIState *s)
@@ -382,7 +382,7 @@ static void amdvi_completion_wait(AMDVIState *s, uint64_t *cmd)
}
/* set completion interrupt */
if (extract64(cmd[0], 1, 1)) {
- amdvi_test_mask(s, AMDVI_MMIO_STATUS, AMDVI_MMIO_STATUS_COMP_INT);
+ amdvi_assign_orq(s, AMDVI_MMIO_STATUS, AMDVI_MMIO_STATUS_COMP_INT);
/* generate interrupt */
amdvi_generate_msi_interrupt(s);
}
--
2.20.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH 1/1] amd_iommu: fix wrong MMIO operations
2021-04-26 8:21 [PATCH 1/1] amd_iommu: fix wrong MMIO operations Roman Kapl
@ 2021-04-26 23:24 ` Michael S. Tsirkin
2021-04-27 10:52 ` Roman Kapl
0 siblings, 1 reply; 3+ messages in thread
From: Michael S. Tsirkin @ 2021-04-26 23:24 UTC (permalink / raw)
To: Roman Kapl; +Cc: Paolo Bonzini, Richard Henderson, Eduardo Habkost, qemu-devel
On Mon, Apr 26, 2021 at 10:21:54AM +0200, Roman Kapl wrote:
> Address was swapped with value when writing MMIO registers, so the user
> saw garbage in lot of cases. The interrupt status was not correctly set.
>
> Signed-off-by: Roman Kapl <rka@sysgo.com>
Ouch. This API is just inconsistent, everyone else
uses addr, value in this order. How about fixing the
function signature instead?
> ---
> hw/i386/amd_iommu.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
> index 74a93a5d93..bb5ce8c04d 100644
> --- a/hw/i386/amd_iommu.c
> +++ b/hw/i386/amd_iommu.c
> @@ -141,13 +141,13 @@ static bool amdvi_test_mask(AMDVIState *s, hwaddr addr, uint64_t val)
> /* OR a 64-bit register with a 64-bit value storing result in the register */
> static void amdvi_assign_orq(AMDVIState *s, hwaddr addr, uint64_t val)
> {
> - amdvi_writeq_raw(s, addr, amdvi_readq(s, addr) | val);
> + amdvi_writeq_raw(s, amdvi_readq(s, addr) | val, addr);
> }
>
> /* AND a 64-bit register with a 64-bit value storing result in the register */
> static void amdvi_assign_andq(AMDVIState *s, hwaddr addr, uint64_t val)
> {
> - amdvi_writeq_raw(s, addr, amdvi_readq(s, addr) & val);
> + amdvi_writeq_raw(s, amdvi_readq(s, addr) & val, addr);
> }
>
> static void amdvi_generate_msi_interrupt(AMDVIState *s)
> @@ -382,7 +382,7 @@ static void amdvi_completion_wait(AMDVIState *s, uint64_t *cmd)
> }
> /* set completion interrupt */
> if (extract64(cmd[0], 1, 1)) {
> - amdvi_test_mask(s, AMDVI_MMIO_STATUS, AMDVI_MMIO_STATUS_COMP_INT);
> + amdvi_assign_orq(s, AMDVI_MMIO_STATUS, AMDVI_MMIO_STATUS_COMP_INT);
> /* generate interrupt */
> amdvi_generate_msi_interrupt(s);
> }
> --
> 2.20.1
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH 1/1] amd_iommu: fix wrong MMIO operations
2021-04-26 23:24 ` Michael S. Tsirkin
@ 2021-04-27 10:52 ` Roman Kapl
0 siblings, 0 replies; 3+ messages in thread
From: Roman Kapl @ 2021-04-27 10:52 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Paolo Bonzini, Richard Henderson, Eduardo Habkost, qemu-devel
On 4/27/21 1:24 AM, Michael S. Tsirkin wrote:
> On Mon, Apr 26, 2021 at 10:21:54AM +0200, Roman Kapl wrote:
>> Address was swapped with value when writing MMIO registers, so the user
>> saw garbage in lot of cases. The interrupt status was not correctly set.
>>
>> Signed-off-by: Roman Kapl <rka@sysgo.com>
> Ouch. This API is just inconsistent, everyone else
> uses addr, value in this order. How about fixing the
> function signature instead?
True, even the code in the same module. I will send v2.
>
>> ---
>> hw/i386/amd_iommu.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
>> index 74a93a5d93..bb5ce8c04d 100644
>> --- a/hw/i386/amd_iommu.c
>> +++ b/hw/i386/amd_iommu.c
>> @@ -141,13 +141,13 @@ static bool amdvi_test_mask(AMDVIState *s, hwaddr addr, uint64_t val)
>> /* OR a 64-bit register with a 64-bit value storing result in the register */
>> static void amdvi_assign_orq(AMDVIState *s, hwaddr addr, uint64_t val)
>> {
>> - amdvi_writeq_raw(s, addr, amdvi_readq(s, addr) | val);
>> + amdvi_writeq_raw(s, amdvi_readq(s, addr) | val, addr);
>> }
>>
>> /* AND a 64-bit register with a 64-bit value storing result in the register */
>> static void amdvi_assign_andq(AMDVIState *s, hwaddr addr, uint64_t val)
>> {
>> - amdvi_writeq_raw(s, addr, amdvi_readq(s, addr) & val);
>> + amdvi_writeq_raw(s, amdvi_readq(s, addr) & val, addr);
>> }
>>
>> static void amdvi_generate_msi_interrupt(AMDVIState *s)
>> @@ -382,7 +382,7 @@ static void amdvi_completion_wait(AMDVIState *s, uint64_t *cmd)
>> }
>> /* set completion interrupt */
>> if (extract64(cmd[0], 1, 1)) {
>> - amdvi_test_mask(s, AMDVI_MMIO_STATUS, AMDVI_MMIO_STATUS_COMP_INT);
>> + amdvi_assign_orq(s, AMDVI_MMIO_STATUS, AMDVI_MMIO_STATUS_COMP_INT);
>> /* generate interrupt */
>> amdvi_generate_msi_interrupt(s);
>> }
>> --
>> 2.20.1
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-04-27 10:56 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-26 8:21 [PATCH 1/1] amd_iommu: fix wrong MMIO operations Roman Kapl
2021-04-26 23:24 ` Michael S. Tsirkin
2021-04-27 10:52 ` Roman Kapl
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.