[Qemu-devel] QEMU API for modelling of user's hardware
Hi, I wanted to prepare QEMU models of new hardware for my students, to let them to write and debug device drivers before the real hardware is available (or even to optimize the design of this hardware before it is really made). I was able to establish the basic communication with memory mapped registers of my device, but when it comes to more advanced topics like DMA and interrupts, things start to be difficult. Is there any reference or manual for people trying to write device models for QEMU? I know that the hw subdirectory is full of examples, but I'd prefere something more formal, with info which solutions are optimal, which are deprecated... Thank you very much in advance, Wojciech Zabolotny
[Qemu-devel] Re: [PATCH 3/4] Introduce machine state
On 03/26/2011 11:28 PM, Blue Swirl wrote: Move generic machine state to machine-state.h, adjust users. What's the distinction between vm state and machine state? Perhaps your vm state is more appropriately called emulator state (i.e. sits between host and vm), and machine state is actually vm state? BTW, uuid should be in machine state rather than (your definition of) vm state. Paolo
[Qemu-devel] Re: [PATCH] floppy: save and restore DIR register
On 03/28/2011 04:40 AM, Jason Wang wrote: On 03/25/2011 07:27 AM, Jason Wang wrote: We need to keep DIR register unchanged across migration, but currently it depends on the media_changed flags from block layer and we do not save/restore it which could let the guest driver think the floppy have changed after migration. To fix this, a new filed media_changed in FDrive strcutre was introduced in order to save and restore the it from block layer through pre_save/post_load callbacks. I guess you can avoid saving if the media changed flag is zero, too (which should be the common case after the guest has booted, right?). Paolo Yes, zero is the common case, but the bdrv_open() called by listening qemu in dest mode would always set the media_changed to one, so we must save and restore it during migration. If zero is the common case, you can do something like this to avoid saving the subsection in the common case and allow seamless migration from 0.15 to 0.14: static void fdrive_pre_save(void *opaque) { FDrive *drive = opaque; if (drive-bs != NULL) { drive-media_changed = drive-bs-media_changed; } else { drive-media_changed = 2; } } static void fdrive_pre_load(void *opaque) { FDrive *drive = opaque; /* Set the default value in case there is no subsection. */ if (drive-bs != NULL) { drive-media_changed = 0; } else { drive-media_changed = 2; } } static void fdrive_post_load(void *opaque) { /* Fail if the source machine had a disk and we don't. */ if (drive-bs == NULL drive-media_changed != 2) { return 1; } /* Reset the field if both us and the source machine have a disk. */ if (drive-bs != NULL drive-media_changed != 2) { /* bdrv_open() called in dest node may set the media_changed flag when trying to open floppy image. Copy it here, so that it is reset even if the subsection was not saved. */ drive-bs-media_changed = drive-media_changed; } return 0; } static bool fdrive_media_changed_needed(void *opaque) { FDrive *drive = opaque; return (drive-bs != NULL drive-media_changed != 0); } Note that there is a small change here: if the media changed flag is 1 and you migrate 0.14 to 0.15, the destination machine will see the flag reset to 0 (while 0.14 to 0.14 will see the flag set to 1 just because that's how bdrv_open leaves it). To fix this, you can define a boolean qdev property default_migration_media_changed and use it instead of the two hard-coded 0 values. Then, register the default value of the property in hw/pc_piix.c to be 0 in pc-0.15, and 1 in pc-0.14 and older. Paolo
Re: [Qemu-devel] [V2 PATCH] floppy: save and restore DIR register
On 03/28/11 04:47, Jason Wang wrote: We need to keep DIR register unchanged across migration, but currently it depends on the media_changed flags from block layer. Since we do not save/restore it and the bdrv_open() called in dest node may set the media_changed flag when trying to open floppy image, guest driver may think the floppy have changed after migration. To fix this, a new filed media_changed in FDrive strcutre was introduced in order to save and restore the it from block layer through pre_save/post_load callbacks. Signed-off-by: Jason Wang jasow...@redhat.com --- hw/fdc.c | 52 +++- 1 files changed, 51 insertions(+), 1 deletions(-) Looks good to me. Reviewed-by: Jes Sorensen jes.soren...@redhat.com
Re: [Qemu-devel] [PATCH v2] hw/pc: Support system flash memory with -pflash parameter
On 03/24/11 03:07, jordan.l.jus...@intel.com wrote: +static void pc_isa_bios_init(ram_addr_t ram_offset, int ram_size) +{ +int isa_bios_size; + +/* map the last 128KB of the BIOS in ISA space */ +isa_bios_size = ram_size; +if (isa_bios_size (128 * 1024)) +isa_bios_size = 128 * 1024; You're missing braces here to comply with coding style. I realize some of the cosmetics comes from old code you are moving, but please fix it up in the move. +cpu_register_physical_memory(0x10 - isa_bios_size, + isa_bios_size, + (ram_offset + ram_size - isa_bios_size) | IO_MEM_ROM); A line is 80 characters, please wrap it or calculate the number before the function call. +bdrv = pflash_drv-bdrv; +size = bdrv_getlength(pflash_drv-bdrv); +sector_bits = 12; +sector_size = 1 sector_bits; + +if ((size % sector_size) != 0) { +fprintf(stderr, +qemu: -pflash size must be a multiple of 0x%x\n, +sector_size); +exit(1); +} + +phys_addr = 0x1ULL - rom_size - size; +addr = qemu_ram_alloc(NULL, system.flash, size); +DPRINTF(flash addr: 0x%lx\n, (int64_t)phys_addr); +#if 1 +pflash_cfi01_register(phys_addr, addr, bdrv, + sector_size, size sector_bits, + 4, 0x, 0x, 0x, 0x, 0); +#else +pflash_cfi02_register(phys_addr, addr, bdrv, + sector_size, size sector_bits, + 1, 4, 0x, 0x, 0x, 0x, 0x55, 0xaa, 0); +#endif Please get rid of the #ifdef here. Cheers, Jes
Re: [Qemu-devel] [PATCH 27/27] Add SLOF-based partition firmware for pSeries machine, allowing more boot options
On 28.03.2011, at 03:19, David Gibson wrote: On Fri, Mar 25, 2011 at 01:29:17PM -0500, Anthony Liguori wrote: On 03/24/2011 10:21 PM, David Gibson wrote: Currently, the emulated pSeries machine requires the use of the -kernel parameter in order to explicitly load a guest kernel. This means booting from the virtual disk, cdrom or network is not possible. This patch addresses this limitation by inserting a within-partition firmware image (derived from the SLOF free Open Firmware project). If -kernel is not specified, qemu will now load the SLOF image, which has access to the qemu boot device list through the device tree, and can boot from any of the usual virtual devices. In order to support the new firmware, an extension to the emulated machine/hypervisor is necessary. Unlike Linux, which expects multi-CPU entry to be handled kexec() style, the SLOF firmware expects only one CPU to be active at entry, and to use a hypervisor RTAS method to enable the other CPUs one by one. This patch also implements this 'start-cpu' method, so that SLOF can start the secondary CPUs and marshal them into the kexec() holding pattern ready for entry into the guest OS. Linux should, and in the future might directly use the start-cpu method to enable initially disabled CPUs, but for now it does require kexec() entry. Signed-off-by: Benjamin Herrenschmidtb...@kernel.crashing.org Signed-off-by: Paul Mackerraspau...@samba.org Signed-off-by: David Gibsond...@au1.ibm.com We should pull in SLOF via a git submodule. That ensures we ship the source code along with the binary. Um, ok. Do I need to do anything about this? I'm also not sure this is too important. Most of our firmware blobs come from svn repos which can't be submoduled. And as long as we don't have a consistent policy about it, we can just as well stick with the README file. Alex
Re: [Qemu-devel] [PATCH] cirrus_vga: Remove unneeded reset
Isaku Yamahata yamah...@valinux.co.jp writes: On Mon, Mar 28, 2011 at 07:18:04AM +0200, Stefan Weil wrote: Am 28.03.2011 04:17, schrieb Isaku Yamahata: [...] On Sat, Mar 26, 2011 at 10:53:09PM +0100, Stefan Weil wrote: cirrus_reset is also called by the pci framework, so there is no need to call it in cirrus_init_common. Cc: Michael S. Tsirkinm...@redhat.com Signed-off-by: Stefan Weilw...@mail.berlios.de [...] I tested the new code with isa pc, too. In gdb, I could see that it also calls cirrus_reset twice. But isa pc is broken since the switch to sea bios, so obviously isa is an unmaintained part of qemu. Even with bochs bios, it no longer works, so it is broken at least twice. Ah, I see. The the second reset is called not via pci reset framework, but qemu reset framework. So removing the above reset call makes sense. It would be another patch to make use of pci reset framework. Then the proposed commit message's claim cirrus_reset() is called by the pci framework is incorrect, isn't it?
Re: [Qemu-devel] [PATCH] cirrus_vga: Remove unneeded reset
On Mon, Mar 28, 2011 at 11:21:23AM +0200, Markus Armbruster wrote: Isaku Yamahata yamah...@valinux.co.jp writes: On Mon, Mar 28, 2011 at 07:18:04AM +0200, Stefan Weil wrote: Am 28.03.2011 04:17, schrieb Isaku Yamahata: [...] On Sat, Mar 26, 2011 at 10:53:09PM +0100, Stefan Weil wrote: cirrus_reset is also called by the pci framework, so there is no need to call it in cirrus_init_common. Cc: Michael S. Tsirkinm...@redhat.com Signed-off-by: Stefan Weilw...@mail.berlios.de [...] I tested the new code with isa pc, too. In gdb, I could see that it also calls cirrus_reset twice. But isa pc is broken since the switch to sea bios, so obviously isa is an unmaintained part of qemu. Even with bochs bios, it no longer works, so it is broken at least twice. Ah, I see. The the second reset is called not via pci reset framework, but qemu reset framework. So removing the above reset call makes sense. It would be another patch to make use of pci reset framework. Then the proposed commit message's claim cirrus_reset() is called by the pci framework is incorrect, isn't it? Yes, incorrect. The commit message should be fixed. The code change itself looks correct. -- yamahata
Re: [Qemu-devel] Re: KVM call agenda for Jan 25
Am 26.03.2011 22:56, schrieb Dushyant Bansal: On the other hand, I think the starting point for a generic in-place converter would be a loop that does something like bdrv_is_allocated() but translates the guest position in the block device into an offset into the image file. That, together with some sort of free map or space allocation bitmap would allow a generic approach to figuring out the data mapping and which parts of the file can be safely used. We can discuss the detailed API later, but I agree that the critical thing to convert is the mapping. You would probably open the file with the source format driver read-only and with the destination driver read-write. For qcow2 you would start with writing a refcount table that marks the whole file as used, other formats use the file size anyway. Then you can start creating L1 and L2 tables and copy the mapping over. Once this is done, you do an fsck to free the metadata of the old format. One thing that may become tricky is the image header which both drivers may want to use and which is fixed at offset 0. And of course, you must make sure that the image is safe at any point if the converter crashes. For image header issue, this is the approach that comes to mind. Lets say, destination format is qcow2. BDRVQcowState is responsible for header fields inside BlockDriverState. We need qcow2 image header to initiliaze all the fields of BDRVQcowState, which is done by bdrv_open(qcow2_open()). So initially, for the qcow2 driver, we do not copy the qcow2 image header (we keep the source header). We can then manually set fields of BDRVQcowState with the desired header fields. And after all other metadata has been copied for the qcow2 format, we can replace the source image header with the qcow2 header. The question is if you can make sure that while you convert the image, qcow2 will never try to modify the header. It will definitely do so if you have to increase the L1 or refcount table, for example. In the end, we may have to use tricks like using a protocol which keeps the first cluster in a ramdisk until the conversion is complete. Kevin
[Qemu-devel] Re: [PATCH 27/27] Add SLOF-based partition firmware for pSeries machine, allowing more boot options
On 25.03.2011, at 04:21, David Gibson wrote: Currently, the emulated pSeries machine requires the use of the -kernel parameter in order to explicitly load a guest kernel. This means booting from the virtual disk, cdrom or network is not possible. This patch addresses this limitation by inserting a within-partition firmware image (derived from the SLOF free Open Firmware project). If -kernel is not specified, qemu will now load the SLOF image, which has access to the qemu boot device list through the device tree, and can boot from any of the usual virtual devices. In order to support the new firmware, an extension to the emulated machine/hypervisor is necessary. Unlike Linux, which expects multi-CPU entry to be handled kexec() style, the SLOF firmware expects only one CPU to be active at entry, and to use a hypervisor RTAS method to enable the other CPUs one by one. This patch also implements this 'start-cpu' method, so that SLOF can start the secondary CPUs and marshal them into the kexec() holding pattern ready for entry into the guest OS. Linux should, and in the future might directly use the start-cpu method to enable initially disabled CPUs, but for now it does require kexec() entry. Signed-off-by: Benjamin Herrenschmidt b...@kernel.crashing.org Signed-off-by: Paul Mackerras pau...@samba.org Signed-off-by: David Gibson d...@au1.ibm.com --- Makefile |2 +- hw/spapr.c | 35 +--- hw/spapr_rtas.c | 78 ++ pc-bios/README |5 +++ pc-bios/slof.bin | Bin 0 - 579072 bytes 5 files changed, 115 insertions(+), 5 deletions(-) create mode 100644 pc-bios/slof.bin diff --git a/Makefile b/Makefile index e0b3fea..989622b 100644 --- a/Makefile +++ b/Makefile @@ -214,7 +214,7 @@ pxe-rtl8139.bin pxe-virtio.bin \ bamboo.dtb petalogix-s3adsp1800.dtb petalogix-ml605.dtb \ multiboot.bin linuxboot.bin \ s390-zipl.rom \ -spapr-rtas.bin +spapr-rtas.bin slof.bin else BLOBS= endif diff --git a/hw/spapr.c b/hw/spapr.c index 9d611a7..c6454e6 100644 --- a/hw/spapr.c +++ b/hw/spapr.c @@ -44,6 +44,10 @@ #define INITRD_LOAD_ADDR0x0280 #define FDT_MAX_SIZE0x1 #define RTAS_MAX_SIZE 0x1 +#define FW_MAX_SIZE 0x40 +#define FW_FILE_NAMEslof.bin + +#define MIN_RAM_SLOF 512UL #define TIMEBASE_FREQ 51200ULL @@ -56,6 +60,7 @@ static void *spapr_create_fdt(int *fdt_size, ram_addr_t ramsize, sPAPREnvironment *spapr, target_phys_addr_t initrd_base, target_phys_addr_t initrd_size, + const char *boot_device, const char *kernel_cmdline, target_phys_addr_t rtas_addr, target_phys_addr_t rtas_size, @@ -104,6 +109,7 @@ static void *spapr_create_fdt(int *fdt_size, ram_addr_t ramsize, start_prop, sizeof(start_prop; _FDT((fdt_property(fdt, linux,initrd-end, end_prop, sizeof(end_prop; +_FDT((fdt_property_string(fdt, qemu,boot-device, boot_device))); _FDT((fdt_end_node(fdt))); @@ -260,7 +266,7 @@ static void ppc_spapr_init(ram_addr_t ram_size, ram_addr_t ram_offset; target_phys_addr_t fdt_addr, rtas_addr; uint32_t kernel_base, initrd_base; -long kernel_size, initrd_size, htab_size, rtas_size; +long kernel_size, initrd_size, htab_size, rtas_size, fw_size; long pteg_shift = 17; int fdt_size; char *filename; @@ -392,13 +398,33 @@ static void ppc_spapr_init(ram_addr_t ram_size, initrd_size = 0; } } else { -fprintf(stderr, pSeries machine needs -kernel for now); -exit(1); +if (ram_size (MIN_RAM_SLOF 20)) { +fprintf(stderr, qemu: pSeries SLOF firmware requires = +%ldM guest RAM\n, MIN_RAM_SLOF); +exit(1); +} +filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, slof.bin); +fw_size = load_image_targphys(filename, 0, FW_MAX_SIZE); +if (fw_size 0) { +hw_error(qemu: could not load LPAR rtas '%s'\n, filename); +exit(1); +} +qemu_free(filename); +kernel_base = 0x100; +initrd_base = 0; +initrd_size = 0; + +/* SLOF will startup the secondary CPUs using RTAS, + rather than expecting a kexec() style entry */ +for (i = 0; i smp_cpus; i++) { +envs[i]-halted = 1; +} } /* Prepare the device tree */ fdt = spapr_create_fdt(fdt_size, ram_size, cpu_model, envs, spapr, - initrd_base, initrd_size, kernel_cmdline, + initrd_base, initrd_size, +
Re: [Qemu-devel] [PATCH 0/2] Let boards state maximum RAM limits in QEMUMachine struct
On 26 March 2011 11:41, Blue Swirl blauwir...@gmail.com wrote: On Mon, Mar 21, 2011 at 7:47 PM, Peter Maydell peter.mayd...@linaro.org wrote: This fairly simple patchset adds a new 'max_ram' field to the QEMUMachine structure so that a board model can specify the maximum RAM it will accept. We can then produce a friendly diagnostic message when the user tries to start qemu with a '-m' option asking for more RAM than that. (Currently most of the ARM devboard models respond with an obscure guest crash when the guest tries to access RAM and finds device registers instead.) This could replace the field max_mem in hwdef structures in sun4m.c. I've had a go at this, and it's revealed a flaw in this patchset, which is that the max_ram field needs to be type target_physaddr_t rather than ram_addr_t. I'll post another patchset including a sun4m cleanup(*) once I've done some testing on it. (*) includes combining the QEMUMachine struct into the sun4*_hwdef struct, which is necessary so the init fn can get at it and also I think neater. -- PMM
[Qemu-devel] Re: [PATCH 27/27] Add SLOF-based partition firmware for pSeries machine, allowing more boot options
On 03/28/2011 12:30 PM, Alexander Graf wrote: diff --git a/pc-bios/README b/pc-bios/README index 3fc0944..646a31a 100644 --- a/pc-bios/README +++ b/pc-bios/README @@ -13,6 +13,11 @@ The included image for PowerPC (for 32 and 64 bit PPC CPUs), Sparc32 and Sparc64 are built from OpenBIOS SVN revision 1018. +- SLOF (Slimline Open Firmware) is a free IEEE 1275 Open Firmware + implementation for certain IBM POWER hardware. The sources are at + https://github.com/dgibson/SLOF, and the image currently in qemu is + built from git tag qemu-slof-20110323. + - The PXE roms come from Rom-o-Matic gPXE 0.9.9 with BANNER_TIMEOUT=0 Is this a line removal? No, it's a bug in your mailer. :) Glad to see Thunderbird isn't the only one that butcher replies to patches (in addition to patches of course). Paolo
[Qemu-devel] Re: [PATCH 27/27] Add SLOF-based partition firmware for pSeries machine, allowing more boot options
On 03/28/2011 12:30 PM, Alexander Graf wrote: On 25.03.2011, at 04:21, David Gibson wrote: Currently, the emulated pSeries machine requires the use of the -kernel parameter in order to explicitly load a guest kernel. This means booting from the virtual disk, cdrom or network is not possible. This patch addresses this limitation by inserting a within-partition firmware image (derived from the SLOF free Open Firmware project). If -kernel is not specified, qemu will now load the SLOF image, which has access to the qemu boot device list through the device tree, and can boot from any of the usual virtual devices. In order to support the new firmware, an extension to the emulated machine/hypervisor is necessary. Unlike Linux, which expects multi-CPU entry to be handled kexec() style, the SLOF firmware expects only one CPU to be active at entry, and to use a hypervisor RTAS method to enable the other CPUs one by one. This patch also implements this 'start-cpu' method, so that SLOF can start the secondary CPUs and marshal them into the kexec() holding pattern ready for entry into the guest OS. Linux should, and in the future might directly use the start-cpu method to enable initially disabled CPUs, but for now it does require kexec() entry. Signed-off-by: Benjamin Herrenschmidtb...@kernel.crashing.org Signed-off-by: Paul Mackerraspau...@samba.org Signed-off-by: David Gibsond...@au1.ibm.com --- Makefile |2 +- hw/spapr.c | 35 +--- hw/spapr_rtas.c | 78 ++ pc-bios/README |5 +++ pc-bios/slof.bin | Bin 0 - 579072 bytes 5 files changed, 115 insertions(+), 5 deletions(-) create mode 100644 pc-bios/slof.bin diff --git a/Makefile b/Makefile index e0b3fea..989622b 100644 --- a/Makefile +++ b/Makefile @@ -214,7 +214,7 @@ pxe-rtl8139.bin pxe-virtio.bin \ bamboo.dtb petalogix-s3adsp1800.dtb petalogix-ml605.dtb \ multiboot.bin linuxboot.bin \ s390-zipl.rom \ -spapr-rtas.bin +spapr-rtas.bin slof.bin else BLOBS= endif diff --git a/hw/spapr.c b/hw/spapr.c index 9d611a7..c6454e6 100644 --- a/hw/spapr.c +++ b/hw/spapr.c @@ -44,6 +44,10 @@ #define INITRD_LOAD_ADDR0x0280 #define FDT_MAX_SIZE0x1 #define RTAS_MAX_SIZE 0x1 +#define FW_MAX_SIZE 0x40 +#define FW_FILE_NAMEslof.bin + +#define MIN_RAM_SLOF 512UL #define TIMEBASE_FREQ 51200ULL @@ -56,6 +60,7 @@ static void *spapr_create_fdt(int *fdt_size, ram_addr_t ramsize, sPAPREnvironment *spapr, target_phys_addr_t initrd_base, target_phys_addr_t initrd_size, + const char *boot_device, const char *kernel_cmdline, target_phys_addr_t rtas_addr, target_phys_addr_t rtas_size, @@ -104,6 +109,7 @@ static void *spapr_create_fdt(int *fdt_size, ram_addr_t ramsize, start_prop, sizeof(start_prop; _FDT((fdt_property(fdt, linux,initrd-end, end_prop, sizeof(end_prop; +_FDT((fdt_property_string(fdt, qemu,boot-device, boot_device))); _FDT((fdt_end_node(fdt))); @@ -260,7 +266,7 @@ static void ppc_spapr_init(ram_addr_t ram_size, ram_addr_t ram_offset; target_phys_addr_t fdt_addr, rtas_addr; uint32_t kernel_base, initrd_base; -long kernel_size, initrd_size, htab_size, rtas_size; +long kernel_size, initrd_size, htab_size, rtas_size, fw_size; long pteg_shift = 17; int fdt_size; char *filename; @@ -392,13 +398,33 @@ static void ppc_spapr_init(ram_addr_t ram_size, initrd_size = 0; } } else { -fprintf(stderr, pSeries machine needs -kernel for now); -exit(1); +if (ram_size (MIN_RAM_SLOF 20)) { +fprintf(stderr, qemu: pSeries SLOF firmware requires= +%ldM guest RAM\n, MIN_RAM_SLOF); +exit(1); +} +filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, slof.bin); +fw_size = load_image_targphys(filename, 0, FW_MAX_SIZE); +if (fw_size 0) { +hw_error(qemu: could not load LPAR rtas '%s'\n, filename); +exit(1); +} +qemu_free(filename); +kernel_base = 0x100; +initrd_base = 0; +initrd_size = 0; + +/* SLOF will startup the secondary CPUs using RTAS, + rather than expecting a kexec() style entry */ +for (i = 0; i smp_cpus; i++) { +envs[i]-halted = 1; +} } /* Prepare the device tree */ fdt = spapr_create_fdt(fdt_size, ram_size, cpu_model, envs, spapr, - initrd_base, initrd_size, kernel_cmdline, + initrd_base, initrd_size, + boot_device, kernel_cmdline,
[Qemu-devel] Re: [PATCH] e1000: check buffer availability
Michael S. Tsirkin m...@redhat.com wrote: Reduce spurious packet drops on RX ring empty by verifying that we have at least 1 buffer ahead of the time. Signed-off-by: Michael S. Tsirkin m...@redhat.com Reviewed-by: Juan Quintela quint...@redhat.com
[Qemu-devel] Re: [PATCH 27/27] Add SLOF-based partition firmware for pSeries machine, allowing more boot options
On 28.03.2011, at 12:51, Paolo Bonzini wrote: On 03/28/2011 12:30 PM, Alexander Graf wrote: diff --git a/pc-bios/README b/pc-bios/README index 3fc0944..646a31a 100644 --- a/pc-bios/README +++ b/pc-bios/README @@ -13,6 +13,11 @@ The included image for PowerPC (for 32 and 64 bit PPC CPUs), Sparc32 and Sparc64 are built from OpenBIOS SVN revision 1018. +- SLOF (Slimline Open Firmware) is a free IEEE 1275 Open Firmware + implementation for certain IBM POWER hardware. The sources are at + https://github.com/dgibson/SLOF, and the image currently in qemu is + built from git tag qemu-slof-20110323. + - The PXE roms come from Rom-o-Matic gPXE 0.9.9 with BANNER_TIMEOUT=0 Is this a line removal? No, it's a bug in your mailer. :) Glad to see Thunderbird isn't the only one that butcher replies to patches (in addition to patches of course). Glad to head that it's a bug O_o. Phew :) Alex
[Qemu-devel] Re: [V2 PATCH] floppy: save and restore DIR register
Jason Wang jasow...@redhat.com wrote: We need to keep DIR register unchanged across migration, but currently it depends on the media_changed flags from block layer. Since we do not save/restore it and the bdrv_open() called in dest node may set the media_changed flag when trying to open floppy image, guest driver may think the floppy have changed after migration. To fix this, a new filed media_changed in FDrive strcutre was introduced in order to save and restore the it from block layer through pre_save/post_load callbacks. Signed-off-by: Jason Wang jasow...@redhat.com Reviewed-by: Juan Quintela quint...@redhat.com
[Qemu-devel] Re: [PATCH v5 0/4] piix_pci: optimize irq data path
On Sun, Mar 27, 2011 at 04:56:29PM +0200, Michael S. Tsirkin wrote: On Wed, Mar 23, 2011 at 11:17:19AM +0900, Isaku Yamahata wrote: v4 has minor typo. I sent it too early. Here's fixed one. v3 - v4 Main changes are - use pirq, pci_intx instead of irq_num in piix_pci.c - patch 4/4 cleans the code a bit With this applied e1000 fails to work for me. Command line: qemu-system-x86_64 -enable-kvm -m 1G -drive if=virtio,file=/home/mst/rhel6.qcow2 -netdev user,id=bar -net nic,netdev=bar,model=e1000,macaddr=52:54:00:12:34:57 -redir tcp:8022::22 -net nic,model=e1000,netdev=foo,macaddr=52:54:00:12:34:56 -netdev tap,id=foo,ifname=msttap0,script=/home/mst/ifup,downscript=no -nographic Could you try that please? Does the following patch help? If so, I'll prepare v6. diff --git a/hw/piix_pci.c b/hw/piix_pci.c index c019793..5f0d92f 100644 --- a/hw/piix_pci.c +++ b/hw/piix_pci.c @@ -277,7 +277,8 @@ static void piix3_set_irq_pic(PIIX3State *piix3, int pic_irq) { qemu_set_irq(piix3-pic[pic_irq], !!(piix3-pic_levels -((PIIX_NUM_PIRQS - 1) (pic_irq * PIIX_NUM_PIRQS; +(((1UL PIIX_NUM_PIRQS) - 1) + (pic_irq * PIIX_NUM_PIRQS; } static void piix3_set_irq_level(PIIX3State *piix3, int pirq, int level) -- yamahata
[Qemu-devel] Re: [0/27] Implement emulation of pSeries logical partitions (v5)
On 25.03.2011, at 04:21, David Gibson wrote: This patch series adds a pseries machine to qemu, allowing it to emulate IBM pSeries logical partitions. More specifically it implements the interface defined by the PowerPC Architecture Platform Requirements document (PAPR, or sPAPR for short). Along the way we add a bunch of support for more modern ppc CPUs than are currently supported. It also makes some significant cleanups to the translation code for hash page table based ppc MMUs. Please apply. Acked-by: Alexander Graf ag...@suse.de Alex
Re: [Qemu-devel] Relative/Absolute timing snapshot problem
On 03/18/11 21:39, Clemens Kolbitsch wrote: Hi list, strange situation: When I create a snapshot using Qemu 0.14.0 stable, everything works smoothly and resuming the CPU takes about 1-2 seconds. If I don't use the snapshot file for some time, the time it takes to resume grows by 2-3 seconds per day. At the moment, I'm looking at a snapshot file from last week and it takes nearly 30 seconds to load. Funny thing about it: if I turn my system time back to the date when the snapshot was created (or before that), resuming CPU works within the expected 1-2 seconds. I have _very briefly_ looked into it and it seems like Qemu spends an aweful long amount of time catching up with timer execution -- is it possible that these are stored using absolute time instead of relative timing? I am using qcow2 file format, because I absolutely rely on CPU-snapshots and support for base-files. I have read here and there that it is more or less broken (or at least very slow), but with the correct cache-options it works for me (except for this bug, of course). Has anyone encountered this or should I start looking into it (although I have some experience with the core source, I'm not very experienced with the snapshotting code). Hi Clemens, Could you clarify what you are doing, when you say snapshot do you mean a savevm operation (ie. checkpoint) or a disk snapshot? Cheers, Jes
[Qemu-devel] Re: [PATCH v5 0/4] piix_pci: optimize irq data path
On Mon, Mar 28, 2011 at 08:19:56PM +0900, Isaku Yamahata wrote: On Sun, Mar 27, 2011 at 04:56:29PM +0200, Michael S. Tsirkin wrote: On Wed, Mar 23, 2011 at 11:17:19AM +0900, Isaku Yamahata wrote: v4 has minor typo. I sent it too early. Here's fixed one. v3 - v4 Main changes are - use pirq, pci_intx instead of irq_num in piix_pci.c - patch 4/4 cleans the code a bit With this applied e1000 fails to work for me. Command line: qemu-system-x86_64 -enable-kvm -m 1G -drive if=virtio,file=/home/mst/rhel6.qcow2 -netdev user,id=bar -net nic,netdev=bar,model=e1000,macaddr=52:54:00:12:34:57 -redir tcp:8022::22 -net nic,model=e1000,netdev=foo,macaddr=52:54:00:12:34:56 -netdev tap,id=foo,ifname=msttap0,script=/home/mst/ifup,downscript=no -nographic Could you try that please? Does the following patch help? Seems to help, but I have to ask - how did you test v5? If so, I'll prepare v6. diff --git a/hw/piix_pci.c b/hw/piix_pci.c index c019793..5f0d92f 100644 --- a/hw/piix_pci.c +++ b/hw/piix_pci.c @@ -277,7 +277,8 @@ static void piix3_set_irq_pic(PIIX3State *piix3, int pic_irq) { qemu_set_irq(piix3-pic[pic_irq], !!(piix3-pic_levels -((PIIX_NUM_PIRQS - 1) (pic_irq * PIIX_NUM_PIRQS; +(((1UL PIIX_NUM_PIRQS) - 1) + (pic_irq * PIIX_NUM_PIRQS; I think we should just make it ~0ULL (pic_irq * PIIX_NUM_PIRQS). Didn't try this though. } static void piix3_set_irq_level(PIIX3State *piix3, int pirq, int level) -- yamahata
[Qemu-devel] Re: [PATCH] e1000: check buffer availability
Am 27.03.2011 17:01, schrieb Michael S. Tsirkin: Reduce spurious packet drops on RX ring empty by verifying that we have at least 1 buffer ahead of the time. Signed-off-by: Michael S. Tsirkin m...@redhat.com Acked-by: Kevin Wolf kw...@redhat.com
Re: [Qemu-devel] [PATCH v23 01/11] trace: move trace objects from Makefile to Makefile.objs
On 03/23/11 14:19, Alon Levy wrote: --- Makefile | 32 Makefile.objs | 32 2 files changed, 32 insertions(+), 32 deletions(-) Acked-by: Jes Sorensen jes.soren...@redhat.com This should be able to go in as is - even other parts of ccid should stall. Cheers, Jes
Re: [Qemu-devel] [PATCH v23 02/11] qemu-thread.h: include inttypes.h
On 03/23/11 14:19, Alon Levy wrote: qemu-thread.h relies on uint64_t being defined, but doesn't include inttypes.h explicitly. This makes it easier to use it from vscclient (part of libcacard). --- qemu-thread.h |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) Acked-by: Jes Sorensen jes.soren...@redhat.com This should be able to go in as is - even other parts of ccid should stall. Cheers, Jes
Re: [Qemu-devel] [PATCH v23 03/11] usb-ccid: add CCID bus
On 03/23/11 14:19, Alon Levy wrote: A CCID device is a smart card reader. It is a USB device, defined at [1]. This patch introduces the usb-ccid device that is a ccid bus. Next patches will introduce two card types to use it, a passthru card and an emulated card. [1] http://www.usb.org/developers/devclass_docs/DWG_Smart-Card_CCID_Rev110. Signed-off-by: Alon Levy al...@redhat.com This looks ok to me now - I noticed that the comments in the header file refers to u8/u16/u32 for types, even though the code uses uint{8,16,32}_t, but I don't think that warrants a fix. If the comment goes in, please fix the spelling of my name. Reviewed-by: Jes Sorensen jes.soren...@redhat.com
Re: [Qemu-devel] [PATCH v23 04/11] introduce libcacard/vscard_common.h
On 03/23/11 14:19, Alon Levy wrote: --- Signed-off-by: Alon Levy al...@redhat.com Reviewed-by: Jes Sorensen jes.soren...@redhat.com
Re: [Qemu-devel] [PATCH v23 05/11] ccid: add passthru card device
On 03/23/11 14:19, Alon Levy wrote: The passthru ccid card is a device sitting on the usb-ccid bus and using a chardevice to communicate with a remote device using the VSCard protocol defined in libcacard/vscard_common.h Usage docs available in following patch in docs/ccid.txt Signed-off-by: Alon Levy al...@redhat.com --- [snip] +static void ccid_card_vscard_send_error(PassthruState *s, +uint32_t reader_id, VSCErrorCode code) +{ +VSCMsgError msg = {.code = htonl(code)}; + +ccid_card_vscard_send_msg( +s, VSC_Error, reader_id, (uint8_t *)msg, sizeof(msg)); +} + +static void ccid_card_vscard_send_init(PassthruState *s) +{ +VSCMsgInit msg = { +.version = htonl(VSCARD_VERSION), +.magic = VSCARD_MAGIC, +.capabilities = {0} +}; + If this goes over the wire, don't you need to htonl(VSCARD_MAGIC) here if someone tries to run passthrough from a big endian system to a little endian system, or vice versa? Otherwise it looks ok to me. Cheers, Jes
Re: [Qemu-devel] [PATCH v23 03/11] usb-ccid: add CCID bus
On Mon, Mar 28, 2011 at 02:01:01PM +0200, Jes Sorensen wrote: On 03/23/11 14:19, Alon Levy wrote: A CCID device is a smart card reader. It is a USB device, defined at [1]. This patch introduces the usb-ccid device that is a ccid bus. Next patches will introduce two card types to use it, a passthru card and an emulated card. [1] http://www.usb.org/developers/devclass_docs/DWG_Smart-Card_CCID_Rev110. Signed-off-by: Alon Levy al...@redhat.com This looks ok to me now - I noticed that the comments in the header file refers to u8/u16/u32 for types, even though the code uses uint{8,16,32}_t, but I don't think that warrants a fix. If the comment goes in, please fix the spelling of my name. Gah, I'm terribly sorry, I usually try to be correct on that point. Reviewed-by: Jes Sorensen jes.soren...@redhat.com
Re: [Qemu-devel] [PATCH v23 05/11] ccid: add passthru card device
On Mon, Mar 28, 2011 at 02:08:45PM +0200, Jes Sorensen wrote: On 03/23/11 14:19, Alon Levy wrote: The passthru ccid card is a device sitting on the usb-ccid bus and using a chardevice to communicate with a remote device using the VSCard protocol defined in libcacard/vscard_common.h Usage docs available in following patch in docs/ccid.txt Signed-off-by: Alon Levy al...@redhat.com --- [snip] +static void ccid_card_vscard_send_error(PassthruState *s, +uint32_t reader_id, VSCErrorCode code) +{ +VSCMsgError msg = {.code = htonl(code)}; + +ccid_card_vscard_send_msg( +s, VSC_Error, reader_id, (uint8_t *)msg, sizeof(msg)); +} + +static void ccid_card_vscard_send_init(PassthruState *s) +{ +VSCMsgInit msg = { +.version = htonl(VSCARD_VERSION), +.magic = VSCARD_MAGIC, +.capabilities = {0} +}; + If this goes over the wire, don't you need to htonl(VSCARD_MAGIC) here if someone tries to run passthrough from a big endian system to a little endian system, or vice versa? The VSCARD_MAGIC definition is actually a cast of a string, so it is already in the correct byte order: libcacard/vscard_common.h:#define VSCARD_MAGIC (*(uint32_t *)VSCD) Otherwise it looks ok to me. Cheers, Jes
[Qemu-devel] [Bug 744195] Re: guest cannot boot with 8 VFs or more
a related issues was reported and fiexed before, but the two is not the same problem. http://sourceforge.net/tracker/?func=detailaid=2847560group_id=180599atid=893831 -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/744195 Title: guest cannot boot with 8 VFs or more Status in QEMU: New Bug description: Environment: Host OS (ia32/ia32e/IA64):All Guest OS (ia32/ia32e/IA64): All Guest OS Type (Linux/Windows): Linux kvm.git Commit:a3e2cba1e702cfe15e2ebb20a75b88f02c834d3f qemu-kvm Commit:2c9bb5d4e5ae3b12ad71bd6a0c1b32003661f53a Host Kernel Version:2.6.38+ Hardware:Westmere-EP / Sandy Bridge Bug detailed description: -- Guest cannot boot with 8 VFs or more VFs, but guest can boot with 7 VFs. If you assign hot-plug VFs, after you assigning the 8th VF, you may get the following message in host. kvm_alloc_slot: no free slot available. And the guest aborts. Reproduce steps: 1.try to create a guest with 8 VFs:(the guest cannot boot) qemu-system-x86_64 -smp 2 -m 1024 -net none -hda /root/rhel6.1-beta1.img -device pci-assign,host=04:10.0 -device pci-assign,host=04:10.1 -device pci-assign,host=04:10.2 -device pci-assign,host=04:10.3 -device pci-assign,host=04:10.4 -device pci-assign,host=04:10.5 -device pci-assign,host=04:10.6 -device pci-assign,host=04:10.7 2.If you want to assign 8 hot-plug VFs, you may use following command in qemu monitor after you start a guest. device_add pci-assign,host=00:19.0,id=my_nic
[Qemu-devel] [Bug 744195] [NEW] guest cannot boot with 8 VFs or more
Public bug reported: Environment: Host OS (ia32/ia32e/IA64):All Guest OS (ia32/ia32e/IA64): All Guest OS Type (Linux/Windows): Linux kvm.git Commit:a3e2cba1e702cfe15e2ebb20a75b88f02c834d3f qemu-kvm Commit:2c9bb5d4e5ae3b12ad71bd6a0c1b32003661f53a Host Kernel Version:2.6.38+ Hardware:Westmere-EP / Sandy Bridge Bug detailed description: -- Guest cannot boot with 8 VFs or more VFs, but guest can boot with 7 VFs. If you assign hot-plug VFs, after you assigning the 8th VF, you may get the following message in host. kvm_alloc_slot: no free slot available. And the guest aborts. Reproduce steps: 1.try to create a guest with 8 VFs:(the guest cannot boot) qemu-system-x86_64 -smp 2 -m 1024 -net none -hda /root/rhel6.1-beta1.img -device pci-assign,host=04:10.0 -device pci-assign,host=04:10.1 -device pci-assign,host=04:10.2 -device pci-assign,host=04:10.3 -device pci-assign,host=04:10.4 -device pci-assign,host=04:10.5 -device pci-assign,host=04:10.6 -device pci-assign,host=04:10.7 2.If you want to assign 8 hot-plug VFs, you may use following command in qemu monitor after you start a guest. device_add pci-assign,host=00:19.0,id=my_nic ** Affects: qemu Importance: Undecided Status: New ** Bug watch added: SourceForge.net Tracker #2847560 http://sourceforge.net/support/tracker.php?aid=2847560 -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/744195 Title: guest cannot boot with 8 VFs or more Status in QEMU: New Bug description: Environment: Host OS (ia32/ia32e/IA64):All Guest OS (ia32/ia32e/IA64): All Guest OS Type (Linux/Windows): Linux kvm.git Commit:a3e2cba1e702cfe15e2ebb20a75b88f02c834d3f qemu-kvm Commit:2c9bb5d4e5ae3b12ad71bd6a0c1b32003661f53a Host Kernel Version:2.6.38+ Hardware:Westmere-EP / Sandy Bridge Bug detailed description: -- Guest cannot boot with 8 VFs or more VFs, but guest can boot with 7 VFs. If you assign hot-plug VFs, after you assigning the 8th VF, you may get the following message in host. kvm_alloc_slot: no free slot available. And the guest aborts. Reproduce steps: 1.try to create a guest with 8 VFs:(the guest cannot boot) qemu-system-x86_64 -smp 2 -m 1024 -net none -hda /root/rhel6.1-beta1.img -device pci-assign,host=04:10.0 -device pci-assign,host=04:10.1 -device pci-assign,host=04:10.2 -device pci-assign,host=04:10.3 -device pci-assign,host=04:10.4 -device pci-assign,host=04:10.5 -device pci-assign,host=04:10.6 -device pci-assign,host=04:10.7 2.If you want to assign 8 hot-plug VFs, you may use following command in qemu monitor after you start a guest. device_add pci-assign,host=00:19.0,id=my_nic
Re: [Qemu-devel] [PATCH 4/4] trace: [simple] always enable trace points
Last patch was missing the trace-events hunk. Signed-off-by: Lluís Vilanova vilan...@ac.upc.edu diff --git a/scripts/tracetool b/scripts/tracetool index 7506776..b355ac5 100755 --- a/scripts/tracetool +++ b/scripts/tracetool @@ -221,15 +221,10 @@ EOF linetoc_simple() { -local name state +local name name=$(get_name $1) -if has_property $1 disable; then -state=0 -else -state=1 -fi cat EOF -{.tp_name = $name, .state=$state}, +{.tp_name = $name, .state=1}, EOF simple_event_num=$((simple_event_num + 1)) } diff --git a/trace-events b/trace-events index 90c9e0b..d871ffc 100644 --- a/trace-events +++ b/trace-events @@ -17,9 +17,6 @@ # Example: qemu_malloc(size_t size) size %zu # # The disable keyword will build without the trace event. -# In case of 'simple' trace backend, it will allow the trace event to be -# compiled, but this would be turned off by default. It can be toggled on via -# the monitor. # # The name must be a valid as a C function name. #
Re: [Qemu-devel] [PATCH v23 06/11] libcacard: initial commit
On Mon, Mar 28, 2011 at 02:35:23PM +0200, Jes Sorensen wrote: On 03/23/11 14:19, Alon Levy wrote: From: Robert Relyea rrel...@redhat.com libcacard emulates a Common Access Card (CAC) which is a standard for smartcards. It is used by the emulated ccid card introduced in a following patch. Docs are available in docs/libcacard.txt Signed-off-by: Alon Levy al...@redhat.com A couple of minor nits. diff --git a/Makefile.objs b/Makefile.objs index 744e1d3..f513ffa 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -352,6 +352,11 @@ user-obj-y += qemu-timer-common.o endif endif +## +# smartcard + +libcacard-y = cac.o event.o vcard.o vreader.o vcard_emul_nss.o vcard_emul_type.o card_7816.o + vl.o: QEMU_CFLAGS+=$(GPROF_CFLAGS) vl.o: QEMU_CFLAGS+=$(SDL_CFLAGS) diff --git a/Makefile.target b/Makefile.target index 62b102a..7f163e3 100644 --- a/Makefile.target +++ b/Makefile.target @@ -353,6 +353,8 @@ obj-y += $(addprefix $(HWDIR)/, $(hw-obj-y)) endif # CONFIG_SOFTMMU +obj-y += $(addprefix ../libcacard/, $(libcacard-$(CONFIG_SMARTCARD_NSS))) + obj-y += $(addprefix ../, $(trace-obj-y)) obj-$(CONFIG_GDBSTUB_XML) += gdbstub-xml.o This is a bit backwards, normally we do foobar-$(CONFIG_FOOBAR) = foo.o bar.o and then later obj-y = $(foobar-y) diff --git a/libcacard/cac.c b/libcacard/cac.c new file mode 100644 index 000..7a910d8 --- /dev/null +++ b/libcacard/cac.c @@ -0,0 +1,406 @@ +/* + * implement the applets for the CAC card. + * + * This code is licensed under the GNU LGPL, version 2.1 or later. + * See the COPYING.LIB file in the top-level directory. + * + */ +#include stdlib.h +#include string.h + +#include qemu-common.h stdlib.h and string.h are both included by qemu-common.h diff --git a/libcacard/card_7816.c b/libcacard/card_7816.c new file mode 100644 index 000..4c10cae --- /dev/null +++ b/libcacard/card_7816.c @@ -0,0 +1,764 @@ +/* + * Implement the 7816 portion of the card spec + * + * This code is licensed under the GNU LGPL, version 2.1 or later. + * See the COPYING.LIB file in the top-level directory. + */ + +#include stdlib.h +#include string.h +#include qemu-common.h same here diff --git a/libcacard/event.c b/libcacard/event.c new file mode 100644 index 000..12722cc --- /dev/null +++ b/libcacard/event.c @@ -0,0 +1,108 @@ +/* + * event queue implementation. + * + * This code is licensed under the GNU LGPL, version 2.1 or later. + * See the COPYING.LIB file in the top-level directory. + */ + +#include stdlib.h + +#include qemu-thread.h +#include qemu-common.h again here diff --git a/libcacard/vcard.c b/libcacard/vcard.c new file mode 100644 index 000..d7828a2 --- /dev/null +++ b/libcacard/vcard.c @@ -0,0 +1,341 @@ +/* + * implement the Java card standard. + * + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later. + * See the COPYING.LIB file in the top-level directory. + */ +#include stdlib.h +#include string.h + +#include qemu-common.h and here diff --git a/libcacard/vcard_emul_nss.c b/libcacard/vcard_emul_nss.c new file mode 100644 index 000..d3ab7ea --- /dev/null +++ b/libcacard/vcard_emul_nss.c @@ -0,0 +1,1159 @@ +/* + * This is the actual card emulator. + * + * These functions can be implemented in different ways on different platforms + * using the underlying system primitives. For Linux it uses NSS, though direct + * to PKCS #11, openssl+pkcs11, or even gnu crypto libraries+pkcs #11 could be + * used. On Windows CAPI could be used. + * + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later. + * See the COPYING.LIB file in the top-level directory. + */ + +/* + * system headers + */ +#include stdlib.h +#include string.h + +/* + * NSS headers + */ +#include nss.h +#include pk11pub.h +#include cert.h +#include key.h +#include secmod.h +#include prthread.h +#include secerr.h + +#include qemu-common.h again here prthread.h do you have a check for it in configure? I have to admit I really would prefer QEMU not relying on the NSPR stuff, but I don't know if it can be avoided with the ccid code? No, unless you mean I should rewrite the emulation to not use NSS, I don't know how. Or are you saying NSS can be used without using NSPR? I admited and will repeat that I have not authored this code (that's why I have Robert Relyea as the author of this patch), so I'm not familiar with NSS/NSPR except superficially. diff --git a/libcacard/vreader.c b/libcacard/vreader.c new file mode 100644 index 000..0b67c6c --- /dev/null +++ b/libcacard/vreader.c @@ -0,0 +1,519 @@ +/* + * emulate the reader + * + * This
Re: [Qemu-devel] [PATCH 4/4] trace: [simple] always enable trace points
Last time there was discussion on disabling all tracee points in simple by default, but providing an argument for a list of trace point to enable at startup. Instead, what I've done is enabling all trace points in simple by default. Given that disable will effectively disable all points, including the dtrace backend, my question is what is preferred: * leave it as it is in this patch * leave all points as disabled, so the user has to remove the disable regardless of the backend * simple acts as stderr, all non-disabled points will generate output * remove the disable on all events * for any backend with trace point states leave the state as disabled by default (dynamically enabling them is a backendspecific operation, when supported) * this means implementing the event list argument for the simple backend in order to easily enable a set of events at program startup Lluis -- And it's much the same thing with knowledge, for whenever you learn something new, the whole world becomes that much richer. -- The Princess of Pure Reason, as told by Norton Juster in The Phantom Tollbooth
Re: [Qemu-devel] [PATCH v23 07/11] libcacard: add vscclient
On 03/23/11 14:19, Alon Levy wrote: From: Robert Relyea rrel...@redhat.com client to talk to ccid-card-passthru and use smartcard on client to perform actual operations. --- libcacard/Makefile|7 +- libcacard/vscclient.c | 730 + 2 files changed, 736 insertions(+), 1 deletions(-) create mode 100644 libcacard/vscclient.c [snip] diff --git a/libcacard/vscclient.c b/libcacard/vscclient.c new file mode 100644 index 000..8dde449 --- /dev/null +++ b/libcacard/vscclient.c @@ -0,0 +1,730 @@ +/* + * Tester for VSCARD protocol, client side. + * + * Can be used with ccid-card-passthru. + * + * Copyright (c) 2011 Red Hat. + * Written by Alon Levy. + * + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later. + * See the COPYING.LIB file in the top-level directory. + */ + +#include sys/types.h +#include stdio.h +#include stdlib.h +#include string.h +#include unistd.h + +#include sys/types.h +#include sys/socket.h +#include netdb.h +#include netinet/in.h +#include arpa/inet.h + +#include qemu-thread.h +#include qemu-common.h Lots of unnecessary includes here. +if (verbose 10) { +printf(sending type=%d id=%d, len =%d (0x%x)\n, + type, reader_id, length, length); +} + +mhHeader.type = htonl(type); +mhHeader.reader_id = 0; +mhHeader.length = htonl(length); +rv = write( +sock, +mhHeader, +sizeof(mhHeader) +); Is this excessive multi-lining necessary? It makes it really hard to read. +if (rv 0) { +/* Error */ +printf(write header error\n); +close(sock); +qemu_mutex_unlock(write_lock); +return 16; +} Shouldn't errors go to stderr ? +rv = write( +sock, +msg, +length +); +if (rv 0) { +/* Error */ +printf(write error\n); +close(sock); +qemu_mutex_unlock(write_lock); +return 16; +} Same here. +static void +do_command(void) +{ +char inbuf[255]; +char *string; +VCardEmulError error; +static unsigned int default_reader_id; +unsigned int reader_id; +VReader *reader = NULL; + +reader_id = default_reader_id; +string = fgets(inbuf, sizeof(inbuf), stdin); +if (string != NULL) { +if (strncmp(string, exit, 4) == 0) { +/* remove all the readers */ +VReaderList *list = vreader_get_reader_list(); +VReaderListEntry *reader_entry; +printf(Active Readers:\n); +for (reader_entry = vreader_list_get_first(list); reader_entry; + reader_entry = vreader_list_get_next(reader_entry)) { +VReader *reader = vreader_list_get_reader(reader_entry); +vreader_id_t reader_id; +reader_id = vreader_get_id(reader); +if (reader_id == -1) { +continue; +} +/* be nice and signal card removal first (qemu probably should + * do this itself) */ +if (vreader_card_is_present(reader) == VREADER_OK) { +send_msg( +VSC_CardRemove, +reader_id, +NULL, +0 +); +} +send_msg( +VSC_ReaderRemove, +reader_id, +NULL, +0 +); +} +exit(0); +} else if (strncmp(string, insert, 6) == 0) { +if (string[6] == ' ') { +reader_id = get_id_from_string(string[7], reader_id); +} +reader = vreader_get_reader_by_id(reader_id); +if (reader != NULL) { +error = vcard_emul_force_card_insert(reader); +printf(insert %s, returned %d\n, + reader ? vreader_get_name(reader) + : invalid reader, error); +} else { +printf(no reader by id %d found\n, reader_id); +} +} else if (strncmp(string, remove, 6) == 0) { +if (string[6] == ' ') { +reader_id = get_id_from_string(string[7], reader_id); +} +reader = vreader_get_reader_by_id(reader_id); +if (reader != NULL) { +error = vcard_emul_force_card_remove(reader); +printf(remove %s, returned %d\n, +reader ? vreader_get_name(reader) +: invalid reader, error); +} else { +printf(no reader by id %d found\n, reader_id); +} +} else if (strncmp(string, select, 6) == 0) { +if (string[6] == ' ') { +
Re: [Qemu-devel] [PATCH 27/27] Add SLOF-based partition firmware for pSeries machine, allowing more boot options
On 03/28/2011 11:03 AM, Alexander Graf wrote: I'm also not sure this is too important. Most of our firmware blobs come from svn repos which can't be submoduled. And as long as we don't have a consistent policy about it, we can just as well stick with the README file. We can have a git mirror of the subversion repository hosted on git.qemu.org, and submodule that. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH v23 06/11] libcacard: initial commit
On 03/23/11 14:19, Alon Levy wrote: From: Robert Relyea rrel...@redhat.com libcacard emulates a Common Access Card (CAC) which is a standard for smartcards. It is used by the emulated ccid card introduced in a following patch. Docs are available in docs/libcacard.txt Signed-off-by: Alon Levy al...@redhat.com A couple of minor nits. diff --git a/Makefile.objs b/Makefile.objs index 744e1d3..f513ffa 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -352,6 +352,11 @@ user-obj-y += qemu-timer-common.o endif endif +## +# smartcard + +libcacard-y = cac.o event.o vcard.o vreader.o vcard_emul_nss.o vcard_emul_type.o card_7816.o + vl.o: QEMU_CFLAGS+=$(GPROF_CFLAGS) vl.o: QEMU_CFLAGS+=$(SDL_CFLAGS) diff --git a/Makefile.target b/Makefile.target index 62b102a..7f163e3 100644 --- a/Makefile.target +++ b/Makefile.target @@ -353,6 +353,8 @@ obj-y += $(addprefix $(HWDIR)/, $(hw-obj-y)) endif # CONFIG_SOFTMMU +obj-y += $(addprefix ../libcacard/, $(libcacard-$(CONFIG_SMARTCARD_NSS))) + obj-y += $(addprefix ../, $(trace-obj-y)) obj-$(CONFIG_GDBSTUB_XML) += gdbstub-xml.o This is a bit backwards, normally we do foobar-$(CONFIG_FOOBAR) = foo.o bar.o and then later obj-y = $(foobar-y) diff --git a/libcacard/cac.c b/libcacard/cac.c new file mode 100644 index 000..7a910d8 --- /dev/null +++ b/libcacard/cac.c @@ -0,0 +1,406 @@ +/* + * implement the applets for the CAC card. + * + * This code is licensed under the GNU LGPL, version 2.1 or later. + * See the COPYING.LIB file in the top-level directory. + * + */ +#include stdlib.h +#include string.h + +#include qemu-common.h stdlib.h and string.h are both included by qemu-common.h diff --git a/libcacard/card_7816.c b/libcacard/card_7816.c new file mode 100644 index 000..4c10cae --- /dev/null +++ b/libcacard/card_7816.c @@ -0,0 +1,764 @@ +/* + * Implement the 7816 portion of the card spec + * + * This code is licensed under the GNU LGPL, version 2.1 or later. + * See the COPYING.LIB file in the top-level directory. + */ + +#include stdlib.h +#include string.h +#include qemu-common.h same here diff --git a/libcacard/event.c b/libcacard/event.c new file mode 100644 index 000..12722cc --- /dev/null +++ b/libcacard/event.c @@ -0,0 +1,108 @@ +/* + * event queue implementation. + * + * This code is licensed under the GNU LGPL, version 2.1 or later. + * See the COPYING.LIB file in the top-level directory. + */ + +#include stdlib.h + +#include qemu-thread.h +#include qemu-common.h again here diff --git a/libcacard/vcard.c b/libcacard/vcard.c new file mode 100644 index 000..d7828a2 --- /dev/null +++ b/libcacard/vcard.c @@ -0,0 +1,341 @@ +/* + * implement the Java card standard. + * + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later. + * See the COPYING.LIB file in the top-level directory. + */ +#include stdlib.h +#include string.h + +#include qemu-common.h and here diff --git a/libcacard/vcard_emul_nss.c b/libcacard/vcard_emul_nss.c new file mode 100644 index 000..d3ab7ea --- /dev/null +++ b/libcacard/vcard_emul_nss.c @@ -0,0 +1,1159 @@ +/* + * This is the actual card emulator. + * + * These functions can be implemented in different ways on different platforms + * using the underlying system primitives. For Linux it uses NSS, though direct + * to PKCS #11, openssl+pkcs11, or even gnu crypto libraries+pkcs #11 could be + * used. On Windows CAPI could be used. + * + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later. + * See the COPYING.LIB file in the top-level directory. + */ + +/* + * system headers + */ +#include stdlib.h +#include string.h + +/* + * NSS headers + */ +#include nss.h +#include pk11pub.h +#include cert.h +#include key.h +#include secmod.h +#include prthread.h +#include secerr.h + +#include qemu-common.h again here prthread.h do you have a check for it in configure? I have to admit I really would prefer QEMU not relying on the NSPR stuff, but I don't know if it can be avoided with the ccid code? diff --git a/libcacard/vreader.c b/libcacard/vreader.c new file mode 100644 index 000..0b67c6c --- /dev/null +++ b/libcacard/vreader.c @@ -0,0 +1,519 @@ +/* + * emulate the reader + * + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later. + * See the COPYING.LIB file in the top-level directory. + */ + +/* + * System includes + */ +#include stdlib.h +#include string.h + +#include qemu-thread.h +#include qemu-common.h and a last one Cheers, Jes
Re: [Qemu-devel] [PATCH v23 07/11] libcacard: add vscclient
On Mon, Mar 28, 2011 at 02:43:38PM +0200, Jes Sorensen wrote: On 03/23/11 14:19, Alon Levy wrote: From: Robert Relyea rrel...@redhat.com client to talk to ccid-card-passthru and use smartcard on client to perform actual operations. --- libcacard/Makefile|7 +- libcacard/vscclient.c | 730 + 2 files changed, 736 insertions(+), 1 deletions(-) create mode 100644 libcacard/vscclient.c [snip] diff --git a/libcacard/vscclient.c b/libcacard/vscclient.c new file mode 100644 index 000..8dde449 --- /dev/null +++ b/libcacard/vscclient.c @@ -0,0 +1,730 @@ +/* + * Tester for VSCARD protocol, client side. + * + * Can be used with ccid-card-passthru. + * + * Copyright (c) 2011 Red Hat. + * Written by Alon Levy. + * + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later. + * See the COPYING.LIB file in the top-level directory. + */ + +#include sys/types.h +#include stdio.h +#include stdlib.h +#include string.h +#include unistd.h + +#include sys/types.h +#include sys/socket.h +#include netdb.h +#include netinet/in.h +#include arpa/inet.h + +#include qemu-thread.h +#include qemu-common.h Lots of unnecessary includes here. Will fix. +if (verbose 10) { +printf(sending type=%d id=%d, len =%d (0x%x)\n, + type, reader_id, length, length); +} + +mhHeader.type = htonl(type); +mhHeader.reader_id = 0; +mhHeader.length = htonl(length); +rv = write( +sock, +mhHeader, +sizeof(mhHeader) +); Is this excessive multi-lining necessary? It makes it really hard to read. It's a loop hole in the coding standard? ok, will fix. +if (rv 0) { +/* Error */ +printf(write header error\n); +close(sock); +qemu_mutex_unlock(write_lock); +return 16; +} Shouldn't errors go to stderr ? will fix. +rv = write( +sock, +msg, +length +); +if (rv 0) { +/* Error */ +printf(write error\n); +close(sock); +qemu_mutex_unlock(write_lock); +return 16; +} Same here. will fix. +static void +do_command(void) +{ +char inbuf[255]; +char *string; +VCardEmulError error; +static unsigned int default_reader_id; +unsigned int reader_id; +VReader *reader = NULL; + +reader_id = default_reader_id; +string = fgets(inbuf, sizeof(inbuf), stdin); +if (string != NULL) { +if (strncmp(string, exit, 4) == 0) { +/* remove all the readers */ +VReaderList *list = vreader_get_reader_list(); +VReaderListEntry *reader_entry; +printf(Active Readers:\n); +for (reader_entry = vreader_list_get_first(list); reader_entry; + reader_entry = vreader_list_get_next(reader_entry)) { +VReader *reader = vreader_list_get_reader(reader_entry); +vreader_id_t reader_id; +reader_id = vreader_get_id(reader); +if (reader_id == -1) { +continue; +} +/* be nice and signal card removal first (qemu probably should + * do this itself) */ +if (vreader_card_is_present(reader) == VREADER_OK) { +send_msg( +VSC_CardRemove, +reader_id, +NULL, +0 +); +} +send_msg( +VSC_ReaderRemove, +reader_id, +NULL, +0 +); +} +exit(0); +} else if (strncmp(string, insert, 6) == 0) { +if (string[6] == ' ') { +reader_id = get_id_from_string(string[7], reader_id); +} +reader = vreader_get_reader_by_id(reader_id); +if (reader != NULL) { +error = vcard_emul_force_card_insert(reader); +printf(insert %s, returned %d\n, + reader ? vreader_get_name(reader) + : invalid reader, error); +} else { +printf(no reader by id %d found\n, reader_id); +} +} else if (strncmp(string, remove, 6) == 0) { +if (string[6] == ' ') { +reader_id = get_id_from_string(string[7], reader_id); +} +reader = vreader_get_reader_by_id(reader_id); +if (reader != NULL) { +error = vcard_emul_force_card_remove(reader); +printf(remove %s, returned %d\n, +
Re: [Qemu-devel] [PATCH 27/27] Add SLOF-based partition firmware for pSeries machine, allowing more boot options
On 28.03.2011, at 14:49, Avi Kivity wrote: On 03/28/2011 11:03 AM, Alexander Graf wrote: I'm also not sure this is too important. Most of our firmware blobs come from svn repos which can't be submoduled. And as long as we don't have a consistent policy about it, we can just as well stick with the README file. We can have a git mirror of the subversion repository hosted on git.qemu.org, and submodule that. *shrug* I'm fairly indifferent on that side. But whatever we do, it's out of scope of this patch set :). I personally don't mind the listing in the README file. Alex
Re: [Qemu-devel] [PATCH 27/27] Add SLOF-based partition firmware for pSeries machine, allowing more boot options
On 03/28/2011 02:53 PM, Alexander Graf wrote: On 28.03.2011, at 14:49, Avi Kivity wrote: On 03/28/2011 11:03 AM, Alexander Graf wrote: I'm also not sure this is too important. Most of our firmware blobs come from svn repos which can't be submoduled. And as long as we don't have a consistent policy about it, we can just as well stick with the README file. We can have a git mirror of the subversion repository hosted on git.qemu.org, and submodule that. *shrug* I'm fairly indifferent on that side. But whatever we do, it's out of scope of this patch set :). I personally don't mind the listing in the README file. It depends on how often the code changes. If it changes regularly and qemu is expected to take in newer versions, then we need to record which slof version comes with which qemu version. Submodules do just that. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 27/27] Add SLOF-based partition firmware for pSeries machine, allowing more boot options
On 28.03.2011, at 15:02, Avi Kivity wrote: On 03/28/2011 02:53 PM, Alexander Graf wrote: On 28.03.2011, at 14:49, Avi Kivity wrote: On 03/28/2011 11:03 AM, Alexander Graf wrote: I'm also not sure this is too important. Most of our firmware blobs come from svn repos which can't be submoduled. And as long as we don't have a consistent policy about it, we can just as well stick with the README file. We can have a git mirror of the subversion repository hosted on git.qemu.org, and submodule that. *shrug* I'm fairly indifferent on that side. But whatever we do, it's out of scope of this patch set :). I personally don't mind the listing in the README file. It depends on how often the code changes. If it changes regularly and qemu is expected to take in newer versions, then we need to record which slof version comes with which qemu version. Submodules do just that. A commit id / tag in the README document it pretty well, no? Also, a README file is human readable. Submodules don't really buy anyone anything. Normal users don't care about the source - they use the bundled binaries. Distributers will have to bundle the source code anyways, because the build process is always offline. You can't always build everything either, as there are targets in qemu that people don't have cross-compilers for. It feels like the submodule idea is more of a feature that's cool to play with rather than of benefit for anyone. Alex
Re: [Qemu-devel] [PATCH 27/27] Add SLOF-based partition firmware for pSeries machine, allowing more boot options
On 03/27/2011 08:19 PM, David Gibson wrote: We should pull in SLOF via a git submodule. That ensures we ship the source code along with the binary. Um, ok. Do I need to do anything about this? We should introduce SLOF as one patch that adds the git submodule and the binary. The best way to do this is for me to pull as binary diffs on the list kind of suck. But before we do the git submodule, I need to mirror SLOF on qemu.org such that everything can be fetched from one place. Ping me later today when you get online and I'll explain how to do the git submodule part. Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH 27/27] Add SLOF-based partition firmware for pSeries machine, allowing more boot options
On 03/28/2011 04:03 AM, Alexander Graf wrote: Um, ok. Do I need to do anything about this? I'm also not sure this is too important. It's GPL compliance so yes, it's very important. Most of our firmware blobs come from svn repos which can't be submoduled. The only firmware blob we're not currently including as a git submodule is OpenBIOS. I believe the main reason is that different boards use different commits so a single submodule is a bit challenge. We probably ought to figure something out here though for the next release. Can anyone comment a bit more about OpenBIOS? BTW, OpenBIOS is already actively mirrored on git.qemu.org so all that's needed is a patch that does a git submodule add with the appropriate commit. And as long as we don't have a consistent policy about it, we can just as well stick with the README file. We do have a consistent policy :-) We're just not enforcing it as tightly as we should. Any binary we ship in the release tgz's should also have corresponding source in a submodule. Regards, Anthony Liguori Alex
Re: [Qemu-devel] [PATCH 27/27] Add SLOF-based partition firmware for pSeries machine, allowing more boot options
On 03/28/2011 08:08 AM, Alexander Graf wrote: It depends on how often the code changes. If it changes regularly and qemu is expected to take in newer versions, then we need to record which slof version comes with which qemu version. Submodules do just that. A commit id / tag in the README document it pretty well, no? Also, a README file is human readable. Submodules don't really buy anyone anything. When I do a release, I do the equivalent of: git submodule update --init rm -rf roms/*/.git rm -rf .git Having the information is submodules makes this process automated and repeatable. The main motivation is that we need to ship source for any binary we include in our tarball. Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH v23 08/11] libcacard: add passthru
On 03/23/11 14:19, Alon Levy wrote: diff --git a/libcacard/passthru.c b/libcacard/passthru.c new file mode 100644 index 000..d78e2db --- /dev/null +++ b/libcacard/passthru.c @@ -0,0 +1,609 @@ +/* + * implement the applets for the CAC card. + * + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later. + * See the COPYING.LIB file in the top-level directory. + */ +#ifdef USE_PASSTHRU +#include stdlib.h +#include string.h + +#include pcsclite.h + +#include qemu-thread.h + +#include vcard.h +#include vcard_emul.h +#include card_7816.h +#include vreader.h +#include vcard_emul.h +#include passthru.h + +/* + * Passthru applet private data + */ +struct VCardAppletPrivateStruct { +char *reader_name; +/* pcsc-lite parameters */ +SCARDHANDLE hCard; +uint32_t hProtocol; +SCARD_IO_REQUEST *send_io; +unsigned char atr[MAX_ATR_SIZE]; +int atr_len; +}; + +static SCARDCONTEXT global_context; + +#define MAX_RESPONSE_LENGTH 261 /*65537 */ +/* + * handle all the APDU's that are common to all CAC applets + */ +static VCardStatus +passthru_process_apdu(VCard *card, VCardAPDU *apdu, VCardResponse **response) +{ +LONG rv; Where does this odd LONG type come from? I think Windows uses LONG so having your own type would conflict with that, and of course we don't really want Windows types directly in QEMU either, so I am curious? Otherwise the code looks fine. Cheers, Jes
Re: [Qemu-devel] [PATCH v23 09/11] libcacard: add docs
On 03/23/11 14:19, Alon Levy wrote: From: Robert Relyea rrel...@redhat.com --- docs/libcacard.txt | 483 1 files changed, 483 insertions(+), 0 deletions(-) create mode 100644 docs/libcacard.txt Didn't review the content in detail, but the way the documentation is put is fine :) Acked-by: Jes Sorensen jes.soren...@redhat.com
Re: [Qemu-devel] [PATCH v23 10/11] ccid: add ccid-card-emulated device
On 03/23/11 14:19, Alon Levy wrote: This devices uses libcacard (internal) to emulate a smartcard conforming to the CAC standard. It attaches to the usb-ccid bus. Usage instructions (example command lines) are in the following patch in docs/ccid.txt. It uses libcacard which uses nss, so it can work with both hw cards and certificates (files). Signed-off-by: Alon Levy al...@redhat.com Acked-by: Jes Sorensen jes.soren...@redhat.com
[Qemu-devel] Re: [PATCH v5 0/4] piix_pci: optimize irq data path
On Mon, Mar 28, 2011 at 01:34:02PM +0200, Michael S. Tsirkin wrote: On Mon, Mar 28, 2011 at 08:19:56PM +0900, Isaku Yamahata wrote: On Sun, Mar 27, 2011 at 04:56:29PM +0200, Michael S. Tsirkin wrote: On Wed, Mar 23, 2011 at 11:17:19AM +0900, Isaku Yamahata wrote: v4 has minor typo. I sent it too early. Here's fixed one. v3 - v4 Main changes are - use pirq, pci_intx instead of irq_num in piix_pci.c - patch 4/4 cleans the code a bit With this applied e1000 fails to work for me. Command line: qemu-system-x86_64 -enable-kvm -m 1G -drive if=virtio,file=/home/mst/rhel6.qcow2 -netdev user,id=bar -net nic,netdev=bar,model=e1000,macaddr=52:54:00:12:34:57 -redir tcp:8022::22 -net nic,model=e1000,netdev=foo,macaddr=52:54:00:12:34:56 -netdev tap,id=foo,ifname=msttap0,script=/home/mst/ifup,downscript=no -nographic Could you try that please? Does the following patch help? Seems to help, but I have to ask - how did you test v5? I booted a guest to see login prompt. My setting happened to use only PIRQ A and B. I found this fix by code reviewing. If so, I'll prepare v6. diff --git a/hw/piix_pci.c b/hw/piix_pci.c index c019793..5f0d92f 100644 --- a/hw/piix_pci.c +++ b/hw/piix_pci.c @@ -277,7 +277,8 @@ static void piix3_set_irq_pic(PIIX3State *piix3, int pic_irq) { qemu_set_irq(piix3-pic[pic_irq], !!(piix3-pic_levels -((PIIX_NUM_PIRQS - 1) (pic_irq * PIIX_NUM_PIRQS; +(((1UL PIIX_NUM_PIRQS) - 1) + (pic_irq * PIIX_NUM_PIRQS; I think we should just make it ~0ULL (pic_irq * PIIX_NUM_PIRQS). Didn't try this though. To get if pic_irq is raised/lowered, only 4bits are checked. bit 63 7 4 3 0 pic_irq | 15| 14|... |... | 1| 0| PIRQ|DCBA|DCBA|DCBA|... |DCBA|DCBA| = (1UL PIIX_NUM_PIRQS) - 1 (pic_irq * PIIX_NUM_PIRQS) thanks, } static void piix3_set_irq_level(PIIX3State *piix3, int pirq, int level) -- yamahata -- yamahata
Re: [Qemu-devel] [PATCH v23 11/11] ccid: add docs
On 03/23/11 14:19, Alon Levy wrote: Add documentation for the usb-ccid device and accompanying two card devices, ccid-card-emulated and ccid-card-passthru. Signed-off-by: Alon Levy al...@redhat.com --- docs/ccid.txt | 135 + 1 files changed, 135 insertions(+), 0 deletions(-) create mode 100644 docs/ccid.txt Acked-by: Jes Sorensen jes.soren...@redhat.com
[Qemu-devel] Re: [PATCH]hw/xen_disk: ioreq not finished on error
On Sun, 27 Mar 2011, Feiran Zheng wrote: Bug fix: routines 'ioreq_runio_qemu_sync' and 'ioreq_runio_qemu_aio' won't call 'ioreq_unmap' or 'ioreq_finish' on errors, leaving ioreq in the blkdev-inflight list and a leak. Signed-off-by: Feiran Zheng famc...@gmail.com --- hw/xen_disk.c | 22 +- 1 files changed, 17 insertions(+), 5 deletions(-) diff --git a/hw/xen_disk.c b/hw/xen_disk.c index 445bf03..7940fab 100644 --- a/hw/xen_disk.c +++ b/hw/xen_disk.c @@ -309,8 +309,10 @@ static int ioreq_runio_qemu_sync(struct ioreq *ioreq) int i, rc, len = 0; off_t pos; -if (ioreq-req.nr_segments ioreq_map(ioreq) == -1) - goto err; +if (ioreq-req.nr_segments) { + if (ioreq_map(ioreq) == -1) + goto err_no_map; +} if (ioreq-presync) bdrv_flush(blkdev-bs); this change is just to make the code clearer and easier to read, right? @@ -364,6 +366,9 @@ static int ioreq_runio_qemu_sync(struct ioreq *ioreq) return 0; err: +ioreq_unmap(ioreq); +err_no_map: +ioreq_finish(ioreq); ioreq-status = BLKIF_RSP_ERROR; return -1; } @@ -392,8 +397,10 @@ static int ioreq_runio_qemu_aio(struct ioreq *ioreq) { struct XenBlkDev *blkdev = ioreq-blkdev; -if (ioreq-req.nr_segments ioreq_map(ioreq) == -1) - goto err; +if (ioreq-req.nr_segments) { + if (ioreq_map(ioreq) == -1) + goto err_no_map; +} ioreq-aio_inflight++; if (ioreq-presync) @@ -425,9 +432,14 @@ static int ioreq_runio_qemu_aio(struct ioreq *ioreq) qemu_aio_complete(ioreq, 0); return 0; + +err_no_map: +ioreq_finish(ioreq); +ioreq-status = BLKIF_RSP_ERROR; +return -1; why aren't you calling ioreq_unmap before ioreq_finish, like in the previous case?
[Qemu-devel] [PATCH v2 2/5] hw: Add maximum RAM specifications for ARM devboard models
Specify the maximum memory permitted for the various ARM devboard models (integratorcp, realview-eb, realview-eb-mpcore, realview-pb-a8, realview-pbx-a9, versatilepb, versatileab). This means we now handle attempts to specify too much RAM gracefully rather than causing the guest to crash in an obscure fashion. Signed-off-by: Peter Maydell peter.mayd...@linaro.org --- hw/integratorcp.c |1 + hw/realview.c | 11 +++ hw/versatilepb.c |5 + 3 files changed, 17 insertions(+), 0 deletions(-) diff --git a/hw/integratorcp.c b/hw/integratorcp.c index b049940..ccc44db 100644 --- a/hw/integratorcp.c +++ b/hw/integratorcp.c @@ -516,6 +516,7 @@ static QEMUMachine integratorcp_machine = { .name = integratorcp, .desc = ARM Integrator/CP (ARM926EJ-S), .init = integratorcp_init, +.max_ram = 256 * 1024 * 1024, .is_default = 1, }; diff --git a/hw/realview.c b/hw/realview.c index a67861e..a158ade 100644 --- a/hw/realview.c +++ b/hw/realview.c @@ -432,6 +432,7 @@ static QEMUMachine realview_eb_machine = { .desc = ARM RealView Emulation Baseboard (ARM926EJ-S), .init = realview_eb_init, .use_scsi = 1, +.max_ram = 256 * 1024 * 1024, }; static QEMUMachine realview_eb_mpcore_machine = { @@ -440,12 +441,18 @@ static QEMUMachine realview_eb_mpcore_machine = { .init = realview_eb_mpcore_init, .use_scsi = 1, .max_cpus = 4, +.max_ram = 256 * 1024 * 1024, }; static QEMUMachine realview_pb_a8_machine = { .name = realview-pb-a8, .desc = ARM RealView Platform Baseboard for Cortex-A8, .init = realview_pb_a8_init, +/* The PB-A8 has 512MB; qemu also supports an extra PBX-A9-like + * 512MB although strictly speaking that area of the address + * space is 'reserved' on the PB-A8. + */ +.max_ram = 1024 * 1024 * 1024, }; static QEMUMachine realview_pbx_a9_machine = { @@ -454,6 +461,10 @@ static QEMUMachine realview_pbx_a9_machine = { .init = realview_pbx_a9_init, .use_scsi = 1, .max_cpus = 4, +/* Realview PBX has 1GB of RAM (512MB on the motherboard + * and another 512MB on the daughterboard) + */ +.max_ram = 1024 * 1024 * 1024, }; static void realview_machine_init(void) diff --git a/hw/versatilepb.c b/hw/versatilepb.c index 9f1bfcf..aeddd28 100644 --- a/hw/versatilepb.c +++ b/hw/versatilepb.c @@ -329,6 +329,10 @@ static QEMUMachine versatilepb_machine = { .desc = ARM Versatile/PB (ARM926EJ-S), .init = vpb_init, .use_scsi = 1, +/* Hardware allows for up to 512MB expansion memory in two + * non-contiguous sections, but we only support up to 256MB + */ +.max_ram = 256 * 1024 * 1024, }; static QEMUMachine versatileab_machine = { @@ -336,6 +340,7 @@ static QEMUMachine versatileab_machine = { .desc = ARM Versatile/AB (ARM926EJ-S), .init = vab_init, .use_scsi = 1, +.max_ram = 256 * 1024 * 1024, }; static void versatile_machine_init(void) -- 1.7.1
[Qemu-devel] [PATCH v2 1/5] Allow boards to specify maximum RAM size
Allow boards to specify their maximum RAM size in the QEMUMachine struct. This allows us to provide a useful diagnostic if the user tries to specify a RAM size that the board cannot support. Signed-off-by: Peter Maydell peter.mayd...@linaro.org --- hw/boards.h |1 + vl.c| 16 +++- 2 files changed, 16 insertions(+), 1 deletions(-) diff --git a/hw/boards.h b/hw/boards.h index 6f0f0d7..5f41fce 100644 --- a/hw/boards.h +++ b/hw/boards.h @@ -19,6 +19,7 @@ typedef struct QEMUMachine { QEMUMachineInitFunc *init; int use_scsi; int max_cpus; +target_phys_addr_t max_ram; unsigned int no_serial:1, no_parallel:1, use_virtcon:1, diff --git a/vl.c b/vl.c index 192a240..69cb29b 100644 --- a/vl.c +++ b/vl.c @@ -166,6 +166,9 @@ int main(int argc, char **argv) //#define DEBUG_NET //#define DEBUG_SLIRP +/* Note that this default RAM size is capped to any maximum + * RAM size specified in the board's QEMUMachine struct. + */ #define DEFAULT_RAM_SIZE 128 #define MAX_VIRTIO_CONSOLES 1 @@ -3046,8 +3049,19 @@ int main(int argc, char **argv, char **envp) exit(1); /* init the memory */ -if (ram_size == 0) +if (ram_size == 0) { ram_size = DEFAULT_RAM_SIZE * 1024 * 1024; +if (machine-max_ram) { +ram_size = MIN(ram_size, machine-max_ram); +} +} else if (machine-max_ram ram_size machine-max_ram) { +/* Since you can only specify ram_size on the command line in MB it's + * OK to round down when printing the machine's maximum. + */ +fprintf(stderr, qemu: maximum permitted RAM size for '%s' is %ldM\n, +machine-name, (ram_addr_t)(machine-max_ram / (1024 * 1024))); +exit(1); +} /* init the dynamic translator */ cpu_exec_init_all(tb_size * 1024 * 1024); -- 1.7.1
[Qemu-devel] [PATCH v2 3/5] hw/sun4m: Move QEMUMachine structs into sun4*_hwdef structs
Combine the per-machine QEMUMachine struct into the per-machine sun4*_hwdef struct. This requires some moving around of init functions to avoid forward references. We also have to move the 'const' attribute from the whole sun4*_hwdef[] array to the individual fields of the structure, because QEMUMachine is not const. The motivation is to allow the init functions to get at the QEMUMachine struct for the board, so we can use its max_ram field rather than having a max_mem field in the sun4*_hwdef struct. Signed-off-by: Peter Maydell peter.mayd...@linaro.org --- hw/sun4m.c | 596 ++-- 1 files changed, 297 insertions(+), 299 deletions(-) diff --git a/hw/sun4m.c b/hw/sun4m.c index df3aa32..db90fbe 100644 --- a/hw/sun4m.c +++ b/hw/sun4m.c @@ -87,51 +87,55 @@ #define ESCC_CLOCK 4915200 struct sun4m_hwdef { -target_phys_addr_t iommu_base, iommu_pad_base, iommu_pad_len, slavio_base; -target_phys_addr_t intctl_base, counter_base, nvram_base, ms_kb_base; -target_phys_addr_t serial_base, fd_base; -target_phys_addr_t afx_base, idreg_base, dma_base, esp_base, le_base; -target_phys_addr_t tcx_base, cs_base, apc_base, aux1_base, aux2_base; -target_phys_addr_t bpp_base, dbri_base, sx_base; -struct { +QEMUMachine machine; +const target_phys_addr_t iommu_base, iommu_pad_base, iommu_pad_len; +const target_phys_addr_t slavio_base; +const target_phys_addr_t intctl_base, counter_base, nvram_base, ms_kb_base; +const target_phys_addr_t serial_base, fd_base; +const target_phys_addr_t afx_base, idreg_base, dma_base, esp_base, le_base; +const target_phys_addr_t tcx_base, cs_base, apc_base, aux1_base, aux2_base; +const target_phys_addr_t bpp_base, dbri_base, sx_base; +const struct { target_phys_addr_t reg_base, vram_base; } vsimm[MAX_VSIMMS]; -target_phys_addr_t ecc_base; -uint32_t ecc_version; -uint8_t nvram_machine_id; -uint16_t machine_id; -uint32_t iommu_version; -uint64_t max_mem; +const target_phys_addr_t ecc_base; +const uint32_t ecc_version; +const uint8_t nvram_machine_id; +const uint16_t machine_id; +const uint32_t iommu_version; +const uint64_t max_mem; const char * const default_cpu_model; }; #define MAX_IOUNITS 5 struct sun4d_hwdef { -target_phys_addr_t iounit_bases[MAX_IOUNITS], slavio_base; -target_phys_addr_t counter_base, nvram_base, ms_kb_base; -target_phys_addr_t serial_base; -target_phys_addr_t espdma_base, esp_base; -target_phys_addr_t ledma_base, le_base; -target_phys_addr_t tcx_base; -target_phys_addr_t sbi_base; -uint8_t nvram_machine_id; -uint16_t machine_id; -uint32_t iounit_version; -uint64_t max_mem; +QEMUMachine machine; +const target_phys_addr_t iounit_bases[MAX_IOUNITS], slavio_base; +const target_phys_addr_t counter_base, nvram_base, ms_kb_base; +const target_phys_addr_t serial_base; +const target_phys_addr_t espdma_base, esp_base; +const target_phys_addr_t ledma_base, le_base; +const target_phys_addr_t tcx_base; +const target_phys_addr_t sbi_base; +const uint8_t nvram_machine_id; +const uint16_t machine_id; +const uint32_t iounit_version; +const uint64_t max_mem; const char * const default_cpu_model; }; struct sun4c_hwdef { -target_phys_addr_t iommu_base, slavio_base; -target_phys_addr_t intctl_base, counter_base, nvram_base, ms_kb_base; -target_phys_addr_t serial_base, fd_base; -target_phys_addr_t idreg_base, dma_base, esp_base, le_base; -target_phys_addr_t tcx_base, aux1_base; -uint8_t nvram_machine_id; -uint16_t machine_id; -uint32_t iommu_version; -uint64_t max_mem; +QEMUMachine machine; +const target_phys_addr_t iommu_base, slavio_base; +const target_phys_addr_t intctl_base, counter_base, nvram_base, ms_kb_base; +const target_phys_addr_t serial_base, fd_base; +const target_phys_addr_t idreg_base, dma_base, esp_base, le_base; +const target_phys_addr_t tcx_base, aux1_base; +const uint8_t nvram_machine_id; +const uint16_t machine_id; +const uint32_t iommu_version; +const uint64_t max_mem; const char * const default_cpu_model; }; @@ -1006,9 +1010,109 @@ enum { ss2000_id, }; -static const struct sun4m_hwdef sun4m_hwdefs[] = { +static struct sun4m_hwdef sun4m_hwdefs[]; + +/* SPARCstation 5 hardware initialisation */ +static void ss5_init(ram_addr_t RAM_size, + const char *boot_device, + const char *kernel_filename, const char *kernel_cmdline, + const char *initrd_filename, const char *cpu_model) +{ +sun4m_hw_init(sun4m_hwdefs[0], RAM_size, boot_device, kernel_filename, + kernel_cmdline, initrd_filename, cpu_model); +} + +/* SPARCstation 10 hardware initialisation */ +static void ss10_init(ram_addr_t RAM_size, +
Re: [Qemu-devel] [PATCH v23 05/11] ccid: add passthru card device
On 03/28/11 14:14, Alon Levy wrote: +static void ccid_card_vscard_send_init(PassthruState *s) +{ +VSCMsgInit msg = { +.version = htonl(VSCARD_VERSION), +.magic = VSCARD_MAGIC, +.capabilities = {0} +}; + If this goes over the wire, don't you need to htonl(VSCARD_MAGIC) here if someone tries to run passthrough from a big endian system to a little endian system, or vice versa? The VSCARD_MAGIC definition is actually a cast of a string, so it is already in the correct byte order: libcacard/vscard_common.h:#define VSCARD_MAGIC (*(uint32_t *)VSCD) Ah, I was staring at it for a bit and I wasn't quite sure, which is why I asked the question. Might be worth adding a comment about it. Otherwise it is good then, so Acked-by: Jes Sorensen jes.soren...@redhat.com
[Qemu-devel] [PATCH v2 5/5] hw/sun4m: Use a macro to hide the repetitive board init functions
Tidy up the repetitive board init functions (which are all the same apart from which hwdef struct they pass in). This also lets us add an assertion that the hwdef points to the init function which uses that hwdef, rather than some other one. Signed-off-by: Peter Maydell peter.mayd...@linaro.org --- hw/sun4m.c | 138 ++-- 1 files changed, 22 insertions(+), 116 deletions(-) diff --git a/hw/sun4m.c b/hw/sun4m.c index 47692dd..75155e2 100644 --- a/hw/sun4m.c +++ b/hw/sun4m.c @@ -1002,96 +1002,27 @@ enum { static struct sun4m_hwdef sun4m_hwdefs[]; -/* SPARCstation 5 hardware initialisation */ -static void ss5_init(ram_addr_t RAM_size, - const char *boot_device, - const char *kernel_filename, const char *kernel_cmdline, - const char *initrd_filename, const char *cpu_model) -{ -sun4m_hw_init(sun4m_hwdefs[0], RAM_size, boot_device, kernel_filename, - kernel_cmdline, initrd_filename, cpu_model); -} - -/* SPARCstation 10 hardware initialisation */ -static void ss10_init(ram_addr_t RAM_size, - const char *boot_device, - const char *kernel_filename, const char *kernel_cmdline, - const char *initrd_filename, const char *cpu_model) -{ -sun4m_hw_init(sun4m_hwdefs[1], RAM_size, boot_device, kernel_filename, - kernel_cmdline, initrd_filename, cpu_model); -} - -/* SPARCserver 600MP hardware initialisation */ -static void ss600mp_init(ram_addr_t RAM_size, - const char *boot_device, - const char *kernel_filename, - const char *kernel_cmdline, - const char *initrd_filename, const char *cpu_model) -{ -sun4m_hw_init(sun4m_hwdefs[2], RAM_size, boot_device, kernel_filename, - kernel_cmdline, initrd_filename, cpu_model); -} - -/* SPARCstation 20 hardware initialisation */ -static void ss20_init(ram_addr_t RAM_size, - const char *boot_device, - const char *kernel_filename, const char *kernel_cmdline, - const char *initrd_filename, const char *cpu_model) -{ -sun4m_hw_init(sun4m_hwdefs[3], RAM_size, boot_device, kernel_filename, - kernel_cmdline, initrd_filename, cpu_model); -} - -/* SPARCstation Voyager hardware initialisation */ -static void vger_init(ram_addr_t RAM_size, - const char *boot_device, - const char *kernel_filename, const char *kernel_cmdline, - const char *initrd_filename, const char *cpu_model) -{ -sun4m_hw_init(sun4m_hwdefs[4], RAM_size, boot_device, kernel_filename, - kernel_cmdline, initrd_filename, cpu_model); +#define SUN4_INITFN(NAME, SUBARCH, OFFSET) \ +static void NAME##_init(ram_addr_t RAM_size, \ + const char *boot_device, \ + const char *kernel_filename, const char *kernel_cmdline, \ + const char *initrd_filename, const char *cpu_model) \ +{ \ +assert(SUBARCH##_hwdefs[OFFSET].machine.init == NAME##_init); \ +SUBARCH##_hw_init(SUBARCH##_hwdefs[OFFSET], RAM_size, boot_device, \ + kernel_filename, kernel_cmdline, initrd_filename, \ + cpu_model); \ } -/* SPARCstation LX hardware initialisation */ -static void ss_lx_init(ram_addr_t RAM_size, - const char *boot_device, - const char *kernel_filename, const char *kernel_cmdline, - const char *initrd_filename, const char *cpu_model) -{ -sun4m_hw_init(sun4m_hwdefs[5], RAM_size, boot_device, kernel_filename, - kernel_cmdline, initrd_filename, cpu_model); -} - -/* SPARCstation 4 hardware initialisation */ -static void ss4_init(ram_addr_t RAM_size, - const char *boot_device, - const char *kernel_filename, const char *kernel_cmdline, - const char *initrd_filename, const char *cpu_model) -{ -sun4m_hw_init(sun4m_hwdefs[6], RAM_size, boot_device, kernel_filename, - kernel_cmdline, initrd_filename, cpu_model); -} - -/* SPARCClassic hardware initialisation */ -static void scls_init(ram_addr_t RAM_size, - const char *boot_device, - const char *kernel_filename, const char *kernel_cmdline, - const char *initrd_filename, const char *cpu_model) -{ -sun4m_hw_init(sun4m_hwdefs[7], RAM_size, boot_device, kernel_filename, - kernel_cmdline, initrd_filename, cpu_model); -} - -/* SPARCbook hardware initialisation */ -static void sbook_init(ram_addr_t RAM_size, - const char *boot_device, - const char *kernel_filename, const char *kernel_cmdline, - const char
Re: [Qemu-devel] [PATCH v23 06/11] libcacard: initial commit
On 03/28/11 14:42, Alon Levy wrote: On Mon, Mar 28, 2011 at 02:35:23PM +0200, Jes Sorensen wrote: +/* + * NSS headers + */ +#include nss.h +#include pk11pub.h +#include cert.h +#include key.h +#include secmod.h +#include prthread.h +#include secerr.h + +#include qemu-common.h again here prthread.h do you have a check for it in configure? I have to admit I really would prefer QEMU not relying on the NSPR stuff, but I don't know if it can be avoided with the ccid code? No, unless you mean I should rewrite the emulation to not use NSS, I don't know how. Or are you saying NSS can be used without using NSPR? I admited and will repeat that I have not authored this code (that's why I have Robert Relyea as the author of this patch), so I'm not familiar with NSS/NSPR except superficially. I don't know enough about NSS to say so, so just leave it in. However, please check that the build doesn't break if one doesn't have the nspr headers installed. diff --git a/libcacard/vreader.c b/libcacard/vreader.c new file mode 100644 index 000..0b67c6c --- /dev/null +++ b/libcacard/vreader.c @@ -0,0 +1,519 @@ +/* + * emulate the reader + * + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later. + * See the COPYING.LIB file in the top-level directory. + */ + +/* + * System includes + */ +#include stdlib.h +#include string.h + +#include qemu-thread.h +#include qemu-common.h and a last one Are these a problem enough that you will without an ack? with respect to your previous acks do you want me to only send this patch again, and Anthony should merge the acked patches? It would be preferred to have it fixed before the patch goes in, it should be a quick fix. I'll be happy to ack it with that change. Cheers, Jes
[Qemu-devel] [PATCH v2 4/5] hw/sun4m: Use the QEMUMachine max_ram to implement memory limit
Use the max_ram field in QEMUMachine to indicate maximum memory, rather than a field in the sun4*_hwdef structure. This allows us to use the vl.c check on RAM specifications rather than having to code our own. Signed-off-by: Peter Maydell peter.mayd...@linaro.org --- hw/sun4m.c | 42 -- 1 files changed, 16 insertions(+), 26 deletions(-) diff --git a/hw/sun4m.c b/hw/sun4m.c index db90fbe..47692dd 100644 --- a/hw/sun4m.c +++ b/hw/sun4m.c @@ -103,7 +103,6 @@ struct sun4m_hwdef { const uint8_t nvram_machine_id; const uint16_t machine_id; const uint32_t iommu_version; -const uint64_t max_mem; const char * const default_cpu_model; }; @@ -121,7 +120,6 @@ struct sun4d_hwdef { const uint8_t nvram_machine_id; const uint16_t machine_id; const uint32_t iounit_version; -const uint64_t max_mem; const char * const default_cpu_model; }; @@ -135,7 +133,6 @@ struct sun4c_hwdef { const uint8_t nvram_machine_id; const uint16_t machine_id; const uint32_t iommu_version; -const uint64_t max_mem; const char * const default_cpu_model; }; @@ -747,13 +744,6 @@ static void ram_init(target_phys_addr_t addr, ram_addr_t RAM_size, RamDevice *d; /* allocate RAM */ -if ((uint64_t)RAM_size max_mem) { -fprintf(stderr, -qemu: Too much memory for this machine: %d, maximum %d\n, -(unsigned int)(RAM_size / (1024 * 1024)), -(unsigned int)(max_mem / (1024 * 1024))); -exit(1); -} dev = qdev_create(NULL, memory); s = sysbus_from_qdev(dev); @@ -834,10 +824,10 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef, ram_addr_t RAM_size, /* set up devices */ -ram_init(0, RAM_size, hwdef-max_mem); +ram_init(0, RAM_size, hwdef-machine.max_ram); /* models without ECC don't trap when missing ram is accessed */ if (!hwdef-ecc_base) { -empty_slot_init(RAM_size, hwdef-max_mem - RAM_size); +empty_slot_init(RAM_size, hwdef-machine.max_ram - RAM_size); } prom_init(hwdef-slavio_base, bios_name); @@ -,6 +1101,7 @@ static struct sun4m_hwdef sun4m_hwdefs[] = { .desc = Sun4m platform, SPARCstation 5, .init = ss5_init, .use_scsi = 1, +.max_ram = 0x1000, .is_default = 1, }, .iommu_base = 0x1000, @@ -1136,7 +1127,6 @@ static struct sun4m_hwdef sun4m_hwdefs[] = { .nvram_machine_id = 0x80, .machine_id = ss5_id, .iommu_version = 0x0500, -.max_mem = 0x1000, .default_cpu_model = Fujitsu MB86904, }, /* SS-10 */ @@ -1147,6 +1137,7 @@ static struct sun4m_hwdef sun4m_hwdefs[] = { .init = ss10_init, .use_scsi = 1, .max_cpus = 4, +.max_ram = 0xfULL, }, .iommu_base = 0xfe000ULL, .tcx_base = 0xe2000ULL, @@ -1169,7 +1160,6 @@ static struct sun4m_hwdef sun4m_hwdefs[] = { .nvram_machine_id = 0x72, .machine_id = ss10_id, .iommu_version = 0x0300, -.max_mem = 0xfULL, .default_cpu_model = TI SuperSparc II, }, /* SS-600MP */ @@ -1180,6 +1170,7 @@ static struct sun4m_hwdef sun4m_hwdefs[] = { .init = ss600mp_init, .use_scsi = 1, .max_cpus = 4, +.max_ram = 0xfULL, }, .iommu_base = 0xfe000ULL, .tcx_base = 0xe2000ULL, @@ -1200,7 +1191,6 @@ static struct sun4m_hwdef sun4m_hwdefs[] = { .nvram_machine_id = 0x71, .machine_id = ss600mp_id, .iommu_version = 0x0100, -.max_mem = 0xfULL, .default_cpu_model = TI SuperSparc II, }, /* SS-20 */ @@ -1211,6 +1201,7 @@ static struct sun4m_hwdef sun4m_hwdefs[] = { .init = ss20_init, .use_scsi = 1, .max_cpus = 4, +.max_ram = 0xfULL, }, .iommu_base = 0xfe000ULL, .tcx_base = 0xe2000ULL, @@ -1249,7 +1240,6 @@ static struct sun4m_hwdef sun4m_hwdefs[] = { .nvram_machine_id = 0x72, .machine_id = ss20_id, .iommu_version = 0x1300, -.max_mem = 0xfULL, .default_cpu_model = TI SuperSparc II, }, /* Voyager */ @@ -1259,6 +1249,7 @@ static struct sun4m_hwdef sun4m_hwdefs[] = { .desc = Sun4m platform, SPARCstation Voyager, .init = vger_init, .use_scsi = 1, +.max_ram = 0x1000, }, .iommu_base = 0x1000, .tcx_base = 0x5000, @@ -1279,7 +1270,6 @@ static struct sun4m_hwdef sun4m_hwdefs[] = { .nvram_machine_id = 0x80, .machine_id = vger_id, .iommu_version = 0x0500, -.max_mem = 0x1000, .default_cpu_model = Fujitsu
Re: [Qemu-devel] [PATCH v23 07/11] libcacard: add vscclient
On 03/28/11 14:51, Alon Levy wrote: as mentioned in an earlier series, I think VSCARD_MAGIC needs to be network converted too, to handle difference in endianness between hosts. Same answer as to your previous comment. Yeah I saw your answer - I am happy with the code in general, so if you fix the minor nits that is good with me. Cheers, Jes
[Qemu-devel] Re: [PATCH]hw/xen_disk: ioreq not finished on error
On Mon, Mar 28, 2011 at 9:42 PM, Stefano Stabellini stefano.stabell...@eu.citrix.com wrote: On Sun, 27 Mar 2011, Feiran Zheng wrote: Bug fix: routines 'ioreq_runio_qemu_sync' and 'ioreq_runio_qemu_aio' won't call 'ioreq_unmap' or 'ioreq_finish' on errors, leaving ioreq in the blkdev-inflight list and a leak. Signed-off-by: Feiran Zheng famc...@gmail.com --- hw/xen_disk.c | 22 +- 1 files changed, 17 insertions(+), 5 deletions(-) diff --git a/hw/xen_disk.c b/hw/xen_disk.c index 445bf03..7940fab 100644 --- a/hw/xen_disk.c +++ b/hw/xen_disk.c @@ -309,8 +309,10 @@ static int ioreq_runio_qemu_sync(struct ioreq *ioreq) int i, rc, len = 0; off_t pos; - if (ioreq-req.nr_segments ioreq_map(ioreq) == -1) - goto err; + if (ioreq-req.nr_segments) { + if (ioreq_map(ioreq) == -1) + goto err_no_map; + } if (ioreq-presync) bdrv_flush(blkdev-bs); this change is just to make the code clearer and easier to read, right? I think so. @@ -364,6 +366,9 @@ static int ioreq_runio_qemu_sync(struct ioreq *ioreq) return 0; err: + ioreq_unmap(ioreq); +err_no_map: + ioreq_finish(ioreq); ioreq-status = BLKIF_RSP_ERROR; return -1; } @@ -392,8 +397,10 @@ static int ioreq_runio_qemu_aio(struct ioreq *ioreq) { struct XenBlkDev *blkdev = ioreq-blkdev; - if (ioreq-req.nr_segments ioreq_map(ioreq) == -1) - goto err; + if (ioreq-req.nr_segments) { + if (ioreq_map(ioreq) == -1) + goto err_no_map; + } ioreq-aio_inflight++; if (ioreq-presync) @@ -425,9 +432,14 @@ static int ioreq_runio_qemu_aio(struct ioreq *ioreq) qemu_aio_complete(ioreq, 0); return 0; + +err_no_map: + ioreq_finish(ioreq); + ioreq-status = BLKIF_RSP_ERROR; + return -1; why aren't you calling ioreq_unmap before ioreq_finish, like in the previous case? It seems not a must but if call qemu_aio_complete, the error info can be printed, I thought it may be helpful. -- Best regards! Fam Zheng
Re: [Qemu-devel] [PATCH 27/27] Add SLOF-based partition firmware for pSeries machine, allowing more boot options
On Mon, Mar 28, 2011 at 08:16:51AM -0500, Anthony Liguori wrote: On 03/28/2011 04:03 AM, Alexander Graf wrote: Um, ok. Do I need to do anything about this? I'm also not sure this is too important. It's GPL compliance so yes, it's very important. Most of our firmware blobs come from svn repos which can't be submoduled. The only firmware blob we're not currently including as a git submodule is OpenBIOS. I believe the main reason is that different boards use different commits so a single submodule is a bit challenge. We probably ought to figure something out here though for the next release. Um.. where? I don't see these sources. Can anyone comment a bit more about OpenBIOS? BTW, OpenBIOS is already actively mirrored on git.qemu.org so all that's needed is a patch that does a git submodule add with the appropriate commit. And as long as we don't have a consistent policy about it, we can just as well stick with the README file. We do have a consistent policy :-) We're just not enforcing it as tightly as we should. Any binary we ship in the release tgz's should also have corresponding source in a submodule. So, again, what do I need to do? -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
Re: [Qemu-devel] [PATCH 1/7] target-arm: Make Neon helper routines use correct FP status
On 14 March 2011 05:35, Nathan Froyd froy...@codesourcery.com wrote: On Fri, Mar 11, 2011 at 10:31:31PM +, Peter Maydell wrote: On 11 March 2011 18:30, Nathan Froyd froy...@codesourcery.com wrote: Is there a reason that you don't simply use the global env rather than passing in an extra parameter everywhere? Just following the pattern that generally seems to be used by most helper functions, ie if you want the CPU env pass it in as a parameter. As far as I know, you can't use the global env unless you're in op_helper.c because that's the only source file compiled with the right flags. Oh, right. I am ambivalent as to whether passing env to such functions is the right thing to do or not. So did this amount to a request for a change to this patchset, or are you happy to let it pass? (I'm planning to stick this patchset into a pull-request with some of the other ARM patches that have had a few weeks for review comment later this week, so if you'd like a change now would be a good time to say so...) thanks -- PMM
Re: [Qemu-devel] [PATCH 00/17] s390x emulation support
On 03/24/2011 04:58 PM, Alexander Graf wrote: We've had support for running s390x guests with KVM for a while now. This patch set also enables support for running s390x guests in system as well as linux-user mode in emulation! Within this scope, I again want to stress that this is _not_ supposed to replace Hercules - the s390 emulator - in any way. The only target supported by qemu is Linux. You can only run Linux applications with linux-user emulation and Linux guest OSs with the system emulation. All the device logic (and 24 bit mode) for running legacy stuff is missing. Use Hercules for those! I have successfully run the following guest OSs: - SUSE Linux Enterprise Server 11 SP1 - Debian Lenny Both of which work just fine on x86_64 and ppc hosts. Other hosts should also work. The only thing that did not work for me is network. Somehow networking only works with KVM enabled, so there is probably some bug involved still. Either way - rejoice! As with this patch set you can finally fulfill your mainframe desires on your local workstation. And - most importantly - finally test patches to virtio against s390! For images, I'm hoping for Aurelien to provide Debian images that run in qemu. Other distributions only provide S390x target support in their enterprise variants, keeping me from redistributing images :(. If you're trying to get things rolling yourself, make sure to use a recent kernel that has support for the virtio architecture and virtio console support - otherwise you won't see output. The linux user mode emulation part only support 64bit binaries, so running Debian binaries with that one is out of question for now. Use the system emulation mode if you really need to run Debian binaries. I forgot the most important piece of information: git://repo.or.cz/qemu/agraf.git s390-tcg-v1 Alex
Re: [Qemu-devel] [PATCH v23 08/11] libcacard: add passthru
On Mon, Mar 28, 2011 at 03:27:49PM +0200, Jes Sorensen wrote: On 03/23/11 14:19, Alon Levy wrote: diff --git a/libcacard/passthru.c b/libcacard/passthru.c new file mode 100644 index 000..d78e2db --- /dev/null +++ b/libcacard/passthru.c @@ -0,0 +1,609 @@ +/* + * implement the applets for the CAC card. + * + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later. + * See the COPYING.LIB file in the top-level directory. + */ +#ifdef USE_PASSTHRU +#include stdlib.h +#include string.h + +#include pcsclite.h + +#include qemu-thread.h + +#include vcard.h +#include vcard_emul.h +#include card_7816.h +#include vreader.h +#include vcard_emul.h +#include passthru.h + +/* + * Passthru applet private data + */ +struct VCardAppletPrivateStruct { +char *reader_name; +/* pcsc-lite parameters */ +SCARDHANDLE hCard; +uint32_t hProtocol; +SCARD_IO_REQUEST *send_io; +unsigned char atr[MAX_ATR_SIZE]; +int atr_len; +}; + +static SCARDCONTEXT global_context; + +#define MAX_RESPONSE_LENGTH 261 /*65537 */ +/* + * handle all the APDU's that are common to all CAC applets + */ +static VCardStatus +passthru_process_apdu(VCard *card, VCardAPDU *apdu, VCardResponse **response) +{ +LONG rv; Where does this odd LONG type come from? I think Windows uses LONG so having your own type would conflict with that, and of course we don't really want Windows types directly in QEMU either, so I am curious? Using the not my code defence, so no idea. I'll fix. Otherwise the code looks fine. Cheers, Jes
Re: [Qemu-devel] [PATCH v23 01/11] trace: move trace objects from Makefile to Makefile.objs
On Mon, Mar 28, 2011 at 01:59:10PM +0200, Jes Sorensen wrote: On 03/23/11 14:19, Alon Levy wrote: --- Makefile | 32 Makefile.objs | 32 2 files changed, 32 insertions(+), 32 deletions(-) Acked-by: Jes Sorensen jes.soren...@redhat.com This should be able to go in as is - even other parts of ccid should stall. ok, for convenience I'm just going to send v24 with all patches, including the acked ones - no patch name has been change, no patch added, so it should be easy to just skip the ones you already acked. Cheers, Jes
Re: [Qemu-devel] [PATCH v23 03/11] usb-ccid: add CCID bus
On Mon, Mar 28, 2011 at 02:01:01PM +0200, Jes Sorensen wrote: On 03/23/11 14:19, Alon Levy wrote: A CCID device is a smart card reader. It is a USB device, defined at [1]. This patch introduces the usb-ccid device that is a ccid bus. Next patches will introduce two card types to use it, a passthru card and an emulated card. [1] http://www.usb.org/developers/devclass_docs/DWG_Smart-Card_CCID_Rev110. Signed-off-by: Alon Levy al...@redhat.com This looks ok to me now - I noticed that the comments in the header file refers to u8/u16/u32 for types, even though the code uses uint{8,16,32}_t, but I don't think that warrants a fix. You mean the comments in hw/usb-ccid.c in the descriptor? that's just shorter. If the comment goes in, please fix the spelling of my name. Reviewed-by: Jes Sorensen jes.soren...@redhat.com
Re: [Qemu-devel] [PATCH v23 03/11] usb-ccid: add CCID bus
On 03/28/11 16:28, Alon Levy wrote: On Mon, Mar 28, 2011 at 02:01:01PM +0200, Jes Sorensen wrote: On 03/23/11 14:19, Alon Levy wrote: A CCID device is a smart card reader. It is a USB device, defined at [1]. This patch introduces the usb-ccid device that is a ccid bus. Next patches will introduce two card types to use it, a passthru card and an emulated card. [1] http://www.usb.org/developers/devclass_docs/DWG_Smart-Card_CCID_Rev110. Signed-off-by: Alon Levy al...@redhat.com This looks ok to me now - I noticed that the comments in the header file refers to u8/u16/u32 for types, even though the code uses uint{8,16,32}_t, but I don't think that warrants a fix. You mean the comments in hw/usb-ccid.c in the descriptor? that's just shorter. I wasn't sure if it was a leftover from some of the headers being used in the kernel. However I am fine with it - no need to change it, and it makes it shorter as you say. Cheers, Jes
Re: [Qemu-devel] [PATCH v23 01/11] trace: move trace objects from Makefile to Makefile.objs
On 03/28/11 16:26, Alon Levy wrote: On Mon, Mar 28, 2011 at 01:59:10PM +0200, Jes Sorensen wrote: On 03/23/11 14:19, Alon Levy wrote: --- Makefile | 32 Makefile.objs | 32 2 files changed, 32 insertions(+), 32 deletions(-) Acked-by: Jes Sorensen jes.soren...@redhat.com This should be able to go in as is - even other parts of ccid should stall. ok, for convenience I'm just going to send v24 with all patches, including the acked ones - no patch name has been change, no patch added, so it should be easy to just skip the ones you already acked. Thats fine - can you include a list of which are changed so it is faster to review just those? Cheers, Jes
[Qemu-devel] [PATCH v2 0/5] Let boards state maximum RAM limits in QEMUMachine struct
This fairly simple patchset adds a new 'max_ram' field to the QEMUMachine structure so that a board model can specify the maximum RAM it will accept. We can then produce a friendly diagnostic message when the user tries to start qemu with a '-m' option asking for more RAM than that. (Currently most of the ARM devboard models respond with an obscure guest crash when the guest tries to access RAM and finds device registers instead.) If no maximum size is specified we default to the old behaviour of do not impose any limit. The advantage of doing this in vl.c rather than in each board (apart from avoiding code duplication) is that we can distinguish between the user asked for more RAM than we support (an error) and the global default RAM size is more than our maximum (just cap the RAM size to the board maximum). Changes in v2: * use target_physaddr_t rather than ram_addr_t for max_ram, so we can specify maximum ram sizes for 64 bit target boards * new patches 3,4 which update sun4m to use the generic max_ram, so we can delete the sun4m-specific code which was doing the same job * patch 5 does some tidy-up of sun4m init functions; not strictly related but the assert() at least is enabled by the cleanup done in patch 3. The number of changed lines in sun4m.c is a bit alarming but it's almost all just moving code around... Peter Maydell (5): Allow boards to specify maximum RAM size hw: Add maximum RAM specifications for ARM devboard models hw/sun4m: Move QEMUMachine structs into sun4*_hwdef structs hw/sun4m: Use the QEMUMachine max_ram to implement memory limit hw/sun4m: Use a macro to hide the repetitive board init functions hw/boards.h |1 + hw/integratorcp.c |1 + hw/realview.c | 11 + hw/sun4m.c| 586 ++--- hw/versatilepb.c |5 + vl.c | 16 ++- 6 files changed, 273 insertions(+), 347 deletions(-)
Re: [Qemu-devel] [PATCH v23 06/11] libcacard: initial commit
On Mon, Mar 28, 2011 at 02:35:23PM +0200, Jes Sorensen wrote: On 03/23/11 14:19, Alon Levy wrote: From: Robert Relyea rrel...@redhat.com libcacard emulates a Common Access Card (CAC) which is a standard for smartcards. It is used by the emulated ccid card introduced in a following patch. Docs are available in docs/libcacard.txt Signed-off-by: Alon Levy al...@redhat.com A couple of minor nits. diff --git a/Makefile.objs b/Makefile.objs index 744e1d3..f513ffa 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -352,6 +352,11 @@ user-obj-y += qemu-timer-common.o endif endif +## +# smartcard + +libcacard-y = cac.o event.o vcard.o vreader.o vcard_emul_nss.o vcard_emul_type.o card_7816.o + vl.o: QEMU_CFLAGS+=$(GPROF_CFLAGS) vl.o: QEMU_CFLAGS+=$(SDL_CFLAGS) diff --git a/Makefile.target b/Makefile.target index 62b102a..7f163e3 100644 --- a/Makefile.target +++ b/Makefile.target @@ -353,6 +353,8 @@ obj-y += $(addprefix $(HWDIR)/, $(hw-obj-y)) endif # CONFIG_SOFTMMU +obj-y += $(addprefix ../libcacard/, $(libcacard-$(CONFIG_SMARTCARD_NSS))) + obj-y += $(addprefix ../, $(trace-obj-y)) obj-$(CONFIG_GDBSTUB_XML) += gdbstub-xml.o This is a bit backwards, normally we do foobar-$(CONFIG_FOOBAR) = foo.o bar.o and then later obj-y = $(foobar-y) diff --git a/libcacard/cac.c b/libcacard/cac.c new file mode 100644 index 000..7a910d8 --- /dev/null +++ b/libcacard/cac.c @@ -0,0 +1,406 @@ +/* + * implement the applets for the CAC card. + * + * This code is licensed under the GNU LGPL, version 2.1 or later. + * See the COPYING.LIB file in the top-level directory. + * + */ +#include stdlib.h +#include string.h + +#include qemu-common.h stdlib.h and string.h are both included by qemu-common.h diff --git a/libcacard/card_7816.c b/libcacard/card_7816.c new file mode 100644 index 000..4c10cae --- /dev/null +++ b/libcacard/card_7816.c @@ -0,0 +1,764 @@ +/* + * Implement the 7816 portion of the card spec + * + * This code is licensed under the GNU LGPL, version 2.1 or later. + * See the COPYING.LIB file in the top-level directory. + */ + +#include stdlib.h +#include string.h +#include qemu-common.h same here diff --git a/libcacard/event.c b/libcacard/event.c new file mode 100644 index 000..12722cc --- /dev/null +++ b/libcacard/event.c @@ -0,0 +1,108 @@ +/* + * event queue implementation. + * + * This code is licensed under the GNU LGPL, version 2.1 or later. + * See the COPYING.LIB file in the top-level directory. + */ + +#include stdlib.h + +#include qemu-thread.h +#include qemu-common.h again here diff --git a/libcacard/vcard.c b/libcacard/vcard.c new file mode 100644 index 000..d7828a2 --- /dev/null +++ b/libcacard/vcard.c @@ -0,0 +1,341 @@ +/* + * implement the Java card standard. + * + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later. + * See the COPYING.LIB file in the top-level directory. + */ +#include stdlib.h +#include string.h + +#include qemu-common.h and here diff --git a/libcacard/vcard_emul_nss.c b/libcacard/vcard_emul_nss.c new file mode 100644 index 000..d3ab7ea --- /dev/null +++ b/libcacard/vcard_emul_nss.c @@ -0,0 +1,1159 @@ +/* + * This is the actual card emulator. + * + * These functions can be implemented in different ways on different platforms + * using the underlying system primitives. For Linux it uses NSS, though direct + * to PKCS #11, openssl+pkcs11, or even gnu crypto libraries+pkcs #11 could be + * used. On Windows CAPI could be used. + * + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later. + * See the COPYING.LIB file in the top-level directory. + */ + +/* + * system headers + */ +#include stdlib.h +#include string.h + +/* + * NSS headers + */ +#include nss.h +#include pk11pub.h +#include cert.h +#include key.h +#include secmod.h +#include prthread.h +#include secerr.h + +#include qemu-common.h again here prthread.h do you have a check for it in configure? I have to admit I really would prefer QEMU not relying on the NSPR stuff, but I don't know if it can be avoided with the ccid code? Tried removing, we do actually use it, but I'll try to remove it in a later patch. (not in this series). diff --git a/libcacard/vreader.c b/libcacard/vreader.c new file mode 100644 index 000..0b67c6c --- /dev/null +++ b/libcacard/vreader.c @@ -0,0 +1,519 @@ +/* + * emulate the reader + * + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later. + * See the COPYING.LIB file in the top-level directory. + */ + +/* + * System includes + */ +#include stdlib.h +#include
Re: [Qemu-devel] [PATCH 12/17] s390x: Prepare cpu.h for emulation
On 24 March 2011 15:58, Alexander Graf ag...@suse.de wrote: diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h Minor nits only. - FPReg fregs[16]; /* FP registers */ + CPU_DoubleU fregs[16]; /* FP registers */ These changes mean that the FPReg typedef in this file is no longer used, so you might as well delete it. Personally I prefer the way target-arm handles float regs, ie it just has 'float64 regs[32]' and relies on them being the right representation to pass in registers. This is less likely to work with float128s though, and anyway I suspect Nathan would disagree with me, so this isn't a request to change this code. +#define EXCP_EXT 1 + +#define EXCP_SVC 2 /* supervisor call (syscall) */ +#define EXCP_PGM 3 /* program interruption */ +/* XXX */ +#define EXCP_EXECUTE_SVC 0xff0 /* supervisor call via execute insn */ This comment ought to have an explanation of what the issue is that means it's 'XXX'... + CC_OP_ADD_64, /* */ + CC_OP_ADDU_64, /* */ + CC_OP_SUB_64, /* */ + CC_OP_SUBU_64, /* */ + CC_OP_ABS_64, /* */ + CC_OP_NABS_64, /* */ Why the empty comments? +static inline uint64_t time2tod(uint64_t time) { + return (time 9) / 125; +} Could maybe use a comment about what units we're converting to and from here. -- PMM
Re: [Xen-devel] Re: [Qemu-devel] [PATCH V11 02/15] xen: Make Xen build once.
On Wed, Mar 23, 2011 at 10:57, Alexander Graf ag...@suse.de wrote: On 01.03.2011, at 19:35, anthony.per...@citrix.com wrote: From: Anthony PERARD anthony.per...@citrix.com xen_domainbuild is now build in libhw. And xen_machine_pv is build only for i386 targets. Signed-off-by: Anthony PERARD anthony.per...@citrix.com --- Makefile.objs | 3 +++ Makefile.target | 2 +- hw/xen_domainbuild.c | 10 +- hw/xen_domainbuild.h | 5 +++-- hw/xen_machine_pv.c | 2 +- 5 files changed, 13 insertions(+), 9 deletions(-) diff --git a/Makefile.objs b/Makefile.objs index 9e98a66..8034115 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -269,6 +269,9 @@ hw-obj-$(CONFIG_DP8393X) += dp8393x.o hw-obj-$(CONFIG_DS1225Y) += ds1225y.o hw-obj-$(CONFIG_MIPSNET) += mipsnet.o +# Xen +hw-obj-$(CONFIG_XEN) += xen_domainbuild.o Why is this in generic code? Xen is x86 only and really should stay that way IMHO. I just try to build more object globally to avoid unnecessary i386-isms. + # Sound sound-obj-y = sound-obj-$(CONFIG_SB16) += sb16.o diff --git a/Makefile.target b/Makefile.target index 220589e..ab0a570 100644 --- a/Makefile.target +++ b/Makefile.target @@ -206,7 +206,7 @@ QEMU_CFLAGS += $(VNC_JPEG_CFLAGS) QEMU_CFLAGS += $(VNC_PNG_CFLAGS) # xen backend driver support -obj-$(CONFIG_XEN) += xen_machine_pv.o xen_domainbuild.o +obj-i386-$(CONFIG_XEN) += xen_machine_pv.o # Inter-VM PCI shared memory obj-$(CONFIG_KVM) += ivshmem.o diff --git a/hw/xen_domainbuild.c b/hw/xen_domainbuild.c index 7f1fd66..b73d47f 100644 --- a/hw/xen_domainbuild.c +++ b/hw/xen_domainbuild.c @@ -1,9 +1,9 @@ #include signal.h -#include xen_backend.h -#include xen_domainbuild.h #include sysemu.h #include qemu-timer.h #include qemu-log.h +#include xen_backend.h +#include xen_domainbuild.h #include xenguest.h @@ -49,7 +49,7 @@ static int xenstore_domain_mkdir(char *path) } int xenstore_domain_init1(const char *kernel, const char *ramdisk, - const char *cmdline) + const char *cmdline, ram_addr_t ram_size) Isn't ram_size a global anyways? What's the rationale behind moving it to a parameter? Not saying I'm against it, just missed the reasoning here :) I put ram_size in a parameter because I don't found a way to access to is global variable, and also because in these function, ram_size is read only. So, I can just remove this patch and just put both xen_machine_pv xen_domainbuild in obj-i386-y. Regards, -- Anthony PERARD
[Qemu-devel] [PATCH v6 0/4] piix_pci: optimize irq data path
Here is v6 which fixed piix3_set_irq_pic(). 4/4 needs more extensive tests. So please feel free to pick it up now or drop it for now. patch description: This patch series optimizes irq data path of piix_pci. So far piix3 tracks each pirq level and checks whether a given pic pins is asserted by seeing if each pirq is mapped into the pic pin. This is independent on irq routing, but data path is on slow path. Given that irq routing is rarely changed and asserting pic pins is on data path, the path that asserts pic pins should be optimized and chainging irq routing should be on slow path. The new behavior with this patch series is to use bitmap which is addressed by pirq and pic pins with a given irq routing. When pirq is asserted, the bitmap is set and see if the pic pins is asserted by checking the bitmaps. When irq routing is changed, rebuild the bitmap and re-assert pic pins. Changes v5 - v6: - fixed piix3_set_irq_pic() Changes v4 - v5: - typo Changes v3 - v4: - use pirq, pci_intx instead of irq_num in piix_pci.c - use symbolic constant PIC_NUM_PINS - introduced new patch 4/4 which cleans up a bit. Changes v2 - v3: - s/dummy_for_save_load_compat/pci_irq_levels_vmstate/g - move down unused member of pci_irq_levels_vmstate in the structure for cache efficiency Changes v1 - v2: - addressed review comments. Isaku Yamahata (4): pci: add accessor function to get irq levels piix_pci: eliminate PIIX3State::pci_irq_levels piix_pci: optimize set irq path piix_pci: load path clean up hw/pci.c |7 +++ hw/pci.h |1 + hw/piix_pci.c | 129 ++--- 3 files changed, 112 insertions(+), 25 deletions(-)
[Qemu-devel] [PATCH v6 2/4] piix_pci: eliminate PIIX3State::pci_irq_levels
PIIX3State::pci_irq_levels are redundant which is already tracked by PCIBus layer. So eliminate them. Cc: Juan Quintela quint...@redhat.com Cc: Michael S. Tsirkin m...@redhat.com Signed-off-by: Isaku Yamahata yamah...@valinux.co.jp --- Changes v3 - v4: - use PCI_NUM_PINS instead of magic number 4 Changes v2 - v3: - rename member s/dummy_for_save_load_compat/pci_irq_levels_vmstate/g --- hw/piix_pci.c | 38 +++--- 1 files changed, 27 insertions(+), 11 deletions(-) diff --git a/hw/piix_pci.c b/hw/piix_pci.c index 358da58..35e420c 100644 --- a/hw/piix_pci.c +++ b/hw/piix_pci.c @@ -37,10 +37,14 @@ typedef PCIHostState I440FXState; +#define PIIX_NUM_PIRQS 4ULL/* PIRQ[A-D] */ + typedef struct PIIX3State { PCIDevice dev; -int pci_irq_levels[4]; qemu_irq *pic; + +/* This member isn't used. Just for save/load compatibility */ +int32_t pci_irq_levels_vmstate[PIIX_NUM_PIRQS]; } PIIX3State; struct PCII440FXState { @@ -162,9 +166,11 @@ static int i440fx_load_old(QEMUFile* f, void *opaque, int version_id) i440fx_update_memory_mappings(d); qemu_get_8s(f, d-smm_enabled); -if (version_id == 2) -for (i = 0; i 4; i++) -d-piix3-pci_irq_levels[i] = qemu_get_be32(f); +if (version_id == 2) { +for (i = 0; i PIIX_NUM_PIRQS; i++) { +qemu_get_be32(f); /* dummy load for compatibility */ +} +} return 0; } @@ -236,7 +242,7 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int *piix3_devfn, qemu_irq * piix3 = DO_UPCAST(PIIX3State, dev, pci_create_simple_multifunction(b, -1, true, PIIX3)); piix3-pic = pic; -pci_bus_irqs(b, piix3_set_irq, pci_slot_get_pirq, piix3, 4); +pci_bus_irqs(b, piix3_set_irq, pci_slot_get_pirq, piix3, PIIX_NUM_PIRQS); (*pi440fx_state)-piix3 = piix3; *piix3_devfn = piix3-dev.devfn; @@ -256,8 +262,6 @@ static void piix3_set_irq(void *opaque, int irq_num, int level) int i, pic_irq, pic_level; PIIX3State *piix3 = opaque; -piix3-pci_irq_levels[irq_num] = level; - /* now we change the pic irq level according to the piix irq mappings */ /* XXX: optimize */ pic_irq = piix3-dev.config[0x60 + irq_num]; @@ -266,8 +270,9 @@ static void piix3_set_irq(void *opaque, int irq_num, int level) to it */ pic_level = 0; for (i = 0; i 4; i++) { -if (pic_irq == piix3-dev.config[0x60 + i]) -pic_level |= piix3-pci_irq_levels[i]; +if (pic_irq == piix3-dev.config[0x60 + i]) { +pic_level |= pci_bus_get_irq_level(piix3-dev.bus, i); +} } qemu_set_irq(piix3-pic[pic_irq], pic_level); } @@ -309,8 +314,17 @@ static void piix3_reset(void *opaque) pci_conf[0xab] = 0x00; pci_conf[0xac] = 0x00; pci_conf[0xae] = 0x00; +} -memset(d-pci_irq_levels, 0, sizeof(d-pci_irq_levels)); +static void piix3_pre_save(void *opaque) +{ +int i; +PIIX3State *piix3 = opaque; + +for (i = 0; i ARRAY_SIZE(piix3-pci_irq_levels_vmstate); i++) { +piix3-pci_irq_levels_vmstate[i] = +pci_bus_get_irq_level(piix3-dev.bus, i); +} } static const VMStateDescription vmstate_piix3 = { @@ -318,9 +332,11 @@ static const VMStateDescription vmstate_piix3 = { .version_id = 3, .minimum_version_id = 2, .minimum_version_id_old = 2, +.pre_save = piix3_pre_save, .fields = (VMStateField []) { VMSTATE_PCI_DEVICE(dev, PIIX3State), -VMSTATE_INT32_ARRAY_V(pci_irq_levels, PIIX3State, 4, 3), +VMSTATE_INT32_ARRAY_V(pci_irq_levels_vmstate, PIIX3State, + PIIX_NUM_PIRQS, 3), VMSTATE_END_OF_LIST() } }; -- 1.7.1.1
[Qemu-devel] [PATCH v6 4/4] piix_pci: load path clean up
The previous patch didn't change the behavior when load, it resulted in ugly code. This patch cleans it up. With this patch, pic irq lines are manipulated when loaded. It is expected that it won't change the behaviour because the interrupts are level: at the moment e.g. pci devices already reassert interrupts on load. Signed-off-by: Isaku Yamahata yamah...@valinux.co.jp --- Changes v3 - v4: - newly introduced - TODO: test more OSes, stress test with save/load, live-migration --- hw/piix_pci.c | 12 1 files changed, 4 insertions(+), 8 deletions(-) diff --git a/hw/piix_pci.c b/hw/piix_pci.c index 7ffb821..5f0d92f 100644 --- a/hw/piix_pci.c +++ b/hw/piix_pci.c @@ -281,8 +281,7 @@ static void piix3_set_irq_pic(PIIX3State *piix3, int pic_irq) (pic_irq * PIIX_NUM_PIRQS; } -static void piix3_set_irq_level(PIIX3State *piix3, int pirq, int level, -bool propagate) +static void piix3_set_irq_level(PIIX3State *piix3, int pirq, int level) { int pic_irq; uint64_t mask; @@ -296,15 +295,13 @@ static void piix3_set_irq_level(PIIX3State *piix3, int pirq, int level, piix3-pic_levels = ~mask; piix3-pic_levels |= mask * !!level; -if (propagate) { -piix3_set_irq_pic(piix3, pic_irq); -} +piix3_set_irq_pic(piix3, pic_irq); } static void piix3_set_irq(void *opaque, int pirq, int level) { PIIX3State *piix3 = opaque; -piix3_set_irq_level(piix3, pirq, level, true); +piix3_set_irq_level(piix3, pirq, level); } /* irq routing is changed. so rebuild bitmap */ @@ -315,8 +312,7 @@ static void piix3_update_irq_levels(PIIX3State *piix3) piix3-pic_levels = 0; for (pirq = 0; pirq PIIX_NUM_PIRQS; pirq++) { piix3_set_irq_level(piix3, pirq, -pci_bus_get_irq_level(piix3-dev.bus, pirq), -false); +pci_bus_get_irq_level(piix3-dev.bus, pirq)); } } -- 1.7.1.1
[Qemu-devel] [PATCH v6 1/4] pci: add accessor function to get irq levels
Introduce accessor function to know INTx levels. It will be used later by q35. Although piix_pci tracks the intx line levels, it can be eliminated by this helper function. Cc: Michael S. Tsirkin m...@redhat.com Signed-off-by: Isaku Yamahata yamah...@valinux.co.jp --- hw/pci.c |7 +++ hw/pci.h |1 + 2 files changed, 8 insertions(+), 0 deletions(-) diff --git a/hw/pci.c b/hw/pci.c index 8b76cea..6ad3f10 100644 --- a/hw/pci.c +++ b/hw/pci.c @@ -126,6 +126,13 @@ static void pci_change_irq_level(PCIDevice *pci_dev, int irq_num, int change) bus-set_irq(bus-irq_opaque, irq_num, bus-irq_count[irq_num] != 0); } +int pci_bus_get_irq_level(PCIBus *bus, int irq_num) +{ +assert(irq_num = 0); +assert(irq_num bus-nirq); +return !!bus-irq_count[irq_num]; +} + /* Update interrupt status bit in config space on interrupt * state change. */ static void pci_update_irq_status(PCIDevice *dev) diff --git a/hw/pci.h b/hw/pci.h index 113e556..092a463 100644 --- a/hw/pci.h +++ b/hw/pci.h @@ -233,6 +233,7 @@ void pci_bus_new_inplace(PCIBus *bus, DeviceState *parent, PCIBus *pci_bus_new(DeviceState *parent, const char *name, int devfn_min); void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq, void *irq_opaque, int nirq); +int pci_bus_get_irq_level(PCIBus *bus, int irq_num); void pci_bus_hotplug(PCIBus *bus, pci_hotplug_fn hotplug, DeviceState *dev); PCIBus *pci_register_bus(DeviceState *parent, const char *name, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq, -- 1.7.1.1
[Qemu-devel] [PATCH v6 3/4] piix_pci: optimize set irq path
optimize irq routing in piix_pic.c which has been a TODO. So far piix3 tracks each pirq level and checks whether a given pic pins is asserted by seeing if each pirq is mapped into the pic pin. This is independent on irq routing, but data path is on slow path. Given that irq routing is rarely changed and asserting pic pins is on data path, the path that asserts pic pins should be optimized and chainging irq routing should be on slow path. The new behavior with this patch series is to use bitmap which is addressed by pirq and pic pins with a given irq routing. When pirq is asserted, the bitmap is set and see if the pic pins is asserted by checking the bitmaps. When irq routing is changed, rebuild the bitmap and re-assert pic pins. Cc: Michael S. Tsirkin m...@redhat.com Signed-off-by: Isaku Yamahata yamah...@valinux.co.jp --- Changes v4 - v5: - fix piix_set_irq_pic() Changes v3 - v4: - replace irq_num with pirq or pci_intx Changes v1 - v2: - some minor clean ups - commit log message --- hw/piix_pci.c | 101 +++- 1 files changed, 84 insertions(+), 17 deletions(-) diff --git a/hw/piix_pci.c b/hw/piix_pci.c index 35e420c..7ffb821 100644 --- a/hw/piix_pci.c +++ b/hw/piix_pci.c @@ -37,10 +37,27 @@ typedef PCIHostState I440FXState; +#define PIIX_NUM_PIC_IRQS 16 /* i8259 * 2 */ #define PIIX_NUM_PIRQS 4ULL/* PIRQ[A-D] */ +#define PIIX_PIRQC 0x60 typedef struct PIIX3State { PCIDevice dev; + +/* + * bitmap to track pic levels. + * The pic level is the logical OR of all the PCI irqs mapped to it + * So one PIC level is tracked by PIIX_NUM_PIRQS bits. + * + * PIRQ is mapped to PIC pins, we track it by + * PIIX_NUM_PIRQS * PIIX_NUM_PIC_IRQS = 64 bits with + * pic_irq * PIIX_NUM_PIRQS + pirq + */ +#if PIIX_NUM_PIC_IRQS * PIIX_NUM_PIRQS 64 +#error unable to encode pic state in 64bit in pic_levels. +#endif +uint64_t pic_levels; + qemu_irq *pic; /* This member isn't used. Just for save/load compatibility */ @@ -59,16 +76,16 @@ struct PCII440FXState { #define I440FX_PAM_SIZE 7 #define I440FX_SMRAM0x72 -static void piix3_set_irq(void *opaque, int irq_num, int level); +static void piix3_set_irq(void *opaque, int pirq, int level); /* return the global irq number corresponding to a given device irq pin. We could also use the bus number to have a more precise mapping. */ -static int pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num) +static int pci_slot_get_pirq(PCIDevice *pci_dev, int pci_intx) { int slot_addend; slot_addend = (pci_dev-devfn 3) - 1; -return (irq_num + slot_addend) 3; +return (pci_intx + slot_addend) 3; } static void update_pam(PCII440FXState *d, uint32_t start, uint32_t end, int r) @@ -256,25 +273,64 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int *piix3_devfn, qemu_irq * } /* PIIX3 PCI to ISA bridge */ +static void piix3_set_irq_pic(PIIX3State *piix3, int pic_irq) +{ +qemu_set_irq(piix3-pic[pic_irq], + !!(piix3-pic_levels +(((1UL PIIX_NUM_PIRQS) - 1) + (pic_irq * PIIX_NUM_PIRQS; +} -static void piix3_set_irq(void *opaque, int irq_num, int level) +static void piix3_set_irq_level(PIIX3State *piix3, int pirq, int level, +bool propagate) +{ +int pic_irq; +uint64_t mask; + +pic_irq = piix3-dev.config[PIIX_PIRQC + pirq]; +if (pic_irq = PIIX_NUM_PIC_IRQS) { +return; +} + +mask = 1ULL ((pic_irq * PIIX_NUM_PIRQS) + pirq); +piix3-pic_levels = ~mask; +piix3-pic_levels |= mask * !!level; + +if (propagate) { +piix3_set_irq_pic(piix3, pic_irq); +} +} + +static void piix3_set_irq(void *opaque, int pirq, int level) { -int i, pic_irq, pic_level; PIIX3State *piix3 = opaque; +piix3_set_irq_level(piix3, pirq, level, true); +} -/* now we change the pic irq level according to the piix irq mappings */ -/* XXX: optimize */ -pic_irq = piix3-dev.config[0x60 + irq_num]; -if (pic_irq 16) { -/* The pic level is the logical OR of all the PCI irqs mapped - to it */ -pic_level = 0; -for (i = 0; i 4; i++) { -if (pic_irq == piix3-dev.config[0x60 + i]) { -pic_level |= pci_bus_get_irq_level(piix3-dev.bus, i); -} +/* irq routing is changed. so rebuild bitmap */ +static void piix3_update_irq_levels(PIIX3State *piix3) +{ +int pirq; + +piix3-pic_levels = 0; +for (pirq = 0; pirq PIIX_NUM_PIRQS; pirq++) { +piix3_set_irq_level(piix3, pirq, +pci_bus_get_irq_level(piix3-dev.bus, pirq), +false); +} +} + +static void piix3_write_config(PCIDevice *dev, + uint32_t address, uint32_t val, int len) +{ +pci_default_write_config(dev, address, val, len); +if
Re: [Qemu-devel] [PATCH v23 08/11] libcacard: add passthru
On Mon, Mar 28, 2011 at 03:27:49PM +0200, Jes Sorensen wrote: On 03/23/11 14:19, Alon Levy wrote: diff --git a/libcacard/passthru.c b/libcacard/passthru.c new file mode 100644 index 000..d78e2db --- /dev/null +++ b/libcacard/passthru.c @@ -0,0 +1,609 @@ +/* + * implement the applets for the CAC card. + * + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later. + * See the COPYING.LIB file in the top-level directory. + */ +#ifdef USE_PASSTHRU +#include stdlib.h +#include string.h + +#include pcsclite.h + +#include qemu-thread.h + +#include vcard.h +#include vcard_emul.h +#include card_7816.h +#include vreader.h +#include vcard_emul.h +#include passthru.h + +/* + * Passthru applet private data + */ +struct VCardAppletPrivateStruct { +char *reader_name; +/* pcsc-lite parameters */ +SCARDHANDLE hCard; +uint32_t hProtocol; +SCARD_IO_REQUEST *send_io; +unsigned char atr[MAX_ATR_SIZE]; +int atr_len; +}; + +static SCARDCONTEXT global_context; + +#define MAX_RESPONSE_LENGTH 261 /*65537 */ +/* + * handle all the APDU's that are common to all CAC applets + */ +static VCardStatus +passthru_process_apdu(VCard *card, VCardAPDU *apdu, VCardResponse **response) +{ +LONG rv; Where does this odd LONG type come from? I think Windows uses LONG so having your own type would conflict with that, and of course we don't really want Windows types directly in QEMU either, so I am curious? This comes from pcsclite (which means I need to add the dependency to configure, I'll fix this). From there it comes via an include called wintypes.h. the PCSC API is actually a copy of a microsoft API, so it makes sense that it uses those types. On windows pcsclite won't be needed, since it has a native equivalent API, so there won't be a redefinition of LONG. So the real fix is that I need to add yet another dependency for qemu's configure, for pcsc-lite - but right now the whole libcacard/passthru.c is protected by an #ifdef USE_PASSTHRU, which we don't define - so in effect this is currently dead code. I could even just remove this commit in its entirety atm, maybe best? Notice that has nothing to do with ccid-card-emulated or with ccid-card-passthru. I will need it later, but I need to add the pcsclite configure check anyway. So is this ok, I'll drop this patch? Otherwise the code looks fine. Cheers, Jes
Re: [Qemu-devel] [PATCH v23 08/11] libcacard: add passthru
On 03/28/11 17:21, Alon Levy wrote: Where does this odd LONG type come from? I think Windows uses LONG so having your own type would conflict with that, and of course we don't really want Windows types directly in QEMU either, so I am curious? This comes from pcsclite (which means I need to add the dependency to configure, I'll fix this). From there it comes via an include called wintypes.h. the PCSC API is actually a copy of a microsoft API, so it makes sense that it uses those types. On windows pcsclite won't be needed, since it has a native equivalent API, so there won't be a redefinition of LONG. So the real fix is that I need to add yet another dependency for qemu's configure, for pcsc-lite - but right now the whole libcacard/passthru.c is protected by an #ifdef USE_PASSTHRU, which we don't define - so in effect this is currently dead code. I could even just remove this commit in its entirety atm, maybe best? Notice that has nothing to do with ccid-card-emulated or with ccid-card-passthru. I will need it later, but I need to add the pcsclite configure check anyway. So is this ok, I'll drop this patch? Interesting - yeah if the code isn't necessary, lets drop it for now. It will be easier to get this in now, and the extra bits later when they are ready. Cheers, Jes
Re: [Xen-devel] Re: [Qemu-devel] [PATCH V11 03/15] xen: Support new libxc calls from xen unstable.
On Wed, Mar 23, 2011 at 10:43, Alexander Graf ag...@suse.de wrote: On 01.03.2011, at 19:35, anthony.per...@citrix.com wrote: From: Anthony PERARD anthony.per...@citrix.com This patch updates the libxenctrl calls in Qemu to use the new interface, otherwise Qemu wouldn't be able to build against new versions of the library. We check libxenctrl version in configure, from Xen 3.3.0 to Xen unstable. Signed-off-by: Anthony PERARD anthony.per...@citrix.com Signed-off-by: Stefano Stabellini stefano.stabell...@eu.citrix.com Acked-by: Alexander Graf ag...@suse.de --- configure | 67 - hw/xen_backend.c | 21 --- hw/xen_backend.h | 6 ++-- hw/xen_common.h | 64 +-- hw/xen_disk.c | 4 +- hw/xen_domainbuild.c | 3 +- 6 files changed, 133 insertions(+), 32 deletions(-) diff --git a/configure b/configure index 3036faf..a84d974 100755 --- a/configure +++ b/configure @@ -126,6 +126,7 @@ vnc_jpeg= vnc_png= vnc_thread=no xen= +xen_ctrl_version= linux_aio= attr= vhost_net= @@ -1147,20 +1148,81 @@ fi if test $xen != no ; then xen_libs=-lxenstore -lxenctrl -lxenguest + + # Xen unstable cat $TMPC EOF #include xenctrl.h #include xs.h -int main(void) { xs_daemon_open(); xc_interface_open(); return 0; } +#include stdint.h +#include xen/hvm/hvm_info_table.h +#if !defined(HVM_MAX_VCPUS) +# error HVM_MAX_VCPUS not defined +#endif +int main(void) { + xc_interface *xc; + xs_daemon_open(); + xc = xc_interface_open(0, 0, 0); + xc_hvm_set_mem_type(0, 0, HVMMEM_ram_ro, 0, 0); + xc_gnttab_open(NULL, 0); + return 0; +} EOF if compile_prog $xen_libs ; then + xen_ctrl_version=410 xen=yes - libs_softmmu=$xen_libs $libs_softmmu + + # Xen 4.0.0 + elif ( + cat $TMPC EOF +#include xenctrl.h +#include xs.h +#include stdint.h +#include xen/hvm/hvm_info_table.h +#if !defined(HVM_MAX_VCPUS) +# error HVM_MAX_VCPUS not defined +#endif +int main(void) { + xs_daemon_open(); + xc_interface_open(); + xc_gnttab_open(); + xc_hvm_set_mem_type(0, 0, HVMMEM_ram_ro, 0, 0); + return 0; +} +EOF + compile_prog $xen_libs + ) ; then + xen_ctrl_version=400 + xen=yes + + # Xen 3.3.0, 3.4.0 + elif ( + cat $TMPC EOF +#include xenctrl.h +#include xs.h +int main(void) { + xs_daemon_open(); + xc_interface_open(); + xc_gnttab_open(); + xc_hvm_set_mem_type(0, 0, HVMMEM_ram_ro, 0, 0); + return 0; +} +EOF + compile_prog $xen_libs + ) ; then + xen_ctrl_version=330 + xen=yes + + # Xen not found or unsupported else if test $xen = yes ; then feature_not_found xen fi xen=no fi + + if test $xen = yes; then + libs_softmmu=$xen_libs $libs_softmmu + fi fi ## @@ -2755,6 +2817,7 @@ if test $bluez = yes ; then fi if test $xen = yes ; then echo CONFIG_XEN=y $config_host_mak + echo CONFIG_XEN_CTRL_INTERFACE_VERSION=$xen_ctrl_version $config_host_mak fi if test $io_thread = yes ; then echo CONFIG_IOTHREAD=y $config_host_mak diff --git a/hw/xen_backend.c b/hw/xen_backend.c index 9f4ec4b..3907b83 100644 --- a/hw/xen_backend.c +++ b/hw/xen_backend.c @@ -43,7 +43,8 @@ /* - */ /* public */ -int xen_xc; +XenXC xen_xc = XC_HANDLER_INITIAL_VALUE; +XenGnttab xen_xcg = XC_HANDLER_INITIAL_VALUE; struct xs_handle *xenstore = NULL; const char *xen_protocol; @@ -214,8 +215,8 @@ static struct XenDevice *xen_be_get_xendev(const char *type, int dom, int dev, xendev-debug = debug; xendev-local_port = -1; - xendev-evtchndev = xc_evtchn_open(); - if (xendev-evtchndev 0) { + xendev-evtchndev = xc_evtchn_open(NULL, 0); + if (xendev-evtchndev == XC_HANDLER_INITIAL_VALUE) { xen_be_printf(NULL, 0, can't open evtchn device\n); qemu_free(xendev); return NULL; @@ -223,15 +224,15 @@ static struct XenDevice *xen_be_get_xendev(const char *type, int dom, int dev, fcntl(xc_evtchn_fd(xendev-evtchndev), F_SETFD, FD_CLOEXEC); if (ops-flags DEVOPS_FLAG_NEED_GNTDEV) { - xendev-gnttabdev = xc_gnttab_open(); - if (xendev-gnttabdev 0) { + xendev-gnttabdev = xc_gnttab_open(NULL, 0); + if (xendev-gnttabdev == XC_HANDLER_INITIAL_VALUE) { xen_be_printf(NULL, 0, can't open gnttab device\n); xc_evtchn_close(xendev-evtchndev); qemu_free(xendev); return NULL; } } else { - xendev-gnttabdev = -1; + xendev-gnttabdev = XC_HANDLER_INITIAL_VALUE; } QTAILQ_INSERT_TAIL(xendevs, xendev, next); @@ -277,10 +278,10 @@ static struct XenDevice *xen_be_del_xendev(int dom, int dev) qemu_free(xendev-fe); } - if (xendev-evtchndev =
Re: [Qemu-devel] [PATCH 16/17] s390x: translate engine for s390x CPU
On 24 March 2011 15:58, Alexander Graf ag...@suse.de wrote: diff --git a/target-s390x/translate.c b/target-s390x/translate.c +typedef struct DisasContext DisasContext; +struct DisasContext { + uint64_t pc; + int is_jmp; + enum cc_op cc_op; + CPUS390XState *env; + struct TranslationBlock *tb; +}; I don't think anything actually uses dc-env, does it? (I like the way that almost none of the translate.c code gets a CPUState pointer, makes it hard to accidentally write buggy code that relies on things not in the tb_flags.) +static char cpu_reg_names[10*3 + 6*4]; I can see code ins390x_translate_init() which sets this up, but I can't see anything which uses it? +#if 0 /* reads four when it should read only 3 */ + case 2: Is there any point having #if'd out broken code? Either fix it and enable it, or just have a comment to the effect that we could have optimised versions for cases 2, 4, 5, 6 but currently do not. + case 0x4: /* LMG R1,R3,D2(B2) [RSE] */ + case 0x24: /* STMG R1,R3,D2(B2) [RSE] */ + case 0x26: /* STMH R1,R3,D2(B2) [RSE] */ + case 0x96: /* LMH R1,R3,D2(B2) [RSE] */ + /* Apparently, unrolling lmg/stmg of any size gains performance - + even for very long ones... */ Doesn't this take you over MAX_OP_PER_INSTR for some cases? + tmp2 = tcg_const_i64uint64_t)i2) 48) | 0xULL); This line is over 80 chars, as are a handful of others in this file. +case 0xa: /* SVCI [RR] */ +insn = ld_code2(s-pc); +debug_insn(insn); +i = insn 0xff; +#ifdef CONFIG_USER_ONLY +s-pc += 2; +#endif +update_psw_addr(s); +gen_op_calc_cc(s); Why do we only need to update s-pc if CONFIG_USER_ONLY? Not saying it's wrong, but it could use an explanatory comment... -- PMM
Re: [Qemu-devel] [PATCH v23 01/11] trace: move trace objects from Makefile to Makefile.objs
On Mon, Mar 28, 2011 at 04:30:26PM +0200, Jes Sorensen wrote: On 03/28/11 16:26, Alon Levy wrote: On Mon, Mar 28, 2011 at 01:59:10PM +0200, Jes Sorensen wrote: On 03/23/11 14:19, Alon Levy wrote: --- Makefile | 32 Makefile.objs | 32 2 files changed, 32 insertions(+), 32 deletions(-) Acked-by: Jes Sorensen jes.soren...@redhat.com This should be able to go in as is - even other parts of ccid should stall. ok, for convenience I'm just going to send v24 with all patches, including the acked ones - no patch name has been change, no patch added, so it should be easy to just skip the ones you already acked. Thats fine - can you include a list of which are changed so it is faster to review just those? Yes, it will be in the cover letter. Cheers, Jes
[Qemu-devel] [PATCH v24 00/10] usb-ccid
This patchset adds three new devices, usb-ccid, ccid-card-passthru and ccid-card-emulated, providing a CCID bus, a simple passthru protocol implementing card requiring a client, and a standalone emulated card. It also introduces a new directory libcaccard with CAC card emulation, CAC is a type of ISO 7816 smart card. Tree for pull: git://anongit.freedesktop.org/~alon/qemu usb_ccid.v24 v23-v24 changes: * libcacard: = changed patches: (that need re-review) * 6 - libcacard: initial commit * 7 - libcacard: add vscclient = changed but trust me don't need rereview: * 5 - ccid: add passthru card device (removed a duplicate line in the header, had two licenses). * drop libcacard add passthru patch, not ready, not used. * remove unrequired includes * use stderr in vscclient for printing errors * cosmetic fixes v22-v23 changes: * libcacard * configure fixes: (reported by Stefan Hajnoczi) * test a = b, not a == b (second isn't portable) * quote $source_path in case it contains spaces - this doesn't really help since there are many other places that need similar fixes, not introduced by this patch. v21-v22 changes: * libcacard: * fix configure to not link libcacard if nss not found (reported by Stefan Hajnoczi) * fix vscclient linkage with simpletrace backend (reported by Stefan Hajnoczi) * card_7816.c: add missing break in ERROR_DATA_NOT_FOUND (reported by William van de Velde) v20-v21 changes: * all: cosmetics * libcacard, ccid-card-passthru: * use qemu-{malloc,free} and qemu-thread, error_report * libcacard: * split to multiple patches v19-v20 changes: * checkpatch.pl. Here are the remaining errors with explanation: * ignored 5 macro errors of the type ERROR: Macros with complex values should be enclosed in parenthesis because fixing them breaks current code, if it really bothers someone I can fix it. * four of them are in libcacard/card_7816t.h: /* give the subfields a unified look */ .. #define a_cla a_header-ah_cla /* class */ #define a_ins a_header-ah_ins /* instruction */ #define a_p1 a_header-ah_p1 /* parameter 1 */ #define a_p2 a_header-ah_p2 /* parameter 2 */ * and the fifth: #4946: FILE: libcacard/vcardt.h:31: +#define VCARD_ATR_PREFIX(size) 0x3b, 0x66+(size), 0x00, 0xff, \ + 'V', 'C', 'A', 'R', 'D', '_' * Ignored this warning since I couldn't figure it out, and it's a test file: WARNING: externs should be avoided in .c files #2343: FILE: libcacard/link_test.c:7: +VCardStatus cac_card_init(const char *flags, VCard *card, v18-v19 changes: * more merges, down to a single digit number of patches. * drop enumeration property, use string. * rebased (trivial) v17-v18 changes: * merge vscard_common.h patches. * actually provide a tree to pull. v16-v17 changes: * merged all the v15-v16 patches * merged some more wherever it was easy (all same file commits). * added signed off by to first four patches * ccid.h: added copyright, removed underscore in defines, and replaced non C89 comments v15-v16 changes: * split vscard_common introducing patch for ease of review * sum of commit logs for the v15-v16 commits: (whitespace fixes removed for space, see original commit messages in later patches) * usb-ccid: * fix abort on client answer after card remove * enable migration * remove side affect code from asserts * return consistent self-powered state * mask out reserved bits in ccid_set_parameters * add missing abRFU in SetParameters (no affect on linux guest) * vscard_common.h protocol change: * VSCMsgInit capabilities and magic * removed ReaderResponse, will use Error instead with code==VSC_SUCCESS. * added Flush and FlushComplete, remove Reconnect. * define VSCARD_MAGIC * added error code VSC_SUCCESS. * ccid-card-passthru * return correct size * return error instead of assert if client sent too large ATR * don't assert if client sent too large a size, but add asserts for indices to buffer * reset vscard_in indices on chardev disconnect * handle init from client * error if no chardev supplied * use ntoh, hton * eradicate reader_id_t * remove Reconnect usage (removed from VSCARD protocol) * send VSC_SUCCESS on card insert/remove and reader add/remove * ccid-card-emulated * fix error reporting in initfn v14-v15 changes: * add patch with --enable-smartcard and --disable-smartcard and only disable ccid-card-emulated if nss not found. * add patch with description strings * s/libcaccard/libcacard/ in docs/ccid.txt v13-v14 changes: - support device_del/device_add on ccid-card-* and usb-ccid * usb-ccid: * lose card reference when card device deleted * check slot number and deny adding a slot if one is already added. * ccid-card-*: use qdev_simple_unplug_cb in both emulated and passthru ccid cards, the exitfn already takes care of triggering card removal in the usb dev. *
[Qemu-devel] [PATCH 01/10] trace: move trace objects from Makefile to Makefile.objs
--- Makefile | 32 Makefile.objs | 32 2 files changed, 32 insertions(+), 32 deletions(-) diff --git a/Makefile b/Makefile index 89e88b4..209e14d 100644 --- a/Makefile +++ b/Makefile @@ -112,38 +112,6 @@ ui/vnc.o: QEMU_CFLAGS += $(VNC_TLS_CFLAGS) bt-host.o: QEMU_CFLAGS += $(BLUEZ_CFLAGS) -ifeq ($(TRACE_BACKEND),dtrace) -trace.h: trace.h-timestamp trace-dtrace.h -else -trace.h: trace.h-timestamp -endif -trace.h-timestamp: $(SRC_PATH)/trace-events config-host.mak - $(call quiet-command,sh $(SRC_PATH)/scripts/tracetool --$(TRACE_BACKEND) -h $ $@, GEN trace.h) - @cmp -s $@ trace.h || cp $@ trace.h - -trace.c: trace.c-timestamp -trace.c-timestamp: $(SRC_PATH)/trace-events config-host.mak - $(call quiet-command,sh $(SRC_PATH)/scripts/tracetool --$(TRACE_BACKEND) -c $ $@, GEN trace.c) - @cmp -s $@ trace.c || cp $@ trace.c - -trace.o: trace.c $(GENERATED_HEADERS) - -trace-dtrace.h: trace-dtrace.dtrace - $(call quiet-command,dtrace -o $@ -h -s $, GEN trace-dtrace.h) - -# Normal practice is to name DTrace probe file with a '.d' extension -# but that gets picked up by QEMU's Makefile as an external dependancy -# rule file. So we use '.dtrace' instead -trace-dtrace.dtrace: trace-dtrace.dtrace-timestamp -trace-dtrace.dtrace-timestamp: $(SRC_PATH)/trace-events config-host.mak - $(call quiet-command,sh $(SRC_PATH)/scripts/tracetool --$(TRACE_BACKEND) -d $ $@, GEN trace-dtrace.dtrace) - @cmp -s $@ trace-dtrace.dtrace || cp $@ trace-dtrace.dtrace - -trace-dtrace.o: trace-dtrace.dtrace $(GENERATED_HEADERS) - $(call quiet-command,dtrace -o $@ -G -s $, GEN trace-dtrace.o) - -simpletrace.o: simpletrace.c $(GENERATED_HEADERS) - version.o: $(SRC_PATH)/version.rc config-host.mak $(call quiet-command,$(WINDRES) -I. -o $@ $, RC$(TARGET_DIR)$@) diff --git a/Makefile.objs b/Makefile.objs index f8cf199..1fa7a29 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -310,6 +310,38 @@ libdis-$(CONFIG_SPARC_DIS) += sparc-dis.o # trace ifeq ($(TRACE_BACKEND),dtrace) +trace.h: trace.h-timestamp trace-dtrace.h +else +trace.h: trace.h-timestamp +endif +trace.h-timestamp: $(SRC_PATH)/trace-events config-host.mak + $(call quiet-command,sh $(SRC_PATH)/scripts/tracetool --$(TRACE_BACKEND) -h $ $@, GEN trace.h) + @cmp -s $@ trace.h || cp $@ trace.h + +trace.c: trace.c-timestamp +trace.c-timestamp: $(SRC_PATH)/trace-events config-host.mak + $(call quiet-command,sh $(SRC_PATH)/scripts/tracetool --$(TRACE_BACKEND) -c $ $@, GEN trace.c) + @cmp -s $@ trace.c || cp $@ trace.c + +trace.o: trace.c $(GENERATED_HEADERS) + +trace-dtrace.h: trace-dtrace.dtrace + $(call quiet-command,dtrace -o $@ -h -s $, GEN trace-dtrace.h) + +# Normal practice is to name DTrace probe file with a '.d' extension +# but that gets picked up by QEMU's Makefile as an external dependancy +# rule file. So we use '.dtrace' instead +trace-dtrace.dtrace: trace-dtrace.dtrace-timestamp +trace-dtrace.dtrace-timestamp: $(SRC_PATH)/trace-events config-host.mak + $(call quiet-command,sh $(SRC_PATH)/scripts/tracetool --$(TRACE_BACKEND) -d $ $@, GEN trace-dtrace.dtrace) + @cmp -s $@ trace-dtrace.dtrace || cp $@ trace-dtrace.dtrace + +trace-dtrace.o: trace-dtrace.dtrace $(GENERATED_HEADERS) + $(call quiet-command,dtrace -o $@ -G -s $, GEN trace-dtrace.o) + +simpletrace.o: simpletrace.c $(GENERATED_HEADERS) + +ifeq ($(TRACE_BACKEND),dtrace) trace-obj-y = trace-dtrace.o else trace-obj-y = trace.o -- 1.7.4.1
[Qemu-devel] [PATCH v24 00/10] usb-ccid
This patchset adds three new devices, usb-ccid, ccid-card-passthru and ccid-card-emulated, providing a CCID bus, a simple passthru protocol implementing card requiring a client, and a standalone emulated card. It also introduces a new directory libcaccard with CAC card emulation, CAC is a type of ISO 7816 smart card. Tree for pull: git://anongit.freedesktop.org/~alon/qemu usb_ccid.v23 v23-v24 changes: * libcacard: = changed patches: (that need re-review) * 6 - libcacard: initial commit * 7 - libcacard: add vscclient * 5 - ccid: add passthru card device * drop libcacard add passthru patch, not ready, not used. * remove unrequired includes * use stderr in vscclient for printing errors * cosmetic fixes v22-v23 changes: * libcacard * configure fixes: (reported by Stefan Hajnoczi) * test a = b, not a == b (second isn't portable) * quote $source_path in case it contains spaces - this doesn't really help since there are many other places that need similar fixes, not introduced by this patch. v21-v22 changes: * libcacard: * fix configure to not link libcacard if nss not found (reported by Stefan Hajnoczi) * fix vscclient linkage with simpletrace backend (reported by Stefan Hajnoczi) * card_7816.c: add missing break in ERROR_DATA_NOT_FOUND (reported by William van de Velde) v20-v21 changes: * all: cosmetics * libcacard, ccid-card-passthru: * use qemu-{malloc,free} and qemu-thread, error_report * libcacard: * split to multiple patches v19-v20 changes: * checkpatch.pl. Here are the remaining errors with explanation: * ignored 5 macro errors of the type ERROR: Macros with complex values should be enclosed in parenthesis because fixing them breaks current code, if it really bothers someone I can fix it. * four of them are in libcacard/card_7816t.h: /* give the subfields a unified look */ .. #define a_cla a_header-ah_cla /* class */ #define a_ins a_header-ah_ins /* instruction */ #define a_p1 a_header-ah_p1 /* parameter 1 */ #define a_p2 a_header-ah_p2 /* parameter 2 */ * and the fifth: #4946: FILE: libcacard/vcardt.h:31: +#define VCARD_ATR_PREFIX(size) 0x3b, 0x66+(size), 0x00, 0xff, \ + 'V', 'C', 'A', 'R', 'D', '_' * Ignored this warning since I couldn't figure it out, and it's a test file: WARNING: externs should be avoided in .c files #2343: FILE: libcacard/link_test.c:7: +VCardStatus cac_card_init(const char *flags, VCard *card, v18-v19 changes: * more merges, down to a single digit number of patches. * drop enumeration property, use string. * rebased (trivial) v17-v18 changes: * merge vscard_common.h patches. * actually provide a tree to pull. v16-v17 changes: * merged all the v15-v16 patches * merged some more wherever it was easy (all same file commits). * added signed off by to first four patches * ccid.h: added copyright, removed underscore in defines, and replaced non C89 comments v15-v16 changes: * split vscard_common introducing patch for ease of review * sum of commit logs for the v15-v16 commits: (whitespace fixes removed for space, see original commit messages in later patches) * usb-ccid: * fix abort on client answer after card remove * enable migration * remove side affect code from asserts * return consistent self-powered state * mask out reserved bits in ccid_set_parameters * add missing abRFU in SetParameters (no affect on linux guest) * vscard_common.h protocol change: * VSCMsgInit capabilities and magic * removed ReaderResponse, will use Error instead with code==VSC_SUCCESS. * added Flush and FlushComplete, remove Reconnect. * define VSCARD_MAGIC * added error code VSC_SUCCESS. * ccid-card-passthru * return correct size * return error instead of assert if client sent too large ATR * don't assert if client sent too large a size, but add asserts for indices to buffer * reset vscard_in indices on chardev disconnect * handle init from client * error if no chardev supplied * use ntoh, hton * eradicate reader_id_t * remove Reconnect usage (removed from VSCARD protocol) * send VSC_SUCCESS on card insert/remove and reader add/remove * ccid-card-emulated * fix error reporting in initfn v14-v15 changes: * add patch with --enable-smartcard and --disable-smartcard and only disable ccid-card-emulated if nss not found. * add patch with description strings * s/libcaccard/libcacard/ in docs/ccid.txt v13-v14 changes: - support device_del/device_add on ccid-card-* and usb-ccid * usb-ccid: * lose card reference when card device deleted * check slot number and deny adding a slot if one is already added. * ccid-card-*: use qdev_simple_unplug_cb in both emulated and passthru ccid cards, the exitfn already takes care of triggering card removal in the usb dev. * libcacard: * remove double include of config-host.mak * add replay of card events to libcacard to support
[Qemu-devel] [PATCH 04/10] introduce libcacard/vscard_common.h
--- Signed-off-by: Alon Levy al...@redhat.com v20-v21 changes: (Jes Sorensen review) * license set to 2+ * long comment fixes, remove empty line at eof. * add reference to COPYING v19-v20 changes: * checkpatch.pl v15-v16 changes: Protocol change: * VSCMsgInit capabilities and magic * removed ReaderResponse, will use Error instead with code==VSC_SUCCESS. * adaded Flush and FlushComplete, remove Reconnect. * define VSCARD_MAGIC * added error code VSC_SUCCESS. Fixes: * update VSCMsgInit comment * fix message type enum * remove underscore from wrapping define * update copyright * updated comments. * Header comment updated * remove C++ style comment * fix comment for VSCMsgError * give names to enums in typedefs --- libcacard/vscard_common.h | 178 + 1 files changed, 178 insertions(+), 0 deletions(-) create mode 100644 libcacard/vscard_common.h diff --git a/libcacard/vscard_common.h b/libcacard/vscard_common.h new file mode 100644 index 000..bebd52d --- /dev/null +++ b/libcacard/vscard_common.h @@ -0,0 +1,178 @@ +/* Virtual Smart Card protocol definition + * + * This protocol is between a host using virtual smart card readers, + * and a client providing the smart cards, perhaps by emulating them or by + * access to real cards. + * + * Definitions for this protocol: + * Host - user of the card + * Client - owner of the card + * + * The current implementation passes the raw APDU's from 7816 and additionally + * contains messages to setup and teardown readers, handle insertion and + * removal of cards, negotiate the protocol via capabilities and provide + * for error responses. + * + * Copyright (c) 2011 Red Hat. + * + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later. + * See the COPYING.LIB file in the top-level directory. + */ + +#ifndef VSCARD_COMMON_H +#define VSCARD_COMMON_H + +#include stdint.h + +#define VERSION_MAJOR_BITS 11 +#define VERSION_MIDDLE_BITS 11 +#define VERSION_MINOR_BITS 10 + +#define MAKE_VERSION(major, middle, minor) \ + ((major (VERSION_MINOR_BITS + VERSION_MIDDLE_BITS)) \ + | (middle VERSION_MINOR_BITS) \ + | (minor)) + +/* + * IMPORTANT NOTE on VERSION + * + * The version below MUST be changed whenever a change in this file is made. + * + * The last digit, the minor, is for bug fix changes only. + * + * The middle digit is for backward / forward compatible changes, updates + * to the existing messages, addition of fields. + * + * The major digit is for a breaking change of protocol, presumably + * something that cannot be accomodated with the existing protocol. + */ + +#define VSCARD_VERSION MAKE_VERSION(0, 0, 2) + +typedef enum VSCMsgType { +VSC_Init = 1, +VSC_Error, +VSC_ReaderAdd, +VSC_ReaderRemove, +VSC_ATR, +VSC_CardRemove, +VSC_APDU, +VSC_Flush, +VSC_FlushComplete +} VSCMsgType; + +typedef enum VSCErrorCode { +VSC_SUCCESS = 0, +VSC_GENERAL_ERROR = 1, +VSC_CANNOT_ADD_MORE_READERS, +VSC_CARD_ALREAY_INSERTED, +} VSCErrorCode; + +#define VSCARD_UNDEFINED_READER_ID 0x +#define VSCARD_MINIMAL_READER_ID0 + +#define VSCARD_MAGIC (*(uint32_t *)VSCD) + +/* + * Header + * Each message starts with the header. + * type - message type + * reader_id - used by messages that are reader specific + * length - length of payload (not including header, i.e. zero for + * messages containing empty payloads) + */ +typedef struct VSCMsgHeader { +uint32_t type; +uint32_t reader_id; +uint32_t length; +uint8_tdata[0]; +} VSCMsgHeader; + +/* + * VSCMsgInit Client - Host + * Client sends it on connection, with its own capabilities. + * Host replies with VSCMsgInit filling in its capabilities. + * + * It is not meant to be used for negotiation, i.e. sending more then + * once from any side, but could be used for that in the future. + */ +typedef struct VSCMsgInit { +uint32_t magic; +uint32_t version; +uint32_t capabilities[1]; /* receiver must check length, + array may grow in the future*/ +} VSCMsgInit; + +/* + * VSCMsgError Client - Host + * This message is a response to any of: + * Reader Add + * Reader Remove + * Card Remove + * If the operation was successful then VSC_SUCCESS + * is returned, other wise a specific error code. + */ +typedef struct VSCMsgError { +uint32_t code; +} VSCMsgError; + +/* + * VSCMsgReaderAdd Client - Host + * Host replies with allocated reader id in VSCMsgError with code==SUCCESS. + * + * name - name of the reader on client side, UTF-8 encoded. Only used + * for client presentation (may be translated to the device presented to the + * guest), protocol wise only reader_id is important. + */ +typedef struct VSCMsgReaderAdd { +uint8_tname[0]; +} VSCMsgReaderAdd; + +/* + * VSCMsgReaderRemove Client - Host + * The client's reader has been removed. + */ +typedef
Re: [Qemu-devel] [PATCH V11 06/15] xen: Add the Xen platform pci device
On Wed, Mar 23, 2011 at 12:08, Alexander Graf ag...@suse.de wrote: diff --git a/hw/pc_piix.c b/hw/pc_piix.c index 6eff06e..417c456 100644 --- a/hw/pc_piix.c +++ b/hw/pc_piix.c @@ -121,6 +121,10 @@ static void pc_init1(ram_addr_t ram_size, pc_vga_init(pci_enabled? pci_bus: NULL); + if (xen_enabled()) { + pci_xen_platform_init(pci_bus); It probably makes sense to fold that function in here. That way you wouldn't need the entry in the stub file. Yes, I will do that. Thanks, -- Anthony PERARD
[Qemu-devel] [PATCH 07/10] libcacard: add vscclient
From: Robert Relyea rrel...@redhat.com client to talk to ccid-card-passthru and use smartcard on client to perform actual operations. v23-v24 changes: (Jes Sorensen review 2) * use qemu_socket instead of socket * use fprintf(stderr,..) for errors * remove unneccessary includes since using qemu_common.h --- libcacard/Makefile|7 +- libcacard/vscclient.c | 652 + 2 files changed, 658 insertions(+), 1 deletions(-) create mode 100644 libcacard/vscclient.c diff --git a/libcacard/Makefile b/libcacard/Makefile index 410fa1e..85e3376 100644 --- a/libcacard/Makefile +++ b/libcacard/Makefile @@ -12,6 +12,11 @@ endif QEMU_OBJS=$(QEMU_THREAD) $(oslib-obj-y) $(trace-obj-y) qemu-malloc.o qemu-timer-common.o +vscclient: $(libcacard-y) $(QEMU_OBJS) vscclient.o + $(call quiet-command,$(CC) $(libcacard_libs) -lrt -o $@ $^, LINK $(TARGET_DIR)$@) + +all: vscclient + clean: - rm -f *.o */*.o *.d */*.d *.a */*.a *~ */*~ + rm -f *.o */*.o *.d */*.d *.a */*.a *~ */*~ vscclient diff --git a/libcacard/vscclient.c b/libcacard/vscclient.c new file mode 100644 index 000..ce33f5a --- /dev/null +++ b/libcacard/vscclient.c @@ -0,0 +1,652 @@ +/* + * Tester for VSCARD protocol, client side. + * + * Can be used with ccid-card-passthru. + * + * Copyright (c) 2011 Red Hat. + * Written by Alon Levy. + * + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later. + * See the COPYING.LIB file in the top-level directory. + */ + +#include netdb.h + +#include qemu-common.h +#include qemu-thread.h +#include qemu_socket.h + +#include vscard_common.h + +#include vreader.h +#include vcard_emul.h +#include vevent.h + +int verbose; + +int sock; + +static void +print_byte_array( +uint8_t *arrBytes, +unsigned int nSize +) { +int i; +for (i = 0; i nSize; i++) { +printf(%02X , arrBytes[i]); +} +printf(\n); +} + +static void +print_usage(void) { +printf(vscclient [-c certname .. -e emul_args -d level%s] +host port\n, +#ifdef USE_PASSTHRU + -p); +printf( -p use passthrough mode\n); +#else + ); +#endif +vcard_emul_usage(); +} + +static QemuMutex write_lock; + +static int +send_msg( +VSCMsgType type, +uint32_t reader_id, +const void *msg, +unsigned int length +) { +int rv; +VSCMsgHeader mhHeader; + +qemu_mutex_lock(write_lock); + +if (verbose 10) { +printf(sending type=%d id=%d, len =%d (0x%x)\n, + type, reader_id, length, length); +} + +mhHeader.type = htonl(type); +mhHeader.reader_id = 0; +mhHeader.length = htonl(length); +rv = write(sock, mhHeader, sizeof(mhHeader)); +if (rv 0) { +/* Error */ +fprintf(stderr, write header error\n); +close(sock); +qemu_mutex_unlock(write_lock); +return 16; +} +rv = write(sock, msg, length); +if (rv 0) { +/* Error */ +fprintf(stderr, write error\n); +close(sock); +qemu_mutex_unlock(write_lock); +return 16; +} +qemu_mutex_unlock(write_lock); + +return 0; +} + +static VReader *pending_reader; +static QemuMutex pending_reader_lock; +static QemuCond pending_reader_condition; + +#define MAX_ATR_LEN 40 +static void * +event_thread(void *arg) +{ +unsigned char atr[MAX_ATR_LEN]; +int atr_len = MAX_ATR_LEN; +VEvent *event = NULL; +unsigned int reader_id; + + +while (1) { +const char *reader_name; + +event = vevent_wait_next_vevent(); +if (event == NULL) { +break; +} +reader_id = vreader_get_id(event-reader); +if (reader_id == VSCARD_UNDEFINED_READER_ID +event-type != VEVENT_READER_INSERT) { +/* ignore events from readers qemu has rejected */ +/* if qemu is still deciding on this reader, wait to see if need to + * forward this event */ +qemu_mutex_lock(pending_reader_lock); +if (!pending_reader || (pending_reader != event-reader)) { +/* wasn't for a pending reader, this reader has already been + * rejected by qemu */ +qemu_mutex_unlock(pending_reader_lock); +vevent_delete(event); +continue; +} +/* this reader hasn't been told it's status from qemu yet, wait for + * that status */ +while (pending_reader != NULL) { +qemu_cond_wait(pending_reader_condition, pending_reader_lock); +} +qemu_mutex_unlock(pending_reader_lock); +/* now recheck the id */ +reader_id = vreader_get_id(event-reader); +if (reader_id == VSCARD_UNDEFINED_READER_ID) { +/* this reader was rejected */ +vevent_delete(event); +continue; +} +/* reader was accepted, now forward the event */ +
[Qemu-devel] [PATCH 08/10] libcacard: add docs
From: Robert Relyea rrel...@redhat.com --- docs/libcacard.txt | 483 1 files changed, 483 insertions(+), 0 deletions(-) create mode 100644 docs/libcacard.txt diff --git a/docs/libcacard.txt b/docs/libcacard.txt new file mode 100644 index 000..5dee6fa --- /dev/null +++ b/docs/libcacard.txt @@ -0,0 +1,483 @@ +This file documents the CAC (Common Access Card) library in the libcacard +subdirectory. + +Virtual Smart Card Emulator + +This emulator is designed to provide emulation of actual smart cards to a +virtual card reader running in a guest virtual machine. The emulated smart +cards can be representations of real smart cards, where the necessary functions +such as signing, card removal/insertion, etc. are mapped to real, physical +cards which are shared with the client machine the emulator is running on, or +the cards could be pure software constructs. + +The emulator is structured to allow multiple replacable or additional pieces, +so it can be easily modified for future requirements. The primary envisioned +modifications are: + +1) The socket connection to the virtual card reader (presumably a CCID reader, +but other ISO-7816 compatible readers could be used). The code that handles +this is in vscclient.c. + +2) The virtual card low level emulation. This is currently supplied by using +NSS. This emulation could be replaced by implementations based on other +security libraries, including but not limitted to openssl+pkcs#11 library, +raw pkcs#11, Microsoft CAPI, direct opensc calls, etc. The code that handles +this is in vcard_emul_nss.c. + +3) Emulation for new types of cards. The current implementation emulates the +original DoD CAC standard with separate pki containers. This emulator lives in +cac.c. More than one card type emulator could be included. Other cards could +be emulated as well, including PIV, newer versions of CAC, PKCS #15, etc. + + +Replacing the Socket Based Virtual Reader Interface. + +The current implementation contains a replacable module vscclient.c. The +current vscclient.c implements a sockets interface to the virtual ccid reader +on the guest. CCID commands that are pertinent to emulation are passed +across the socket, and their responses are passed back along that same socket. +The protocol that vscclient uses is defined in vscard_common.h and connects +to a qemu ccid usb device. Since this socket runs as a client, vscclient.c +implements a program with a main entry. It also handles argument parsing for +the emulator. + +An application that wants to use the virtual reader can replace vscclient.c +with it's own implementation that connects to it's own CCID reader. The calls +that the CCID reader can call are: + + VReaderList * vreader_get_reader_list(); + + This function returns a list of virtual readers. These readers may map to + physical devices, or simulated devices depending on vcard the back end. Each + reader in the list should represent a reader to the virtual machine. Virtual + USB address mapping is left to the CCID reader front end. This call can be + made any time to get an updated list. The returned list is a copy of the + internal list that can be referenced by the caller without locking. This copy + must be freed by the caller with vreader_list_delete when it is no longer + needed. + + VReaderListEntry *vreader_list_get_first(VReaderList *); + + This function gets the first entry on the reader list. Along with + vreader_list_get_next(), vreader_list_get_first() can be used to walk the + reader list returned from vreader_get_reader_list(). VReaderListEntries are + part of the list themselves and do not need to be freed separately from the + list. If there are no entries on the list, it will return NULL. + + VReaderListEntry *vreader_list_get_next(VReaderListEntry *); + + This function gets the next entry in the list. If there are no more entries + it will return NULL. + + VReader * vreader_list_get_reader(VReaderListEntry *) + + This function returns the reader stored in the reader List entry. Caller gets + a new reference to a reader. The caller must free it's reference when it is + finished with vreader_free(). + + void vreader_free(VReader *reader); + + This function frees a reference to a reader. Reader's are reference counted + and are automatically deleted when the last reference is freed. + + void vreader_list_delete(VReaderList *list); + + This function frees the list, all the elements on the list, and all the + reader references held by the list. + + VReaderStatus vreader_power_on(VReader *reader, char *atr, int *len); + + This functions simulates a card power on. Virtual cards do not care about + the actual voltage and other physical parameters, but it does care that the + card is actually on or off. Cycling the card causes the card to reset. If + the caller provides enough space, vreader_power_on will return
[Qemu-devel] [PATCH 05/10] ccid: add passthru card device
The passthru ccid card is a device sitting on the usb-ccid bus and using a chardevice to communicate with a remote device using the VSCard protocol defined in libcacard/vscard_common.h Usage docs available in following patch in docs/ccid.txt Signed-off-by: Alon Levy al...@redhat.com --- Changes from v23-v24: * fixed double license line in header. Changes from v20-v21: (Jes Sorensen review) * add reference to COPYING in header * long comment reformatting Changes from v19-v20: * checkpatch.pl Changes from v18-v19: * add qdev.desc * remove .qdev.unplug (no hot unplug support for ccid bus) Changes from v16-v17: * fix wrong cast when receiving VSC_Error * ccid-card-passthru: force chardev user wakeup by sending Init see lengthy comment below. Changes from v15-v16: Behavioral changes: * return correct size * return error instead of assert if client sent too large ATR * don't assert if client sent too large a size, but add asserts for indices to buffer * reset vscard_in indices on chardev disconnect * handle init from client * error if no chardev supplied * use ntoh, hton * eradicate reader_id_t * remove Reconnect usage (removed from VSCARD protocol) * send VSC_SUCCESS on card insert/remove and reader add/remove Style fixes: * width of line fix * update copyright * remove old TODO's * update file header comment * use macros for debug levels * c++ style comment replacement * update copyright license * fix ATR size comment * fix whitespace in struct def * fix DPRINTF prefix * line width fix ccid-card-passthru: force chardev user wakeup by sending Init The problem: how to wakeup the user of the smartcard when the smartcard device is initialized? Long term solution: have a callback interface. This was done via the deprecated so called chardev ioctl interface. Short term solution: do a write. Specifically we write an Init message. And we change the client to send it's own Init message regardless of receiving this one. Additional Init messages will be regarded as acceptable, the first one received after connection establishment is the determining one wrt capabilities. --- Makefile.objs |2 +- hw/ccid-card-passthru.c | 340 +++ 2 files changed, 341 insertions(+), 1 deletions(-) create mode 100644 hw/ccid-card-passthru.c diff --git a/Makefile.objs b/Makefile.objs index 489a46b..744e1d3 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -200,7 +200,7 @@ hw-obj-$(CONFIG_APM) += pm_smbus.o apm.o hw-obj-$(CONFIG_DMA) += dma.o hw-obj-$(CONFIG_HPET) += hpet.o hw-obj-$(CONFIG_APPLESMC) += applesmc.o -hw-obj-$(CONFIG_SMARTCARD) += usb-ccid.o +hw-obj-$(CONFIG_SMARTCARD) += usb-ccid.o ccid-card-passthru.o # PPC devices hw-obj-$(CONFIG_OPENPIC) += openpic.o diff --git a/hw/ccid-card-passthru.c b/hw/ccid-card-passthru.c new file mode 100644 index 000..8506fed --- /dev/null +++ b/hw/ccid-card-passthru.c @@ -0,0 +1,340 @@ +/* + * CCID Passthru Card Device emulation + * + * Copyright (c) 2011 Red Hat. + * Written by Alon Levy. + * + * This work is licensed under the terms of the GNU GPL, version 2.1 or later. + * See the COPYING file in the top-level directory. + */ + +#include arpa/inet.h + +#include qemu-char.h +#include monitor.h +#include hw/ccid.h +#include libcacard/vscard_common.h + +#define DPRINTF(card, lvl, fmt, ...)\ +do {\ +if (lvl = card-debug) { \ +printf(ccid-card-passthru: fmt , ## __VA_ARGS__); \ +} \ +} while (0) + +#define D_WARN 1 +#define D_INFO 2 +#define D_MORE_INFO 3 +#define D_VERBOSE 4 + +/* TODO: do we still need this? */ +uint8_t DEFAULT_ATR[] = { +/* + * From some example somewhere + * 0x3B, 0xB0, 0x18, 0x00, 0xD1, 0x81, 0x05, 0xB1, 0x40, 0x38, 0x1F, 0x03, 0x28 + */ + +/* From an Athena smart card */ + 0x3B, 0xD5, 0x18, 0xFF, 0x80, 0x91, 0xFE, 0x1F, 0xC3, 0x80, 0x73, 0xC8, 0x21, + 0x13, 0x08 +}; + + +#define PASSTHRU_DEV_NAME ccid-card-passthru +#define VSCARD_IN_SIZE 65536 + +/* maximum size of ATR - from 7816-3 */ +#define MAX_ATR_SIZE40 + +typedef struct PassthruState PassthruState; + +struct PassthruState { +CCIDCardState base; +CharDriverState *cs; +uint8_t vscard_in_data[VSCARD_IN_SIZE]; +uint32_t vscard_in_pos; +uint32_t vscard_in_hdr; +uint8_t atr[MAX_ATR_SIZE]; +uint8_t atr_length; +uint8_t debug; +}; + +/* + * VSCard protocol over chardev + * This code should not depend on the card type. + */ + +static void ccid_card_vscard_send_msg(PassthruState *s, +VSCMsgType type, uint32_t reader_id, +const uint8_t *payload, uint32_t length) +{ +VSCMsgHeader scr_msg_header; + +scr_msg_header.type = htonl(type); +scr_msg_header.reader_id = htonl(reader_id); +scr_msg_header.length = htonl(length); +qemu_chr_write(s-cs, (uint8_t *)scr_msg_header,
[Qemu-devel] [PATCH 09/10] ccid: add ccid-card-emulated device
This devices uses libcacard (internal) to emulate a smartcard conforming to the CAC standard. It attaches to the usb-ccid bus. Usage instructions (example command lines) are in the following patch in docs/ccid.txt. It uses libcacard which uses nss, so it can work with both hw cards and certificates (files). Signed-off-by: Alon Levy al...@redhat.com --- changes from v20-v21: (Jes Sorenson review) * cosmetics * use qemu-thread and qemu_malloc/qemu_free changes from v19-v20: * checkpatch.pl changes from v18-v19: * add qdev.desc * backend: drop the enumeration property, back to using a string one. changes from v16-v17: * use PROP_TYPE_ENUM for backend changes from v15-v16: * fix error reporting in initfn * bump copyright year * update copyright license changes from v1: * remove stale comments, use only c-style comments * bugfix, forgot to set recv_len * change reader name to 'Virtual Reader' --- Makefile.objs |1 + hw/ccid-card-emulated.c | 595 +++ 2 files changed, 596 insertions(+), 0 deletions(-) create mode 100644 hw/ccid-card-emulated.c diff --git a/Makefile.objs b/Makefile.objs index f513ffa..389b336 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -201,6 +201,7 @@ hw-obj-$(CONFIG_DMA) += dma.o hw-obj-$(CONFIG_HPET) += hpet.o hw-obj-$(CONFIG_APPLESMC) += applesmc.o hw-obj-$(CONFIG_SMARTCARD) += usb-ccid.o ccid-card-passthru.o +hw-obj-$(CONFIG_SMARTCARD_NSS) += ccid-card-emulated.o # PPC devices hw-obj-$(CONFIG_OPENPIC) += openpic.o diff --git a/hw/ccid-card-emulated.c b/hw/ccid-card-emulated.c new file mode 100644 index 000..0b07184 --- /dev/null +++ b/hw/ccid-card-emulated.c @@ -0,0 +1,595 @@ +/* + * CCID Card Device. Emulated card. + * + * Copyright (c) 2011 Red Hat. + * Written by Alon Levy. + * + * This code is licenced under the GNU LGPL, version 2 or later. + */ + +/* + * It can be used to provide access to the local hardware in a non exclusive + * way, or it can use certificates. It requires the usb-ccid bus. + * + * Usage 1: standard, mirror hardware reader+card: + * qemu .. -usb -device usb-ccid -device ccid-card-emulated + * + * Usage 2: use certificates, no hardware required + * one time: create the certificates: + * for i in 1 2 3; do + * certutil -d /etc/pki/nssdb -x -t CT,CT,CT -S -s CN=user$i -n user$i + * done + * qemu .. -usb -device usb-ccid \ + * -device ccid-card-emulated,cert1=user1,cert2=user2,cert3=user3 + * + * If you use a non default db for the certificates you can specify it using + * the db parameter. + */ + +#include eventt.h +#include vevent.h +#include vreader.h +#include vcard_emul.h + +#include qemu-thread.h +#include qemu-char.h +#include monitor.h +#include hw/ccid.h + +#define DPRINTF(card, lvl, fmt, ...) \ +do {\ +if (lvl = card-debug) {\ +printf(ccid-card-emul: %s: fmt , __func__, ## __VA_ARGS__);\ +} \ +} while (0) + +#define EMULATED_DEV_NAME ccid-card-emulated + +#define BACKEND_NSS_EMULATED_NAME nss-emulated +#define BACKEND_CERTIFICATES_NAME certificates + +enum { +BACKEND_NSS_EMULATED = 1, +BACKEND_CERTIFICATES +}; + +#define DEFAULT_BACKEND BACKEND_NSS_EMULATED + +typedef struct EmulatedState EmulatedState; + +enum { +EMUL_READER_INSERT = 0, +EMUL_READER_REMOVE, +EMUL_CARD_INSERT, +EMUL_CARD_REMOVE, +EMUL_GUEST_APDU, +EMUL_RESPONSE_APDU, +EMUL_ERROR, +}; + +static const char *emul_event_to_string(uint32_t emul_event) +{ +switch (emul_event) { +case EMUL_READER_INSERT: +return EMUL_READER_INSERT; +case EMUL_READER_REMOVE: +return EMUL_READER_REMOVE; +case EMUL_CARD_INSERT: +return EMUL_CARD_INSERT; +case EMUL_CARD_REMOVE: +return EMUL_CARD_REMOVE; +case EMUL_GUEST_APDU: +return EMUL_GUEST_APDU; +case EMUL_RESPONSE_APDU: +return EMUL_RESPONSE_APDU; +case EMUL_ERROR: +return EMUL_ERROR; +} +return UNKNOWN; +} + +typedef struct EmulEvent { +QSIMPLEQ_ENTRY(EmulEvent) entry; +union { +struct { +uint32_t type; +} gen; +struct { +uint32_t type; +uint64_t code; +} error; +struct { +uint32_t type; +uint32_t len; +uint8_t data[]; +} data; +} p; +} EmulEvent; + +#define MAX_ATR_SIZE 40 +struct EmulatedState { +CCIDCardState base; +uint8_t debug; +char*backend_str; +uint32_t backend; +char*cert1; +char*cert2; +char*cert3; +char*db; +uint8_t atr[MAX_ATR_SIZE]; +uint8_t atr_length; +QSIMPLEQ_HEAD(event_list, EmulEvent) event_list; +QemuMutex event_list_mutex; +VReader *reader; +QSIMPLEQ_HEAD(guest_apdu_list, EmulEvent) guest_apdu_list; +QemuMutex vreader_mutex; /* and guest_apdu_list mutex */ +QemuMutex handle_apdu_mutex; +QemuCond handle_apdu_cond; +int pipe[2]; +int quit_apdu_thread; +
Re: [Xen-devel] Re: [Qemu-devel] [PATCH V11 13/15] xen: Initialize event channels and io rings
On Wed, Mar 23, 2011 at 12:36, Alexander Graf ag...@suse.de wrote: +/* Compatibility with older version */ +#if __XEN_LATEST_INTERFACE_VERSION__ 0x0003020a +# define xen_vcpu_eport(shared_page, i) \ + (shared_page-vcpu_iodata[i].vp_eport) +# define xen_vcpu_ioreq(shared_page, vcpu) \ + (shared_page-vcpu_iodata[vcpu].vp_ioreq) +# define FMT_ioreq_size PRIx64 +#else +# define xen_vcpu_eport(shared_page, i) \ + (shared_page-vcpu_ioreq[i].vp_eport) +# define xen_vcpu_ioreq(shared_page, vcpu) \ + (shared_page-vcpu_ioreq[vcpu]) Could you please change these to static inline functions? Yes, I will do that. Regards, -- Anthony PERARD
[Qemu-devel] Re: [PATCH v6 0/4] piix_pci: optimize irq data path
On Tue, Mar 29, 2011 at 12:05:26AM +0900, Isaku Yamahata wrote: Here is v6 which fixed piix3_set_irq_pic(). 4/4 needs more extensive tests. So please feel free to pick it up now or drop it for now. Could you pls clarify how did you test and which patches? patch description: This patch series optimizes irq data path of piix_pci. So far piix3 tracks each pirq level and checks whether a given pic pins is asserted by seeing if each pirq is mapped into the pic pin. This is independent on irq routing, but data path is on slow path. Given that irq routing is rarely changed and asserting pic pins is on data path, the path that asserts pic pins should be optimized and chainging irq routing should be on slow path. The new behavior with this patch series is to use bitmap which is addressed by pirq and pic pins with a given irq routing. When pirq is asserted, the bitmap is set and see if the pic pins is asserted by checking the bitmaps. When irq routing is changed, rebuild the bitmap and re-assert pic pins. Changes v5 - v6: - fixed piix3_set_irq_pic() Changes v4 - v5: - typo Changes v3 - v4: - use pirq, pci_intx instead of irq_num in piix_pci.c - use symbolic constant PIC_NUM_PINS - introduced new patch 4/4 which cleans up a bit. Changes v2 - v3: - s/dummy_for_save_load_compat/pci_irq_levels_vmstate/g - move down unused member of pci_irq_levels_vmstate in the structure for cache efficiency Changes v1 - v2: - addressed review comments. Isaku Yamahata (4): pci: add accessor function to get irq levels piix_pci: eliminate PIIX3State::pci_irq_levels piix_pci: optimize set irq path piix_pci: load path clean up hw/pci.c |7 +++ hw/pci.h |1 + hw/piix_pci.c | 129 ++--- 3 files changed, 112 insertions(+), 25 deletions(-)
Re: [Qemu-devel] [PATCH] cirrus_vga: Remove unneeded reset
On Mon, Mar 28, 2011 at 06:20:15PM +0200, Stefan Weil wrote: Am 28.03.2011 11:24, schrieb Isaku Yamahata: On Mon, Mar 28, 2011 at 11:21:23AM +0200, Markus Armbruster wrote: Isaku Yamahata yamah...@valinux.co.jp writes: On Mon, Mar 28, 2011 at 07:18:04AM +0200, Stefan Weil wrote: Am 28.03.2011 04:17, schrieb Isaku Yamahata: [...] On Sat, Mar 26, 2011 at 10:53:09PM +0100, Stefan Weil wrote: cirrus_reset is also called by the pci framework, so there is no need to call it in cirrus_init_common. Cc: Michael S. Tsirkinm...@redhat.com Signed-off-by: Stefan Weilw...@mail.berlios.de [...] I tested the new code with isa pc, too. In gdb, I could see that it also calls cirrus_reset twice. But isa pc is broken since the switch to sea bios, so obviously isa is an unmaintained part of qemu. Even with bochs bios, it no longer works, so it is broken at least twice. Ah, I see. The the second reset is called not via pci reset framework, but qemu reset framework. So removing the above reset call makes sense. It would be another patch to make use of pci reset framework. Then the proposed commit message's claim cirrus_reset() is called by the pci framework is incorrect, isn't it? Yes, incorrect. The commit message should be fixed. The code change itself looks correct. For current qemu it is correct, or is there a working configuration with isa cirrus? I asked that question on #qemu but did not get an answer (Anthony replied that isa was broken long ago). This was the reason why I wrote the commit text as it is. I don't mind if the committer adds more descriptive text, but the main focus should be fixing isa emulation. I also noticed that some more emulations obviously also include redundant reset calls. These should be fixed, too. *I tweaked the commit log a bit to make everyone happy and applied that. Thanks!
Re: [Qemu-devel] [PATCH v24 00/10] usb-ccid
On Mon, Mar 28, 2011 at 06:11:04PM +0200, Alon Levy wrote: Sorry, please ignore this mail. This patchset adds three new devices, usb-ccid, ccid-card-passthru and ccid-card-emulated, providing a CCID bus, a simple passthru protocol implementing card requiring a client, and a standalone emulated card. It also introduces a new directory libcaccard with CAC card emulation, CAC is a type of ISO 7816 smart card. Tree for pull: git://anongit.freedesktop.org/~alon/qemu usb_ccid.v23 v23-v24 changes: * libcacard: = changed patches: (that need re-review) * 6 - libcacard: initial commit * 7 - libcacard: add vscclient * 5 - ccid: add passthru card device * drop libcacard add passthru patch, not ready, not used. * remove unrequired includes * use stderr in vscclient for printing errors * cosmetic fixes v22-v23 changes: * libcacard * configure fixes: (reported by Stefan Hajnoczi) * test a = b, not a == b (second isn't portable) * quote $source_path in case it contains spaces - this doesn't really help since there are many other places that need similar fixes, not introduced by this patch. v21-v22 changes: * libcacard: * fix configure to not link libcacard if nss not found (reported by Stefan Hajnoczi) * fix vscclient linkage with simpletrace backend (reported by Stefan Hajnoczi) * card_7816.c: add missing break in ERROR_DATA_NOT_FOUND (reported by William van de Velde) v20-v21 changes: * all: cosmetics * libcacard, ccid-card-passthru: * use qemu-{malloc,free} and qemu-thread, error_report * libcacard: * split to multiple patches v19-v20 changes: * checkpatch.pl. Here are the remaining errors with explanation: * ignored 5 macro errors of the type ERROR: Macros with complex values should be enclosed in parenthesis because fixing them breaks current code, if it really bothers someone I can fix it. * four of them are in libcacard/card_7816t.h: /* give the subfields a unified look */ .. #define a_cla a_header-ah_cla /* class */ #define a_ins a_header-ah_ins /* instruction */ #define a_p1 a_header-ah_p1 /* parameter 1 */ #define a_p2 a_header-ah_p2 /* parameter 2 */ * and the fifth: #4946: FILE: libcacard/vcardt.h:31: +#define VCARD_ATR_PREFIX(size) 0x3b, 0x66+(size), 0x00, 0xff, \ + 'V', 'C', 'A', 'R', 'D', '_' * Ignored this warning since I couldn't figure it out, and it's a test file: WARNING: externs should be avoided in .c files #2343: FILE: libcacard/link_test.c:7: +VCardStatus cac_card_init(const char *flags, VCard *card, v18-v19 changes: * more merges, down to a single digit number of patches. * drop enumeration property, use string. * rebased (trivial) v17-v18 changes: * merge vscard_common.h patches. * actually provide a tree to pull. v16-v17 changes: * merged all the v15-v16 patches * merged some more wherever it was easy (all same file commits). * added signed off by to first four patches * ccid.h: added copyright, removed underscore in defines, and replaced non C89 comments v15-v16 changes: * split vscard_common introducing patch for ease of review * sum of commit logs for the v15-v16 commits: (whitespace fixes removed for space, see original commit messages in later patches) * usb-ccid: * fix abort on client answer after card remove * enable migration * remove side affect code from asserts * return consistent self-powered state * mask out reserved bits in ccid_set_parameters * add missing abRFU in SetParameters (no affect on linux guest) * vscard_common.h protocol change: * VSCMsgInit capabilities and magic * removed ReaderResponse, will use Error instead with code==VSC_SUCCESS. * added Flush and FlushComplete, remove Reconnect. * define VSCARD_MAGIC * added error code VSC_SUCCESS. * ccid-card-passthru * return correct size * return error instead of assert if client sent too large ATR * don't assert if client sent too large a size, but add asserts for indices to buffer * reset vscard_in indices on chardev disconnect * handle init from client * error if no chardev supplied * use ntoh, hton * eradicate reader_id_t * remove Reconnect usage (removed from VSCARD protocol) * send VSC_SUCCESS on card insert/remove and reader add/remove * ccid-card-emulated * fix error reporting in initfn v14-v15 changes: * add patch with --enable-smartcard and --disable-smartcard and only disable ccid-card-emulated if nss not found. * add patch with description strings * s/libcaccard/libcacard/ in docs/ccid.txt v13-v14 changes: - support device_del/device_add on ccid-card-* and usb-ccid * usb-ccid: * lose card reference when card device deleted * check slot number and deny adding a slot if one is already added. * ccid-card-*: use qdev_simple_unplug_cb in both
[Qemu-devel] Re: [RFC][PATCH v1 05/12] qapi: fix handling for null-return async callbacks
On 03/28/2011 11:47 AM, Luiz Capitulino wrote: On Fri, 25 Mar 2011 16:22:16 -0500 Anthony Liguorialigu...@us.ibm.com wrote: On 03/25/2011 02:47 PM, Michael Roth wrote: Async commands like 'guest-ping' have NULL retvals. Handle these by inserting an empty dictionary in the response's return field. Signed-off-by: Michael Rothmdr...@linux.vnet.ibm.com --- qmp-core.c |5 - 1 files changed, 4 insertions(+), 1 deletions(-) diff --git a/qmp-core.c b/qmp-core.c index e33f7a4..9f3d182 100644 --- a/qmp-core.c +++ b/qmp-core.c @@ -922,9 +922,12 @@ void qmp_async_complete_command(QmpCommandState *cmd, QObject *retval, Error *er rsp = qdict_new(); if (err) { qdict_put_obj(rsp, error, error_get_qobject(err)); -} else { +} else if (retval) { qobject_incref(retval); qdict_put_obj(rsp, return, retval); +} else { +/* add empty return dict, this is the standard for NULL returns */ +qdict_put_obj(rsp, return, QOBJECT(qdict_new())); Luiz, I know we decided to return empty dicts because it lets us extend things better, but did we want to rule out the use of a 'null' return value entirely? For asynchronous commands you mean? No we didn't. No, nothing to do with asynchronous commands. Just in general. The question is, is it legal for a command to return 'null'. It's certain valid JSON, but is it valid QMP? Regards, Anthony Liguori *iirc*, what happens today is that no command using this api is truly async, for two reasons. First, changing from sync to async can break clients (that happened to query-balloon). Second, although I can't remember the exact details, the api that exists in the tree today is limited. But for a new thing, like QAPI, having different semantics for async commands seems the right thing to be done (ie. delaying the response). For a command like this, I can't imagine ever wanting to extend the return value... Regards, Anthony Liguori } if (cmd-tag) { qdict_put_obj(rsp, tag, cmd-tag);
Re: [Qemu-devel] Re: [RFC][PATCH v1 00/11] QEMU Guest Agent: QMP-based host/guest communication (virtagent)
On 03/25/2011 05:36 PM, Michael Roth wrote: Basically just need a way to pull whatever is stored in the response qdict's return field out without specifying the QTYPE in advance... which exists in qdict.c:qdict_get_obj(), it's just not currently exposed to outside callers. Just use qdict_get()--but I still don't understand what the problem is. Argh! So that solves the problem completely. Thanks :) Yeah, the asymmetry is confusing. qdict_put() takes anything but a QObject yet qdict_get() returns a QObject. qdict_put_obj() takes a QObject and qdict_get_obj() is static. Regards, Anthony Liguori Regards, Anthony Liguori
[Qemu-devel] Re: [RFC][PATCH v1 05/12] qapi: fix handling for null-return async callbacks
On Mon, 28 Mar 2011 12:01:16 -0500 Anthony Liguori aligu...@us.ibm.com wrote: On 03/28/2011 11:47 AM, Luiz Capitulino wrote: On Fri, 25 Mar 2011 16:22:16 -0500 Anthony Liguorialigu...@us.ibm.com wrote: On 03/25/2011 02:47 PM, Michael Roth wrote: Async commands like 'guest-ping' have NULL retvals. Handle these by inserting an empty dictionary in the response's return field. Signed-off-by: Michael Rothmdr...@linux.vnet.ibm.com --- qmp-core.c |5 - 1 files changed, 4 insertions(+), 1 deletions(-) diff --git a/qmp-core.c b/qmp-core.c index e33f7a4..9f3d182 100644 --- a/qmp-core.c +++ b/qmp-core.c @@ -922,9 +922,12 @@ void qmp_async_complete_command(QmpCommandState *cmd, QObject *retval, Error *er rsp = qdict_new(); if (err) { qdict_put_obj(rsp, error, error_get_qobject(err)); -} else { +} else if (retval) { qobject_incref(retval); qdict_put_obj(rsp, return, retval); +} else { +/* add empty return dict, this is the standard for NULL returns */ +qdict_put_obj(rsp, return, QOBJECT(qdict_new())); Luiz, I know we decided to return empty dicts because it lets us extend things better, but did we want to rule out the use of a 'null' return value entirely? For asynchronous commands you mean? No we didn't. No, nothing to do with asynchronous commands. Just in general. The question is, is it legal for a command to return 'null'. It's certain valid JSON, but is it valid QMP? No, it's not valid. Regards, Anthony Liguori *iirc*, what happens today is that no command using this api is truly async, for two reasons. First, changing from sync to async can break clients (that happened to query-balloon). Second, although I can't remember the exact details, the api that exists in the tree today is limited. But for a new thing, like QAPI, having different semantics for async commands seems the right thing to be done (ie. delaying the response). For a command like this, I can't imagine ever wanting to extend the return value... Regards, Anthony Liguori } if (cmd-tag) { qdict_put_obj(rsp, tag, cmd-tag);
[Qemu-devel] [PATCH 03/10] usb-ccid: add CCID bus
A CCID device is a smart card reader. It is a USB device, defined at [1]. This patch introduces the usb-ccid device that is a ccid bus. Next patches will introduce two card types to use it, a passthru card and an emulated card. [1] http://www.usb.org/developers/devclass_docs/DWG_Smart-Card_CCID_Rev110. Signed-off-by: Alon Levy al...@redhat.com --- changes from v20-v21: (Jes Sorenson review) * cosmetic changes - fix multi line comments. * reorder fields in USBCCIDState * add reference to COPYING * add --enable-smartcard and --disable-smartcard here (moved from last patch) changes from v19-v20: * checkpatch.pl changes from v18-v19: * merged: ccid.h: add copyright, fix define and remove non C89 comments * add qdev.desc changes from v15-v16: Behavioral changes: * fix abort on client answer after card remove * enable migration * remove side affect code from asserts * return consistent self-powered state * mask out reserved bits in ccid_set_parameters * add missing abRFU in SetParameters (no affect on linux guest) whitefixes / comments / consts defines: * remove stale comment * remove ccid_print_pending_answers if no DEBUG_CCID * replace printf's with DPRINTF, remove DEBUG_CCID, add verbosity defines * use error_report * update copyright (most of the code is not original) * reword known bug comment * add missing closing quote in comment * add missing whitespace on one line * s/CCID_SetParameter/CCID_SetParameters/ * add comments * use define for max packet size Comment for return consistent self-powered state: the Configuration Descriptor bmAttributes claims we are self powered, but we were returning not self powered to USB_REQ_GET_STATUS control message. In practice, this message is not sent by a linux 2.6.35.10-74.fc14.x86_64 guest (not tested on other guests), unless you issue lsusb -v as root (for example). --- Makefile.objs |1 + configure | 11 + hw/ccid.h | 59 +++ hw/usb-ccid.c | 1419 + 4 files changed, 1490 insertions(+), 0 deletions(-) create mode 100644 hw/ccid.h create mode 100644 hw/usb-ccid.c diff --git a/Makefile.objs b/Makefile.objs index 1fa7a29..489a46b 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -200,6 +200,7 @@ hw-obj-$(CONFIG_APM) += pm_smbus.o apm.o hw-obj-$(CONFIG_DMA) += dma.o hw-obj-$(CONFIG_HPET) += hpet.o hw-obj-$(CONFIG_APPLESMC) += applesmc.o +hw-obj-$(CONFIG_SMARTCARD) += usb-ccid.o # PPC devices hw-obj-$(CONFIG_OPENPIC) += openpic.o diff --git a/configure b/configure index 5a5827f..159549d 100755 --- a/configure +++ b/configure @@ -175,6 +175,7 @@ trace_backend=nop trace_file=trace spice= rbd= +smartcard= # parse CC options first for opt do @@ -724,6 +725,10 @@ for opt do ;; --enable-rbd) rbd=yes ;; + --disable-smartcard) smartcard=no + ;; + --enable-smartcard) smartcard=yes + ;; *) echo ERROR: unknown option $opt; show_help=yes ;; esac @@ -921,6 +926,8 @@ echoDefault:trace-pid echo --disable-spice disable spice echo --enable-spice enable spice echo --enable-rbd enable building the rados block device (rbd) +echo --disable-smartcard disable smartcard support +echo --enable-smartcard enable smartcard support echo echo NOTE: The object files are built at the place where configure is launched exit 1 @@ -2822,6 +2829,10 @@ if test $spice = yes ; then echo CONFIG_SPICE=y $config_host_mak fi +if test $smartcard = yes ; then + echo CONFIG_SMARTCARD=y $config_host_mak +fi + # XXX: suppress that if [ $bsd = yes ] ; then echo CONFIG_BSD=y $config_host_mak diff --git a/hw/ccid.h b/hw/ccid.h new file mode 100644 index 000..dbfc13c --- /dev/null +++ b/hw/ccid.h @@ -0,0 +1,59 @@ +/* + * CCID Passthru Card Device emulation + * + * Copyright (c) 2011 Red Hat. + * Written by Alon Levy. + * + * This code is licenced under the GNU LGPL, version 2 or later. + */ + +#ifndef CCID_H +#define CCID_H + +#include qdev.h + +typedef struct CCIDCardState CCIDCardState; +typedef struct CCIDCardInfo CCIDCardInfo; + +/* + * state of the CCID Card device (i.e. hw/ccid-card-*.c) + */ +struct CCIDCardState { +DeviceState qdev; +uint32_tslot; /* For future use with multiple slot reader. */ +}; + +/* + * callbacks to be used by the CCID device (hw/usb-ccid.c) to call + * into the smartcard device (hw/ccid-card-*.c) + */ +struct CCIDCardInfo { +DeviceInfo qdev; +void (*print)(Monitor *mon, CCIDCardState *card, int indent); +const uint8_t *(*get_atr)(CCIDCardState *card, uint32_t *len); +void (*apdu_from_guest)(CCIDCardState *card, +const uint8_t *apdu, +uint32_t len); +int (*exitfn)(CCIDCardState *card); +int (*initfn)(CCIDCardState *card); +}; + +/* + * API for smartcard calling the CCID device (used by hw/ccid-card-*.c) + */ +void
Re: [Qemu-devel] Re: [RFC][PATCH v1 05/12] qapi: fix handling for null-return async callbacks
On 03/28/2011 12:06 PM, Luiz Capitulino wrote: On Mon, 28 Mar 2011 12:01:16 -0500 Anthony Liguorialigu...@us.ibm.com wrote: On 03/28/2011 11:47 AM, Luiz Capitulino wrote: On Fri, 25 Mar 2011 16:22:16 -0500 Anthony Liguorialigu...@us.ibm.com wrote: On 03/25/2011 02:47 PM, Michael Roth wrote: Async commands like 'guest-ping' have NULL retvals. Handle these by inserting an empty dictionary in the response's return field. Signed-off-by: Michael Rothmdr...@linux.vnet.ibm.com --- qmp-core.c |5 - 1 files changed, 4 insertions(+), 1 deletions(-) diff --git a/qmp-core.c b/qmp-core.c index e33f7a4..9f3d182 100644 --- a/qmp-core.c +++ b/qmp-core.c @@ -922,9 +922,12 @@ void qmp_async_complete_command(QmpCommandState *cmd, QObject *retval, Error *er rsp = qdict_new(); if (err) { qdict_put_obj(rsp, error, error_get_qobject(err)); -} else { +} else if (retval) { qobject_incref(retval); qdict_put_obj(rsp, return, retval); +} else { +/* add empty return dict, this is the standard for NULL returns */ +qdict_put_obj(rsp, return, QOBJECT(qdict_new())); Luiz, I know we decided to return empty dicts because it lets us extend things better, but did we want to rule out the use of a 'null' return value entirely? For asynchronous commands you mean? No we didn't. No, nothing to do with asynchronous commands. Just in general. The question is, is it legal for a command to return 'null'. It's certain valid JSON, but is it valid QMP? No, it's not valid. Do we have a reason for this? Regards, Anthony Liguori
[Qemu-devel] Re: [PATCH 3/4] Introduce machine state
On Mon, Mar 28, 2011 at 10:25 AM, Paolo Bonzini pbonz...@redhat.com wrote: On 03/26/2011 11:28 PM, Blue Swirl wrote: Move generic machine state to machine-state.h, adjust users. What's the distinction between vm state and machine state? Machine state or configuration is visible to guest, for example number of CPUs. Machine level actions are also guest visible, for example hotplugging. VM state should be invisible (except for PV devices). Perhaps your vm state is more appropriately called emulator state (i.e. sits between host and vm), and machine state is actually vm state? I agree that the names are not so great. But 'emulator' is a bit generic. About 'machine', I was thinking about -M switch to specify the emulated machine type. BTW, uuid should be in machine state rather than (your definition of) vm state. Right. About patch sequencing, it should be possible to avoid most of the changes until the last patch set, by adding for example #include vm-state.h to sysemu.h when vm-state.h is introduced. The last patch, which removes sysemu.h, would adjust all sysemu.h users at once. If the general view is that this patch set goes to the right direction, the next step after machine/vm state would be to introduce generic VCPU state, probably based on cpus.[ch].
Re: [Qemu-devel] [PATCH 14/17] s390x: Implement opcode helpers
On 03/24/2011 06:29 PM, Peter Maydell wrote: On 24 March 2011 15:58, Alexander Grafag...@suse.de wrote: This is more random comments in passing than a thorough review; sorry. +#if HOST_LONG_BITS == 64 defined(__GNUC__) +/* assuming 64-bit hosts have __uint128_t */ +__uint128_t dividend = (((__uint128_t)env-regs[r1]) 64) | + (env-regs[r1+1]); +__uint128_t quotient = dividend / divisor; +env-regs[r1+1] = quotient; +__uint128_t remainder = dividend % divisor; +env-regs[r1] = remainder; +#else +/* 32-bit hosts would need special wrapper functionality - just abort if + we encounter such a case; it's very unlikely anyways. */ +cpu_abort(env, 128 - 64/64 division not implemented\n); +#endif ...I'm still using a 32 bit system :-) +/* condition codes for binary FP ops */ +static uint32_t set_cc_f32(float32 v1, float32 v2) +{ +if (float32_is_any_nan(v1) || float32_is_any_nan(v2)) { +return 3; +} else if (float32_eq(v1, v2,env-fpu_status)) { +return 0; +} else if (float32_lt(v1, v2,env-fpu_status)) { +return 1; +} else { +return 2; +} +} Can you not use float32_compare_quiet() (returns a value telling you if it's less/equal/greater/unordered)? If not, needs a comment saying why you need to do it the hard way. I just checked the macros there and it looks like float32_compare_quiet returns eq when both numbers are NaN. We would still have to convert from the return value from that over to a CC value. I honestly don't see any benefit - the code doesn't become cleaner or smaller. int float32_compare_quiet( float32 a, float32 b STATUS_PARAM ) { if (isless(a, b)) { return float_relation_less; } else if (a == b) { return float_relation_equal; } else if (isgreater(a, b)) { return float_relation_greater; } else { return float_relation_unordered; } } so static uint32_t set_cc_f32(float32 v1, float32 v2) { if (float32_is_any_nan(v1) || float32_is_any_nan(v2)) { return 3; } else if (float32_eq(v1, v2, env-fpu_status)) { return 0; } else if (float32_lt(v1, v2, env-fpu_status)) { return 1; } else { return 2; } } would become static uint32_t set_cc_f32(float32 v1, float32 v2) { int r; if (float32_is_any_nan(v1) || float32_is_any_nan(v2)) { return 3; } r = float32_compare_quiet(v1, v2, env-fpu_status); switch (r) { case float_relation_equal: return 0; case float_relation_less: return 1; default: return 2; } } +/* negative absolute of 32-bit float */ +uint32_t HELPER(lcebr)(uint32_t f1, uint32_t f2) +{ +env-fregs[f1].l.upper = float32_sub(float32_zero, env-fregs[f2].l.upper, +env-fpu_status); +return set_cc_nz_f32(env-fregs[f1].l.upper); +} Google suggests this is wrong: http://publib.boulder.ibm.com/cgi-bin/bookmgr/BOOKS/DZ9AR006/19.4.10?SHELF=DT=19990630131355CASE= says for lcebr that: The sign is inverted for any operand, including a QNaN or SNaN, without causing an arithmetic exception. but float32_sub will raise exceptions for NaNs. You want float32_chs() (and similarly for the other types). Ah, nice :). Thanks! +/* convert 64-bit float to 128-bit float */ +uint32_t HELPER(lcxbr)(uint32_t f1, uint32_t f2) Wrong comment? Looks like another invert-sign op from the online POO. Yup, wrong comment and the same as above for chs :). +/* 128-bit FP compare RR */ +uint32_t HELPER(cxbr)(uint32_t f1, uint32_t f2) +{ +CPU_QuadU v1; +v1.ll.upper = env-fregs[f1].ll; +v1.ll.lower = env-fregs[f1 + 2].ll; +CPU_QuadU v2; +v2.ll.upper = env-fregs[f2].ll; +v2.ll.lower = env-fregs[f2 + 2].ll; +if (float128_is_any_nan(v1.q) || float128_is_any_nan(v2.q)) { +return 3; +} else if (float128_eq(v1.q, v2.q,env-fpu_status)) { +return 0; +} else if (float128_lt(v1.q, v2.q,env-fpu_status)) { +return 1; +} else { +return 2; +} +} float128_compare_quiet() again? See above :) +/* convert 32-bit float to 64-bit int */ +uint32_t HELPER(cgebr)(uint32_t r1, uint32_t f2, uint32_t m3) +{ +float32 v2 = env-fregs[f2].l.upper; +set_round_mode(m3); +env-regs[r1] = float32_to_int64(v2,env-fpu_status); +return set_cc_nz_f32(v2); +} Should this really be permanently setting the rounding mode for future instructions as well as for the op it does itself? IIUC every op that does rounding sets the rounding mode, no? +/* load 32-bit FP zero */ +void HELPER(lzer)(uint32_t f1) +{ +env-fregs[f1].l.upper = float32_zero; +} Surely this is trivial enough to inline rather than calling a helper function... Lots of the FPU code could use inlining. The CC stuff does too. For now, I kept things the way Uli wrote them :). +/* load 128-bit FP zero */ +void HELPER(lzxr)(uint32_t f1) +{ +CPU_QuadU x; +