Am Tue, 21 Apr 2015 10:24:48 +1000
schrieb David Gibson <da...@gibson.dropbear.id.au>:
> 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 <th...@redhat.com>
>
> > ---
> > 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.
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.
> 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
