hw/ppc/spapr_iommu: Fix the check for invalid upper bits in liobn
PATCH 1/1 hw/ppc/spapr_iommu: Fix the check for invalid upper bits in liobn
The check "liobn & 0xFFFFFFFF00000000ULL" in spapr_tce_find_by_liobn() is completely useless since liobn is only declared as an uint32_t parameter. Fix this by using target_ulong instead (this is what most of the callers of this function are using, too).
And while we're at it, change the error message printing into a proper qemu_log_mask(LOG_GUEST_ERROR, ...) call so that it is also possible to enable this warning without recompiling the binary.
--- hw/ppc/spapr_iommu.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
hw/ppc/spapr_iommu.c +4 -3
@@ -41,18 +41,19 @@ enum sPAPRTCEAccess { static QLIST_HEAD(spapr_tce_tables, sPAPRTCETable) spapr_tce_tables; -static sPAPRTCETable *spapr_tce_find_by_liobn (uint32_t liobn) +static sPAPRTCETable *spapr_tce_find_by_liobn (target_ulong liobn) {sPAPRTCETable *tcet;if (liobn &0xFFFFFFFF00000000ULL ) { -hcall_dprintf ("Request for out-of-bounds LIOBN 0x" TARGET_FMT_lx"\n" , +qemu_log_mask (LOG_GUEST_ERROR , +"Request for out-of-bounds LIOBN 0x" TARGET_FMT_lx"\n" , liobn);return NULL ; } QLIST_FOREACH(tcet, &spapr_tce_tables, list) { -if (tcet->liobn == liobn) { +if (tcet->liobn == (uint32_t )liobn) {return tcet; } }
Signature · 1 line
1.8.3.1
Quoted 22 lines · Thomas Huth
The check "liobn &
0xFFFFFFFF00000000ULL" inspapr_tce_find_by_liobn()is completely useless since liobn is only declared as anuint32_tparameter. Fix this by usingtarget_ulonginstead (this is what most of the callers of this function are using, too).
And while we're at it, change the error message printing into a properqemu_log_mask(LOG_GUEST_ERROR, ...)call so that it is also possible to enable this warning without recompiling the binary.Signed-off-by Thomas Huth <[email protected]>--- hw/ppc/spapr_iommu.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)hw/ppc/spapr_iommu.c +0
-0@@ -41,18 +41,19 @@ enum sPAPRTCEAccess { static QLIST_HEAD(spapr_tce_tables, sPAPRTCETable) spapr_tce_tables;
hw/ppc/spapr_iommu.c enum sPAPRTCEAccess { · line 51
Quoted 10 lines
hw/ppc/spapr_iommu.c +3
-2-static sPAPRTCETable *spapr_tce_find_by_liobn (uint32_t liobn) +static sPAPRTCETable *spapr_tce_find_by_liobn (target_ulong liobn) {sPAPRTCETable *tcet;if (liobn &0xFFFFFFFF00000000ULL ) { -hcall_dprintf ("Request for out-of-bounds LIOBN 0x" TARGET_FMT_lx"\n" , +qemu_log_mask (LOG_GUEST_ERROR , +"Request for out-of-bounds LIOBN 0x" TARGET_FMT_lx"\n" , liobn);
I'd actually prefer to see this left for the time being. The dprintf stuff may be ugly, but it's used throughout this stuff, whereas the qemu logging is not. Plus I've found the qemu logging stuff (as opposed to the tracepoint stuff) to be a pain to actually use.
So I'd prefer that you just fix the mask bug, and we look at a different patch to change the logging of the spapr stuff en masse.
Quoted 9 lines
hw/ppc/spapr_iommu.c +1
-1return NULL ; } QLIST_FOREACH(tcet, &spapr_tce_tables, list) { -if (tcet->liobn == liobn) { +if (tcet->liobn == (uint32_t )liobn) {return tcet; } }
Signature · 4 lines
David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
Am Tue, 21 Apr 2015 10:24:48 +1000
schrieb David Gibson <[email protected]>:
Quoted 28 lines · Thomas Huth
Quoted 27 lines
The check "liobn &
0xFFFFFFFF00000000ULL" inspapr_tce_find_by_liobn()is completely useless since liobn is only declared as anuint32_tparameter. Fix this by usingtarget_ulonginstead (this is what most of the callers of this function are using, too).
And while we're at it, change the error message printing into a properqemu_log_mask(LOG_GUEST_ERROR, ...)call so that it is also possible to enable this warning without recompiling the binary.Signed-off-by Thomas Huth <[email protected]>--- hw/ppc/spapr_iommu.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)hw/ppc/spapr_iommu.c +1
-1@@ -41,18 +41,19 @@ enum sPAPRTCEAccess { static QLIST_HEAD(spapr_tce_tables, sPAPRTCETable) spapr_tce_tables; -static sPAPRTCETable *spapr_tce_find_by_liobn (uint32_t liobn) +static sPAPRTCETable *spapr_tce_find_by_liobn (target_ulong liobn) {sPAPRTCETable *tcet;
Quoted 10 lines
hw/ppc/spapr_iommu.c enum sPAPRTCEAccess { · line 51
Quoted 5 lines
hw/ppc/spapr_iommu.c +2
-1if (liobn &0xFFFFFFFF00000000ULL ) { -hcall_dprintf ("Request for out-of-bounds LIOBN 0x" TARGET_FMT_lx"\n" , +qemu_log_mask (LOG_GUEST_ERROR , +"Request for out-of-bounds LIOBN 0x" TARGET_FMT_lx"\n" , liobn);I'd actually prefer to see this left for the time being. The dprintf stuff may be ugly, but it's used throughout this stuff, whereas the qemu logging is not. Plus I've found the qemu logging stuff (as opposed to the tracepoint stuff) to be a pain to actually use.
I used it a couple of times already and once you get used to it, I think it's quite useful. IMHO it's at least easier than to recompile the binary for detecting such issues.
Quoted 2 lines
So I'd prefer that you just fix the mask bug, and we look at a different patch to change the logging of the spapr stuff en masse.
Ok, fair, I'll change my patch accordingly.
Thomas