From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49766) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YkRc6-0001Ru-5R for qemu-devel@nongnu.org; Tue, 21 Apr 2015 02:24:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YkRZy-0006Cp-F6 for qemu-devel@nongnu.org; Tue, 21 Apr 2015 02:22:43 -0400 Date: Tue, 21 Apr 2015 08:22:32 +0200 From: Thomas Huth Message-ID: <20150421082232.5e59aa5c@thh440s> In-Reply-To: <20150421002448.GA31815@voom.redhat.com> References: <1429544096-1387-1-git-send-email-thuth@redhat.com> <20150421002448.GA31815@voom.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] hw/ppc/spapr_iommu: Fix the check for invalid upper bits in liobn List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Gibson Cc: aik@ozlabs.ru, qemu-ppc@nongnu.org, qemu-devel@nongnu.org, agraf@suse.de Am Tue, 21 Apr 2015 10:24:48 +1000 schrieb David Gibson : > 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 > > > --- > > 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