From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36048) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XWpf8-00009s-Hb for qemu-devel@nongnu.org; Wed, 24 Sep 2014 12:43:35 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XWpf3-00032a-SL for qemu-devel@nongnu.org; Wed, 24 Sep 2014 12:43:30 -0400 Received: from mx1.redhat.com ([209.132.183.28]:5521) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XWpf3-00031B-Jx for qemu-devel@nongnu.org; Wed, 24 Sep 2014 12:43:25 -0400 Message-ID: <5422F4A2.6020305@redhat.com> Date: Wed, 24 Sep 2014 10:43:14 -0600 From: Eric Blake MIME-Version: 1.0 References: In-Reply-To: Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="Uv7qr2R18Fh9Og2VQpclStjhc73LgENjG" Subject: Re: [Qemu-devel] [IGDVFIO] [PATCH 1/8] RFC and help completing: Intel IGD Direct Assignment with VFIO List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Andrew Barnes , qemu-devel@nongnu.org Cc: jike.song@intel.com, Alex Williamson , intel-gfx@lists.freedesktop.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --Uv7qr2R18Fh9Og2VQpclStjhc73LgENjG Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 09/24/2014 07:20 AM, Andrew Barnes wrote: > hw/isa/lpc_ich9.c >=20 > this patch adds: > * debug output, if enabled > * enforces correct intel config, if enabled. (unsure if this is needed)= > * redirects some PCI Config to host > * uses hosts LPC device id >=20 > patch > --------------------- >=20 Stylistic review (I'll leave the content review to someone more knowledgeable) > @@ -46,6 +47,16 @@ > #include "exec/address-spaces.h" > #include "sysemu/sysemu.h" >=20 > +/* #define DEBUG_LPC */ > +#ifdef DEBUG_LPC > +# define LPC_DPRINTF(format, ...) print("LPC: " format, ## __VA_ARGS_= _) Most debugging prints to stderr, not stdout. Even better is using a tracepoint instead of a macro. > +#else > +# define LPC_DPRINTF(format, ...) do {} while (0) There has also been work to make sure that debugging printfs are still compiled for the sake of format checking, although optimized out by if (0), in the normal case, so as to avoid bit-rot in the debug format strings when not enabled. >=20 > +/* BEWARE: only set this if you are passing IGD through to guest */ > +/* TODO: detect IGD automatically */ > +static bool IGD_PASSTHROUGH =3D true; all-caps names are usually reserved for macros and constants, not variabl= es. > + > /* chipset configuration register > * to access chipset configuration registers, pci_[sg]et_{byte, word, = long} > * are used. > @@ -425,6 +440,9 @@ static void ich9_lpc_config_write(PCIDevice *d, > ICH9LPCState *lpc =3D ICH9_LPC_DEVICE(d); > uint32_t rbca_old =3D pci_get_long(d->config + ICH9_LPC_RCBA); >=20 > + LPC_DPRINTF("%s(%04x:%02x:%02x.%x, @0x%x, 0x%x, len=3D0x%x)\n", __= func__, > + 0000, 00, PCI_SLOT(d->devfn),PCI_FUNC(d->devfn), addr,= Style: space after comma. > +static uint32_t ich9_lpc_config_read(PCIDevice *d, > + uint32_t addr, int len) > +{ > + uint32_t val; > + if (IGD_PASSTHROUGH) > + { > + if (ranges_overlap(addr, len, 0x2c, 2) || /* SVID - Subsystem Vendor > Identification */ Indentation is off. May be related to how you pasted the patch in your mailer, rather than a flaw in your actual patch. > + ranges_overlap(addr, len, 0x2e, 2)) /* SID - Subsystem Identific= aion > */ > + { Style - we prefer the { on the same line as the if. Running your patches through scripts/checkpatch.pl will catch many of the style issues= =2E > + val =3D > host_pci_read_config(0,PCI_SLOT(d->devfn),PCI_FUNC(d->devfn),addr,len);= space after comma > + } > + else > + { > + goto defaultread; > + } More indentation damage. + > + /* For a UN-MODIFIED guest, the following 3 registers need to be r= ead > from the host. > + * alternatively, modify i915_drv.c, intel_detect_pch, add a check= for > + * PCI_DEVICE_ID_INTEL_ICH9_8 and copy the settings from the PCH y= ou > desire */ > + k->vendor_id =3D host_pci_read_config(0,0x1f,0,0x00,2); > + k->device_id =3D host_pci_read_config(0,0x1f,0,0x02,2); > +// k->revision =3D host_pci_read_config(0,0x1f,0,0x08,1); /**/ comments are preferred over //; also, it's good to explain with a comment why you are adding dead code. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --Uv7qr2R18Fh9Og2VQpclStjhc73LgENjG Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg iQEcBAEBCAAGBQJUIvSiAAoJEKeha0olJ0NqQmUH/jmEX2oFrkB835/c2jZCCsOL 9VxlMXPe//Uiaxq+64EFykflXFdH9a01W5Qxu4Gd5tJ0uck5M8y/k8VrlfxexOkA ghy3SMHfwPgza+/GjispeTc5y4XeAJfn3rKTQroPgMX+QgMZi/oc6kAiaU3sgr8M 4E4Vp1rj3r0TFAgByHru+zIb9KDST91NgIHvrtDTh4vtOAhCGaMXTGGNTzR8c7qg mEnVFrEF0Dx8YPhtlH3tZXHYBwp4LLCsFl5D+jmxOqQ1kqpF43E4nhve4HGt4uTr mgqAE6p1qz4BbyGCxN19kG0AMxVNJmCRiHgbKSEYJmIuMruT9ZScl+H/w83qp1c= =Y20u -----END PGP SIGNATURE----- --Uv7qr2R18Fh9Og2VQpclStjhc73LgENjG--