From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:34798) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TjH1z-00075W-UV for qemu-devel@nongnu.org; Thu, 13 Dec 2012 17:13:29 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TjH1w-0001wt-VC for qemu-devel@nongnu.org; Thu, 13 Dec 2012 17:13:27 -0500 Received: from smtp1-g21.free.fr ([2a01:e0c:1:1599::10]:60129) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TjH1w-0001vq-BE for qemu-devel@nongnu.org; Thu, 13 Dec 2012 17:13:24 -0500 Message-ID: <50CA52F3.3060508@reactos.org> Date: Thu, 13 Dec 2012 23:13:07 +0100 From: =?ISO-8859-15?Q?Herv=E9_Poussineau?= MIME-Version: 1.0 References: <1355410133-3281-1-git-send-email-lmr@redhat.com> <1355410133-3281-3-git-send-email-lmr@redhat.com> <50C9F6DC.3020508@suse.de> In-Reply-To: <50C9F6DC.3020508@suse.de> Content-Type: text/plain; charset=ISO-8859-15; format=flowed Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 1/2] hw: add isa-debug-exit device v3 List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?ISO-8859-15?Q?Andreas_F=E4rber?= Cc: Lucas Meneghel Rodrigues , jan.kiszka@siemens.com, Anthony Liguori , qemu-devel@nongnu.org, kraxel@redhat.com Andreas F=E4rber a =E9crit : > Am 13.12.2012 15:48, schrieb Lucas Meneghel Rodrigues: >> From: Gerd Hoffmann >> >> When present it makes qemu exit on any write. >> Mapped to port 0x501 by default. >> >> Without this patch Anthony doesn't allow me to >> remove the bochs bios debug ports because his >> test suite uses this. >> >> Changes from v2: Fixed rebase mistake >> >> Changes from v1: Add access_size property to the >> device, needed for kvm-unit-tests. >> >> Signed-off-by: Gerd Hoffmann >=20 >> --- >> hw/debugexit.c | 73 ++++++++++++++++++++++++++++++++++++++++= +++++++++ >> hw/i386/Makefile.objs | 2 +- >> 2 files changed, 74 insertions(+), 1 deletion(-) >> create mode 100644 hw/debugexit.c >> >> diff --git a/hw/debugexit.c b/hw/debugexit.c >> new file mode 100644 >> index 0000000..dfe1b2d >> --- /dev/null >> +++ b/hw/debugexit.c >> @@ -0,0 +1,73 @@ >> +/* >=20 > Add a one-line description plus some copyright line? >=20 >> + * This program is free software; you can redistribute it and/or >> + * modify it under the terms of the GNU General Public License as >> + * published by the Free Software Foundation; either version 2 or >> + * (at your option) version 3 of the License. >=20 > Why not "any later version"? >=20 >> + */ >> + >> +#include "hw.h" >> +#include "isa.h" >> + >> +#define TYPE_ISA_DEBUG_EXIT_DEVICE "isa-debug-exit" >> +#define ISA_DEBUG_EXIT_DEVICE(obj) \ >> + OBJECT_CHECK(ISADebugExitState, (obj), TYPE_ISA_DEBUG_EXIT_DEVIC= E) >> + >> +typedef struct ISADebugExitState { >> + ISADevice parent_obj; >> + >> + uint32_t iobase; >> + uint8_t access_size; >> + MemoryRegion io; >> +} ISADebugExitState; >> + >> +static void debug_exit_write(void *opaque, hwaddr addr, uint64_t val, >> + unsigned width) >> +{ >> + exit((val << 1) | 1); >> +} >> + >> +static const MemoryRegionOps debug_exit_ops =3D { >> + .write =3D debug_exit_write, >> + .valid.min_access_size =3D 1, >> + .valid.max_access_size =3D 1, >> + .endianness =3D DEVICE_LITTLE_ENDIAN, >> +}; >> + >> +static int debug_exit_initfn(ISADevice *dev) >> +{ >> + ISADebugExitState *isa =3D ISA_DEBUG_EXIT_DEVICE(dev); >> + >> + memory_region_init_io(&isa->io, &debug_exit_ops, isa, >> + TYPE_ISA_DEBUG_EXIT_DEVICE, isa->access_siz= e); >> + memory_region_add_subregion(isa_address_space_io(dev), >> + isa->iobase, &isa->io); >> + return 0; >> +} >> + >> +static Property debug_exit_properties[] =3D { >> + DEFINE_PROP_HEX32("iobase", ISADebugExitState, iobase, 0x501), >> + DEFINE_PROP_UINT8("access-size", ISADebugExitState, access_size, = 1), >> + DEFINE_PROP_END_OF_LIST(), >> +}; >> + >> +static void debug_exit_class_initfn(ObjectClass *klass, void *data) >> +{ >> + DeviceClass *dc =3D DEVICE_CLASS(klass); >> + ISADeviceClass *ic =3D ISA_DEVICE_CLASS(klass); >> + ic->init =3D debug_exit_initfn; >> + dc->props =3D debug_exit_properties; >> +} >> + >> +static TypeInfo debug_exit_info =3D { >=20 > static const, if resubmitted >=20 >> + .name =3D TYPE_ISA_DEBUG_EXIT_DEVICE, >> + .parent =3D TYPE_ISA_DEVICE, >> + .instance_size =3D sizeof(ISADebugExitState), >> + .class_init =3D debug_exit_class_initfn, >> +}; >> + >> +static void debug_exit_register_types(void) >> +{ >> + type_register_static(&debug_exit_info); >> +} >> + >> +type_init(debug_exit_register_types) >> diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs >> index 0d3f6a8..56aaa9d 100644 >> --- a/hw/i386/Makefile.objs >> +++ b/hw/i386/Makefile.objs >> @@ -3,7 +3,7 @@ obj-y +=3D apic_common.o apic.o kvmvapic.o >> obj-y +=3D sga.o ioapic_common.o ioapic.o piix_pci.o >> obj-y +=3D vmport.o >> obj-y +=3D pci-hotplug.o smbios.o wdt_ib700.o >> -obj-y +=3D debugcon.o multiboot.o >> +obj-y +=3D debugcon.o debugexit.o multiboot.o >> obj-y +=3D pc_piix.o >> obj-y +=3D pc_sysfw.o >> obj-y +=3D lpc_ich9.o q35.o pc_q35.o >=20 > Otherwise looks good, CC'ing Herv=E9 who did an earlier patch like this= . I see some differences with my patch available at=20 http://patchwork.ozlabs.org/patch/169542/ Your patch breaks behaviour for access sizes > 1. - Current code in QEMU accepts access sizes of 2, which means that if=20 you write value 0x1234 to port 0x501, you'll exit with (0x1234 << 1) | 1. - After your patch, as you're explicitly stating that=20 valid.max_access_size =3D 1, a write of size 2 will be considered as inva= lid. I never got any answer from Anthony if he is using access size of 1 or 2=20 for his test suite... (see http://patchwork.ozlabs.org/patch/169543/) I can easily revive my patch if Anthony wants something else that=20 iobase=3D0x501 and access-size=3D1 by default. Once again, putting Anthony in Cc to have an answer. Regards, Herv=E9