From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53320) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XOvOK-0004e8-3d for qemu-devel@nongnu.org; Tue, 02 Sep 2014 17:13:32 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XOvOF-0002t2-GJ for qemu-devel@nongnu.org; Tue, 02 Sep 2014 17:13:28 -0400 Received: from mx1.redhat.com ([209.132.183.28]:18289) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XOvOF-0002ss-8m for qemu-devel@nongnu.org; Tue, 02 Sep 2014 17:13:23 -0400 Message-ID: <1409692388.3804.135.camel@ul30vt.home> From: Alex Williamson Date: Tue, 02 Sep 2014 15:13:08 -0600 In-Reply-To: <54056E0C.6090507@linaro.org> References: <1407594349-9291-1-git-send-email-eric.auger@linaro.org> <1407594349-9291-7-git-send-email-eric.auger@linaro.org> <1407785130.9800.99.camel@ul30vt.home> <53E9AF8C.2070507@linaro.org> <1407959979.9800.186.camel@ul30vt.home> <54049F53.30102@linaro.org> <54056E0C.6090507@linaro.org> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v5 06/10] hw/vfio: create common module List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Auger Cc: "peter.maydell@linaro.org" , "kim.phillips@freescale.com" , "eric.auger@st.com" , "patches@linaro.org" , "qemu-devel@nongnu.org" , "joel.schopp@amd.com" , "will.deacon@arm.com" , "a.rigo@virtualopensystems.com" , Alexander Graf , "Bharat.Bhushan@freescale.com" , Kim Phillips , "stuart.yoder@freescale.com" , "a.motakis@virtualopensystems.com" , "kvmarm@lists.cs.columbia.edu" , "christoffer.dall@linaro.org" On Tue, 2014-09-02 at 09:13 +0200, Eric Auger wrote: > On 09/01/2014 07:41 PM, Alexander Graf wrote: > > > > > >> Am 01.09.2014 um 18:31 schrieb Eric Auger : > >> > >>> On 08/13/2014 09:59 PM, Alex Williamson wrote: > >>>> On Tue, 2014-08-12 at 08:09 +0200, Eric Auger wrote: > >>>>> On 08/11/2014 09:25 PM, Alex Williamson wrote: > >>>>>> On Sat, 2014-08-09 at 15:25 +0100, Eric Auger wrote: > >>>>>> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h > >>>>>> new file mode 100644 > >>>>>> index 0000000..4684ee5 > >>>>>> --- /dev/null > >>>>>> +++ b/include/hw/vfio/vfio-common.h > >>>>>> @@ -0,0 +1,151 @@ > >>>>>> +/* > >>>>>> + * common header for vfio based device assignment support > >>>>>> + * > >>>>>> + * Copyright Red Hat, Inc. 2012 > >>>>>> + * > >>>>>> + * Authors: > >>>>>> + * Alex Williamson > >>>>>> + * > >>>>>> + * This work is licensed under the terms of the GNU GPL, version 2. See > >>>>>> + * the COPYING file in the top-level directory. > >>>>>> + * > >>>>>> + * Based on qemu-kvm device-assignment: > >>>>>> + * Adapted for KVM by Qumranet. > >>>>>> + * Copyright (c) 2007, Neocleus, Alex Novik (alex@neocleus.com) > >>>>>> + * Copyright (c) 2007, Neocleus, Guy Zana (guy@neocleus.com) > >>>>>> + * Copyright (C) 2008, Qumranet, Amit Shah (amit.shah@qumranet.com) > >>>>>> + * Copyright (C) 2008, Red Hat, Amit Shah (amit.shah@redhat.com) > >>>>>> + * Copyright (C) 2008, IBM, Muli Ben-Yehuda (muli@il.ibm.com) > >>>>>> + */ > >>>>>> +#ifndef HW_VFIO_VFIO_COMMON_H > >>>>>> +#define HW_VFIO_VFIO_COMMON_H > >>>>>> + > >>>>>> +#include "qemu-common.h" > >>>>>> +#include "exec/address-spaces.h" > >>>>>> +#include "exec/memory.h" > >>>>>> +#include "qemu/queue.h" > >>>>>> +#include "qemu/notify.h" > >>>>>> + > >>>>>> +/*#define DEBUG_VFIO*/ > >>>>>> +#ifdef DEBUG_VFIO > >>>>>> +#define DPRINTF(fmt, ...) \ > >>>>>> + do { fprintf(stderr, "vfio: " fmt, ## __VA_ARGS__); } while (0) > >>>>>> +#else > >>>>>> +#define DPRINTF(fmt, ...) \ > >>>>>> + do { } while (0) > >>>>>> +#endif > >>>>> > >>>>> > >>>>> DPRINTF also need to be renamed to avoid conflicting namespace issues. > >>>> Ji Alex, > >>>> > >>>> OK. > >>>> > >>>> As I am going to touch at traces, > >>>> - are you OK if I use the new .name field to simply format strings? > >>> > >>> Sure, that's fine. > >>> > >>>> DPRINTF("%s(%04x:%02x:%02x.%x) Pin %c\n", __func__, vdev->host.domain, > >>>> vdev->host.bus, vdev->host.slot, vdev->host.function, > >>>> 'A' + vdev->intx.pin); > >>>> - Also Alex was suggesting to use trace points. What is your position > >>>> about that? Also I am not 100% sure of what it consists in? is it trace > >>>> events as documented in docs/tracing.txt > >>> > >>> I think it would be a great conversion, but it's not required. Thanks, > >> > >> Hi Alex, > >> > >> I am currently progressing on the conversion to trace points (I did it > >> for platform and common and now do the job for PCI). I wonder whether it > >> makes sense I convert all DPRINTF into trace-points or only convert a > >> subset (state transitions, ...). Would you accept a mixture of DPRINTFs > >> and trace-points or do you advise to convert everything? > > > > Yeah, it's perfectly good to even just nit introduce new dprintfs. > ok thanks > > > >> > >> Also the tracing.txt doc says we should use the name of the function as > >> prefix. That being said it could be interesting to trace all pci* or all > >> platform* and wildcard seems to work fine to select the trace-events. So > >> my second question is would you accept using pci__* as a > >> generic pattern. > > > > Not sure - maybe be more explicit and call it vfio_pci_...? > well. maybe as a first draft I will follow the tracing.txt guideline and > you will tell me, both Alex's, what you think of the outcome. Anyway it > is not a big deal then to change ... I haven't touched tracing yet, so I'll defer to you and agraf for now ;) Thanks, Alex