[Qemu-devel] QEMU API for modelling of user's hardware

2011-03-28 Thread wzab
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

2011-03-28 Thread Paolo Bonzini

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

2011-03-28 Thread Paolo Bonzini

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

2011-03-28 Thread Jes Sorensen
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

2011-03-28 Thread Jes Sorensen
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

2011-03-28 Thread Alexander Graf

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

2011-03-28 Thread Markus Armbruster
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

2011-03-28 Thread 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.
-- 
yamahata



Re: [Qemu-devel] Re: KVM call agenda for Jan 25

2011-03-28 Thread Kevin Wolf
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

2011-03-28 Thread Alexander Graf

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

2011-03-28 Thread Peter Maydell
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

2011-03-28 Thread Paolo Bonzini

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

2011-03-28 Thread Paolo Bonzini

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

2011-03-28 Thread Juan Quintela
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

2011-03-28 Thread Alexander Graf

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

2011-03-28 Thread Juan Quintela
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

2011-03-28 Thread Isaku Yamahata
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)

2011-03-28 Thread Alexander Graf

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

2011-03-28 Thread Jes Sorensen
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

2011-03-28 Thread Michael S. Tsirkin
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

2011-03-28 Thread Kevin Wolf
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

2011-03-28 Thread Jes Sorensen
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

2011-03-28 Thread Jes Sorensen
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

2011-03-28 Thread Jes Sorensen
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

2011-03-28 Thread Jes Sorensen
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

2011-03-28 Thread Jes Sorensen
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

2011-03-28 Thread Alon Levy
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

2011-03-28 Thread Alon Levy
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

2011-03-28 Thread Yongjie Ren
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

2011-03-28 Thread Yongjie Ren
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

2011-03-28 Thread Lluís
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

2011-03-28 Thread Alon Levy
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

2011-03-28 Thread Lluís
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

2011-03-28 Thread Jes Sorensen
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

2011-03-28 Thread Avi Kivity

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

2011-03-28 Thread Jes Sorensen
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

2011-03-28 Thread Alon Levy
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

2011-03-28 Thread Alexander Graf

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

2011-03-28 Thread Avi Kivity

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

2011-03-28 Thread Alexander Graf

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

2011-03-28 Thread Anthony Liguori

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

2011-03-28 Thread Anthony Liguori

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

2011-03-28 Thread Anthony Liguori

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

2011-03-28 Thread Jes Sorensen
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

2011-03-28 Thread Jes Sorensen
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

2011-03-28 Thread Jes Sorensen
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

2011-03-28 Thread Isaku Yamahata
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

2011-03-28 Thread Jes Sorensen
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

2011-03-28 Thread Stefano Stabellini
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

2011-03-28 Thread Peter Maydell
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

2011-03-28 Thread Peter Maydell
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

2011-03-28 Thread Peter Maydell
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

2011-03-28 Thread Jes Sorensen
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

2011-03-28 Thread Peter Maydell
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

2011-03-28 Thread Jes Sorensen
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

2011-03-28 Thread Peter Maydell
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

2011-03-28 Thread Jes Sorensen
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

2011-03-28 Thread Feiran Zheng
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

2011-03-28 Thread David Gibson
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

2011-03-28 Thread Peter Maydell
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

2011-03-28 Thread Alexander Graf

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

2011-03-28 Thread Alon Levy
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

2011-03-28 Thread Alon Levy
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

2011-03-28 Thread Alon Levy
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

2011-03-28 Thread Jes Sorensen
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

2011-03-28 Thread Jes Sorensen
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

2011-03-28 Thread Peter Maydell
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

2011-03-28 Thread Alon Levy
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

2011-03-28 Thread Peter Maydell
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.

2011-03-28 Thread Anthony PERARD
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

2011-03-28 Thread Isaku Yamahata
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

2011-03-28 Thread Isaku Yamahata
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

2011-03-28 Thread Isaku Yamahata
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

2011-03-28 Thread Isaku Yamahata
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

2011-03-28 Thread Isaku Yamahata
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

2011-03-28 Thread Alon Levy
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

2011-03-28 Thread Jes Sorensen
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.

2011-03-28 Thread Anthony PERARD
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

2011-03-28 Thread Peter Maydell
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

2011-03-28 Thread Alon Levy
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

2011-03-28 Thread Alon Levy
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

2011-03-28 Thread Alon Levy
---
 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

2011-03-28 Thread Alon Levy
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

2011-03-28 Thread Alon Levy
---

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

2011-03-28 Thread Anthony PERARD
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

2011-03-28 Thread Alon Levy
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

2011-03-28 Thread Alon Levy
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

2011-03-28 Thread Alon Levy
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

2011-03-28 Thread Alon Levy
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

2011-03-28 Thread Anthony PERARD
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

2011-03-28 Thread Michael S. Tsirkin
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

2011-03-28 Thread Michael S. Tsirkin
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

2011-03-28 Thread Alon Levy
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

2011-03-28 Thread Anthony Liguori

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)

2011-03-28 Thread Anthony Liguori

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

2011-03-28 Thread Luiz Capitulino
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

2011-03-28 Thread Alon Levy
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

2011-03-28 Thread Anthony Liguori

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

2011-03-28 Thread Blue Swirl
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

2011-03-28 Thread Alexander Graf

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;
+

  1   2   >