From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38957) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XOiHh-0002tJ-Tv for qemu-devel@nongnu.org; Tue, 02 Sep 2014 03:13:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XOiHc-00085I-Vx for qemu-devel@nongnu.org; Tue, 02 Sep 2014 03:13:45 -0400 Received: from mail-wi0-f171.google.com ([209.85.212.171]:54229) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XOiHc-00085E-Nt for qemu-devel@nongnu.org; Tue, 02 Sep 2014 03:13:40 -0400 Received: by mail-wi0-f171.google.com with SMTP id hi2so13686475wib.16 for ; Tue, 02 Sep 2014 00:13:40 -0700 (PDT) Message-ID: <54056E0C.6090507@linaro.org> Date: Tue, 02 Sep 2014 09:13:16 +0200 From: Eric Auger MIME-Version: 1.0 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> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 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: Alexander Graf Cc: "peter.maydell@linaro.org" , "kim.phillips@freescale.com" , "eric.auger@st.com" , "patches@linaro.org" , Kim Phillips , "joel.schopp@amd.com" , "will.deacon@arm.com" , "qemu-devel@nongnu.org" , "a.rigo@virtualopensystems.com" , "Bharat.Bhushan@freescale.com" , Alex Williamson , "stuart.yoder@freescale.com" , "a.motakis@virtualopensystems.com" , "kvmarm@lists.cs.columbia.edu" , "christoffer.dall@linaro.org" 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 ... Thanks Eric > > > Alex > >> >> Thanks in advance >> >> Best Regards >> >> Eric >>> >>> Alex >>