qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] hw/ppc/spapr_iommu: Fix the check for invalid u


From: David Gibson
Subject: Re: [Qemu-devel] [PATCH] hw/ppc/spapr_iommu: Fix the check for invalid upper bits in liobn
Date: Tue, 21 Apr 2015 10:24:48 +1000
User-agent: Mutt/1.5.23 (2014-03-12)

On Mon, Apr 20, 2015 at 05:34:56PM +0200, Thomas Huth wrote:
> 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.
> 
> Signed-off-by: Thomas Huth <address@hidden>

> ---
>  hw/ppc/spapr_iommu.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c
> index f3990fd..cd26a06 100644
> --- a/hw/ppc/spapr_iommu.c
> +++ b/hw/ppc/spapr_iommu.c
> @@ -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);

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.

>          return NULL;
>      }
>  
>      QLIST_FOREACH(tcet, &spapr_tce_tables, list) {
> -        if (tcet->liobn == liobn) {
> +        if (tcet->liobn == (uint32_t)liobn) {
>              return tcet;
>          }
>      }

-- 
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

Attachment: pgpowWBJAWW5P.pgp
Description: PGP signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]