Re: [Qemu-devel] [RFC 0/4] Adding -cdrom, -hd[abcd] and -drive file=... to Q35

[Markus Armbruster]

John Snow  writes:

> On 08/19/2014 02:08 PM, Markus Armbruster wrote:
>> John Snow  writes:
>>
>>> On 08/19/2014 04:05 AM, Markus Armbruster wrote:
 John Snow  writes:

> Currently, the drive definitions created by drive_new() when using
> the -drive file=...[,if=ide] or -cdrom or -hd[abcd] options are not
> picked up by the Q35 initialization routine.
>
> To fix this, we have to add hooks to search for these drives using
> something like pc_piix's ide_drive_get and then add them using
> something like pci_ide_create_devs.

 ide_drive_get() isn't pc_piix's, it's a helper function in the IDE core
 which boards (not just pc_piix) use to find the if=ide drives.  It fills
 in an array of DriveInfo.

 pci_ide_create_devs() is a helper function in the IDE PCI code which PCI
 IDE controllers (not just piix3-ide) use to create IDE devices for an
 array of DriveInfo.
>>>
>>> Yes. I meant to say pc_piix's /call to/ ide_drive_get. I would have to
>>> patch up the other boards if I changed this function! Only an RFC
>>> before I got too far down this path :]
>>>
> Where it gets slightly wonky is the fact that if=ide is specified
> to use two devices per bus, whereas AHCI does not utilize that
> same master/slave mechanic. Therefore, many places inside of
> blockdev.c where we add and define new drives use incorrect math
> for AHCI devices and try to place them on impossible buses.
> Notably -hdb and -hdd would become inaccessible.

 Yes.

> To remedy this, I added a new interface type, IF_AHCI. Corresponding
> to this change, I modified the default machine properties for Q35
> to use this interface as a default.
>
> The changes appear to work well, but where I'd like some feedback
> is what should happen if people do something like:
>
> qemu -M q35 -drive if=ide,file=fedora.qcow2
>
> The code as presented here is not going to look for or attempt to
> connect IDE devices, because it is now looking for /AHCI/ devices.
>
> At worst, this may break a few existing scripts, but I actually think
> that since the if=ide,file=... shorthand never worked to begin with,
> the impact might actually be minimal.
>
> But since the legacy IDE interface of the ICH9 is as of yet unemulated,
> the if=ide drives don't have a reasonable place to go yet. I am also
> not sure what a reasonable way to handle people specifying BOTH
> if=ide and if=ahci drives would be.

 We've been through IF_AHCI before, more than once, but that was before
 you got involved :)

 The problem at hand is that "-drive if=ide" and its sugared forms -hda,
 -hdb, -cdrom, ... don't work with Q35.

 You provide a solution for the sugared forms, you leave the problem
 unsolved for the unsugared form, and you add a new problem: -drive
 if=ahci doesn't work with i440FX.  Progress, sort of.
>>>
>>> Adding a call to boards that support the AHCI device during their
>>> initialization should be easy enough, if we decide that "ide means
>>> ISA/PCI, ahci means the AHCI HBA." I could probably even write one
>>> generic routine between i440fx and q35 to do both IDE/AHCI.
>>>
>>> If we decide that IF_IDE and IF_AHCI mean different things, the
>>> problem of the unsugared form being unsolved depends on me (well, or
>>> someone) implementing the legacy IDE interface for Q35.
>>
>> Let me come back to this further down.

I didn't.  This time I will.

 Let's take a step back, and recap previous discussion.  There are two
 defensible point of views, in my opinion.

 One is that IDE and AHCI should be separate interface types, just like
 IDE and SCSI are.

 Attempts to define an if=X drive with a board that doesn't provide a
 controller for X should fail[*].  Only onboard controllers matter,
 add-ons plugged in with -device don't.  An i440FX board provides only
 IDE.  A Q35 board provides only AHCI, not IDE.  If we implement an
 ich9-ahci legacy mode, and switch it on, then it provides only IDE, not
 AHCI.  Or maybe both, depending on how we do it.
>>>
>>> I think I am leaning towards this viewpoint, but it depends on what
>>> "interface" means in QEMU. Currently, the number of units per bus is
>>> tied to the "interface" and clearly the AHCI SATA interface only
>>> supports one per bus, so semantically this makes sense.
>>
>> An index <-> (bus, unit) mapping doesn't make an interface!  Yes, it's
>> tightly coupled to the interface, but that became wrong way back when we
>> went beyond 8-bit SCSI HBAs, long before we added up AHCI HBAs.
>
> "Interface" seems nebulous in QEMU. In the physical world, there
> definitely is a difference between IDE/EIDE/PATA and
> SATA/AHCI. Different physical and link layers -- the verbs stayed the
> same. What I am getting at is that I am not sure what an "interface"
> is /supposed/ to

Re: [Qemu-devel] [RFC PATCH v1 07/11] memory.c: Add address_space_init_shareable()

[Peter Crosthwaite]

On Wed, Jun 4, 2014 at 1:30 AM, Peter Maydell  wrote:
> On 3 June 2014 03:10, Peter Crosthwaite  wrote:
>> This will either create a new AS or return a pointer to an
>> already existing equivalent one. Both name and root mr must
>> match.
>>
>> The motivation is to reuse address spaces as much as possible.
>> Its going to be quite common that bus masters out in device land
>> have pointers to the same memory region for their mastering yet
>> each will need to create its own address space. Let the memory
>> API implement sharing for them.
>>
>> Signed-off-by: Peter Crosthwaite 
>> ---
>> I know this leaks memory. I'll fix that post RFC. I think we need
>> AS ref counters to do it properly if anyone has any input on how
>> that should be done.
>>
>> We could change the equivalency test only match mr to support device
>> specific naming of these shared ASes. The singleton AS can ultimately
>> only have one name however. So perhaps some strcatting each time a new
>> sharer is added to the share. That or first-in-best-dressed.
>
> Is this here because it looked like it would be really easy
> to do, or because you tried this series without shared ASes
> and found it was too inefficient?
>

Because it look easy. Although less actual as's means less info mtree
output. I would suspect that tool is un-usable without shared AS.

Regards,
Peter

> thanks
> -- PMM
>

[Qemu-devel] [Bug] QEMU crashed with latest qemu master tree code

[Gonglei (Arei)]

Hi,

Command line:

#./qemu-system-x86_64 -enable-kvm -m 2048 -machine q35 -device 
ide-drive,bus=ide.2,drive=MacHDD \
-drive id=MacHDD,if=none,file=/mnt/sdb/gonglei/image/redhat_q35.img -monitor 
stdio -vnc :10 -readconfig ../docs/q35-chipset.cfg
(qemu) Segmentation fault (core dumped)

Some clews:

1. the bad commit is 0e4a77370594c91dd126f9872893ed473374cc72
2. if I do not add command line argument '-readconfig ../docs/q35-chipset.cfg 
', it works well.
3. backtrace as below:

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7257c700 (LWP 27649)]
0x558b4ad3 in object_class_dynamic_cast (class=0x56523720, 
typename=0x559ba852 "cpu") at qom/object.c:502
502 if (type->name == typename) {
(gdb) p *type
Cannot access memory at address 0x0
(gdb) bt
#0  0x558b4ad3 in object_class_dynamic_cast (class=0x56523720, 
typename=0x559ba852 "cpu") at qom/object.c:502
#1  0x558b485a in object_dynamic_cast (obj=0x5643abe0, 
typename=0x559ba852 "cpu") at qom/object.c:443
#2  0x556a4ee3 in acpi_add_cpu_info (o=0x5643abe0, 
opaque=0x7257b930) at /home/new/qemu/hw/i386/acpi-build.c:133
#3  0x558b50fa in object_child_foreach (obj=0x563e8770, 
fn=0x556a4eac , opaque=0x7257b930)
at qom/object.c:676
#4  0x556a4f4b in acpi_add_cpu_info (o=0x563e8770, 
opaque=0x7257b930) at /home/new/qemu/hw/i386/acpi-build.c:140
#5  0x558b50fa in object_child_foreach (obj=0x562efa40, 
fn=0x556a4eac , opaque=0x7257b930)
at qom/object.c:676
#6  0x556a4f4b in acpi_add_cpu_info (o=0x562efa40, 
opaque=0x7257b930) at /home/new/qemu/hw/i386/acpi-build.c:140
#7  0x558b50fa in object_child_foreach (obj=0x562d26a0, 
fn=0x556a4eac , opaque=0x7257b930)
at qom/object.c:676
#8  0x556a4f4b in acpi_add_cpu_info (o=0x562d26a0, 
opaque=0x7257b930) at /home/new/qemu/hw/i386/acpi-build.c:140
#9  0x558b50fa in object_child_foreach (obj=0x562d5770, 
fn=0x556a4eac , opaque=0x7257b930)
at qom/object.c:676
#10 0x556a5024 in acpi_get_cpu_info (cpu=0x7257b930) at 
/home/new/qemu/hw/i386/acpi-build.c:149
#11 0x556a8bb7 in acpi_build (guest_info=0x56309880, 
tables=0x7257b980) at /home/new/qemu/hw/i386/acpi-build.c:1486
#12 0x556a90cd in acpi_build_update (build_opaque=0x564353b0, 
offset=0) at /home/new/qemu/hw/i386/acpi-build.c:1623
#13 0x55826373 in fw_cfg_read (s=0x5630d010) at 
hw/nvram/fw_cfg.c:255
#14 0x55826565 in fw_cfg_comb_read (opaque=0x5630d010, addr=1, 
size=1) at hw/nvram/fw_cfg.c:291
#15 0x55645f52 in memory_region_read_accessor (mr=0x5630f4f0, 
addr=1, value=0x7257bb10, size=1, shift=0, mask=255)
at /home/new/qemu/memory.c:410
#16 0x5564624c in access_with_adjusted_size (addr=1, 
value=0x7257bb10, size=1, access_size_min=1, access_size_max=4, 
access=0x55645ef4 , mr=0x5630f4f0) at 
/home/new/qemu/memory.c:480
#17 0x55649651 in memory_region_dispatch_read1 (mr=0x5630f4f0, 
addr=1, size=1) at /home/new/qemu/memory.c:1096
#18 0x55649709 in memory_region_dispatch_read (mr=0x5630f4f0, 
addr=1, pval=0x7257bc00, size=1)
at /home/new/qemu/memory.c:1118
#19 0x5564d325 in io_mem_read (mr=0x5630f4f0, addr=1, 
pval=0x7257bc00, size=1) at /home/new/qemu/memory.c:1961
#20 0x555f7bfe in address_space_rw (as=0x55df6d20 
, addr=1297, buf=0x77ff3000 "&", len=1, 
is_write=false) at /home/new/qemu/exec.c:2086
#21 0x556429d6 in kvm_handle_io (port=1297, data=0x77ff3000, 
direction=0, size=1, count=1024)
at /home/new/qemu/kvm-all.c:1597
#22 0x55642f7a in kvm_cpu_exec (cpu=0x562f3060) at 
/home/new/qemu/kvm-all.c:1734
#23 0x5562a4d8 in qemu_kvm_cpu_thread_fn (arg=0x562f3060) at 
/home/new/qemu/cpus.c:939
#24 0x7572a7f6 in start_thread () from /lib64/libpthread.so.0
#25 0x7548609d in clone () from /lib64/libc.so.6
#26 0x in ?? ()
(gdb) p *class
$2 = {type = 0x0, interfaces = 0x21 = {
(gdb)

Best regards,
-Gonglei

Re: [Qemu-devel] [RFC PATCH v1 09/11] exec: use per-cpu address-spaces for cpuisms

[Peter Crosthwaite]

On Wed, Jun 4, 2014 at 1:28 AM, Peter Maydell  wrote:
> On 3 June 2014 03:11, Peter Crosthwaite  wrote:
>> Rather than address_space_memory.
>>
>> Needs a few random callsites with hardcoded address_space_memory to be
>> patched. Use first_cpu->as in these cases.
>>
>> Signed-off-by: Peter Crosthwaite 
>> ---
>> Long term, cpu_physical_memory_rw and friends needs to be replaced.
>
> Yeah. The other two are not so bad, but cpu_physical_memory_rw()
> really shouldn't have a first_cpu reference in it.
>

Dropping that. I guess the plan is to convert users on
cpu_physical_memory_rw to address_space_rw anyways. Can fix them on a
demand basis.

Regards,
Peter

>
> thanks
> -- PMM
>

Re: [Qemu-devel] [RFC PATCH v2 10/13] linux headers update for DDW

[Alexey Kardashevskiy]

On 08/19/2014 03:42 AM, Alex Williamson wrote:
> On Fri, 2014-08-15 at 20:12 +1000, Alexey Kardashevskiy wrote:
>> Since the changes are not in upstream yet, no tag or branch is specified 
>> here.
>>
>> Signed-off-by: Alexey Kardashevskiy 
>> ---
>>  linux-headers/linux/vfio.h | 37 -
>>  1 file changed, 36 insertions(+), 1 deletion(-)
>>
>> diff --git a/linux-headers/linux/vfio.h b/linux-headers/linux/vfio.h
>> index 26c218e..f0aa97d 100644
>> --- a/linux-headers/linux/vfio.h
>> +++ b/linux-headers/linux/vfio.h
>> @@ -448,13 +448,48 @@ struct vfio_iommu_type1_dma_unmap {
>>   */
>>  struct vfio_iommu_spapr_tce_info {
>>  __u32 argsz;
>> -__u32 flags;/* reserved for future use */
>> +__u32 flags;
>> +#define VFIO_IOMMU_SPAPR_TCE_FLAG_DDW   1 /* Support dynamic windows */
>>  __u32 dma32_window_start;   /* 32 bit window start (bytes) */
>>  __u32 dma32_window_size;/* 32 bit window size (bytes) */
>>  };
>>  
>>  #define VFIO_IOMMU_SPAPR_TCE_GET_INFO   _IO(VFIO_TYPE, VFIO_BASE + 12)
>>  
>> +/*
>> + * Dynamic DMA windows
>> + */
>> +struct vfio_iommu_spapr_tce_query {
>> +__u32 argsz;
>> +/* out */
>> +__u32 windows_available;
>> +__u32 page_size_mask;
>> +};
> 
> Why do we need a new ioctl for this vs extending tce_info to include it?
> That's sort of the point of including argsz and flags in the ioctl.


It is not actual now but I can imagine that these numbers may change
depending on multiple calls of create()/remove().


> 
>> +#define VFIO_IOMMU_SPAPR_TCE_QUERY  _IO(VFIO_TYPE, VFIO_BASE + 17)
>> +
>> +struct vfio_iommu_spapr_tce_create {
>> +__u32 argsz;
>> +/* in */
>> +__u32 page_shift;
>> +__u32 window_shift;
>> +/* out */
>> +__u64 start_addr;
>> +
>> +};
>> +#define VFIO_IOMMU_SPAPR_TCE_CREATE _IO(VFIO_TYPE, VFIO_BASE + 18)
>> +
>> +struct vfio_iommu_spapr_tce_remove {
>> +__u32 argsz;
>> +/* in */
>> +__u64 start_addr;
>> +};
>> +#define VFIO_IOMMU_SPAPR_TCE_REMOVE _IO(VFIO_TYPE, VFIO_BASE + 19)
>> +
>> +struct vfio_iommu_spapr_tce_reset {
>> +__u32 argsz;
>> +};
>> +#define VFIO_IOMMU_SPAPR_TCE_RESET  _IO(VFIO_TYPE, VFIO_BASE + 20)
>> +
> 
> argsz by itself seems rather pointless if we don't have a flags field to
> augment the structure.  Thanks,


Add flags and check for zero  or  remove it? Cannot choose, please help :)


> Alex
> 
>>  /* * */
>>  
>>  #endif /* VFIO_H */
> 
> 
> 


-- 
Alexey

Re: [Qemu-devel] [PULL 19/20] memory: Use canonical path component as the name

[Peter Maydell]

On 20 August 2014 06:04, Peter Crosthwaite  wrote:
> On Wed, Aug 20, 2014 at 5:01 AM, Peter Maydell  
> wrote:
>> I think we need to revert this (commit b0225c2c0d8) until
>> both the Xen callsites are fixed and the leak issue is
>> dealt with.
>>
>
> Have half a plan on the leak issue. With
> object_get_canonical_path_component() being used increasing as a "get
> me the already set, name of this object", I think it's better to just
> change to API to return const and remove the free burden from all call
> sites completely. If call sites really want a fresh copy they can take
> one themselves. Patch on list imminently.

I'm going to go ahead and revert this patch for the moment so
we can take our time reviewing that patch set.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 0/3] q35: Minor fixes/enhancements to improve usability of root ports

[Markus Armbruster]

Please make sure the parts of your series are threaded together properly
with In-Reply-To and References headers, because that helps e-mail
readers to keep the parts together.  Lack of threading is annoying, and
annoying reviewers intentionally would be rude :)

Suggested workflow:

git-format-patch --cover-letter -ns ...
look over patch files, edit the cover letter to taste
git-send-email --to qemu-devel@nongnu.org 0*.patch

See also http://wiki.qemu.org/Contribute/SubmitAPatch

Re: [Qemu-devel] QEMU block layer todo list

[Christian Borntraeger]

On 19/08/14 16:20, Stefan Hajnoczi wrote:
> The following wiki page contains a list of proposed tasks for the QEMU
> block layer:
> 
> http://qemu-project.org/Features/Block/Todo
> 
> Benoit requested that we make the todo list viewable/editable.  This
> is a good idea as it helps us focus and see what other developers are
> thinking about.
> 
> There is no requirement to put tasks on the wiki.  Just sending
> patches or discussing on the mailing list is fine.
> 
> The todo list is useful for large tasks or anything which cannot be
> completed within a couple of weeks.  That way the tasks will not be
> forgotten.
> 
> You can subscribe to the wiki changes RSS feed to follow edits:
> http://qemu-project.org/index.php?title=Special:RecentChanges&feed=rss
> 
> Stefan
> 

I would like to add geometry and block size passthrough for host devices. See
the patches from Ekaterina Tumanova.
https://lists.nongnu.org/archive/html/qemu-devel/2014-07/msg03791.html

On s390 we have a stricter requirement of passing through geometry and block 
size for DASD disk devices as this geometry is more than a imaginary number 
triplet. It is required for proper partition detection and is actually a 
property of the real (or better emulated on storage servers like DS8x000) disk.

Christian

Re: [Qemu-devel] [RFC PATCH v2 02/13] spapr_iommu: Disable in-kernel IOMMU tables for >4GB windows

[Alexey Kardashevskiy]

On 08/19/2014 10:43 AM, David Gibson wrote:
> On Fri, Aug 15, 2014 at 08:12:24PM +1000, Alexey Kardashevskiy wrote:
>> The existing KVM_CREATE_SPAPR_TCE ioctl only support 4G windows max.
>> We are going to add huge DMA windows support so this will create small
>> window and unexpectedly fail later.
>>
>> This disables KVM_CREATE_SPAPR_TCE for windows bigger that 4GB. Since
>> those windows are normally mapped at the boot time, there will be no
>> performance impact.
>>
>> Signed-off-by: Alexey Kardashevskiy 
> 
> I think perhaps the crucial point here is that the window size
> parameter to the kernel ioctl() is only 32-bit, so there's no way of
> expressing a TCE window > 4GB.

That exactly the point. I'll steal these bit and commit log will start with:

===
The existing KVM_CREATE_SPAPR_TCE ioctl only support 4G windows max a
the window size parameter to the kernel ioctl() is only 32-bit, so
there's no way of expressing a TCE window > 4GB.
===


>> ---
>>  hw/ppc/spapr_iommu.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c
>> index f6e32a4..36f5d27 100644
>> --- a/hw/ppc/spapr_iommu.c
>> +++ b/hw/ppc/spapr_iommu.c
>> @@ -113,11 +113,11 @@ static MemoryRegionIOMMUOps spapr_iommu_ops = {
>>  static int spapr_tce_table_realize(DeviceState *dev)
>>  {
>>  sPAPRTCETable *tcet = SPAPR_TCE_TABLE(dev);
>> +uint64_t window_size = tcet->nb_table << tcet->page_shift;
> 
> tcet->nb_table is only 32-bit itself, so this isn't going to work as
> intended without a cast.

Oh. Thanks. /me is thinking how to catch this kind of errors from a script...



-- 
Alexey

Re: [Qemu-devel] [libvirt] [PATCHv2 libvirt] qemu: Issue rtc-reset-reinjection command after guest-set-time

[Michal Privoznik]

On 19.08.2014 19:23, Marcelo Tosatti wrote:

On Tue, Aug 19, 2014 at 11:00:26AM -0600, Eric Blake wrote:

On 08/19/2014 10:57 AM, Marcelo Tosatti wrote:


rtc-reset-reinjection has been introduced because certain Windows
versions will advance the guest system time (via rtc interrupt
reinjection).

So if libvirt adjusts the guest system time via guest-set-time,
allowing rtc interrupt reinjection to compensate for lost time,
as well, will cause an incorrect guest system time.

So you should always use the

 guest-set-time
 rtc-reset-reinjection

pair.


But is that true both for the 'guest-set-time' no-arg case (which tells
the guest to read the current RTC and update in-memory time
accordingly), as well as the 'guest-set-time with time argument' case
(which tells the guest to forcefully set in-memory time, then write that
time back to the RTC)?


Yes.



Okay then. I'm posting v3 here [1] which unconditionally calls the 
monitor command right after the guest agent command.


Michal

1: https://www.redhat.com/archives/libvir-list/2014-August/msg00867.html

Re: [Qemu-devel] [PULL 19/20] memory: Use canonical path component as the name

[Paolo Bonzini]

Il 20/08/2014 09:50, Peter Maydell ha scritto:
> On 20 August 2014 06:04, Peter Crosthwaite  
> wrote:
>> On Wed, Aug 20, 2014 at 5:01 AM, Peter Maydell  
>> wrote:
>>> I think we need to revert this (commit b0225c2c0d8) until
>>> both the Xen callsites are fixed and the leak issue is
>>> dealt with.
>>>
>>
>> Have half a plan on the leak issue. With
>> object_get_canonical_path_component() being used increasing as a "get
>> me the already set, name of this object", I think it's better to just
>> change to API to return const and remove the free burden from all call
>> sites completely. If call sites really want a fresh copy they can take
>> one themselves. Patch on list imminently.
> 
> I'm going to go ahead and revert this patch for the moment so
> we can take our time reviewing that patch set.

Yes, thanks.

Paolo

Re: [Qemu-devel] [PATCH 0/3] q35: Minor fixes/enhancements to improve usability of root ports

[Knut Omang]

On Wed, 2014-08-20 at 09:54 +0200, Markus Armbruster wrote:
> Please make sure the parts of your series are threaded together properly
> with In-Reply-To and References headers, because that helps e-mail
> readers to keep the parts together.  Lack of threading is annoying, and
> annoying reviewers intentionally would be rude :)

I am sorry about that and thanks for quickly notifying me.  

> Suggested workflow:
> 
> git-format-patch --cover-letter -ns ...
> look over patch files, edit the cover letter to taste
> git-send-email --to qemu-devel@nongnu.org 0*.patch
> 
> See also http://wiki.qemu.org/Contribute/SubmitAPatch

I did follow it apart from the last step.
Will re-post as soon as I get git-send-email to work with the corporate
mail server.

Knut

Re: [Qemu-devel] [PATCH 1/4] hd-geometry.c: Integrate HDIO_GETGEO in guessing for target-s390x

[Paolo Bonzini]

Il 29/07/2014 14:27, Ekaterina Tumanova ha scritto:
> The new HDIO_GETGEO logic is required for two use cases:
> a) Support for geometries of Direct Attached Storage Disks (DASD)
> on s390x configured as backing of virtio block devices.

Is this still relevant now that QEMU can emulate 512-byte sectors on top
of host devices with 4k sectors?

> b) Support for FCP attached SCSI disks that do not yet have a
> partition table. Without this patch, fdisk -l on the host would
> return different results then fdisk -l in the guest.

Can you provide an example?

Paolo

Re: [Qemu-devel] [PATCH 3/3] ioh3420: Support ARI forwarding

[Gonglei (Arei)]

Hi,

> Subject: [PATCH 3/3] ioh3420: Support ARI forwarding
> 
> 
> Enable the PCIe capability bit that indicates that this port is able to 
> support
> and forward requests to > 8 functions for ARI capable devices.
> 
> Signed-off-by: Knut Omang 
> ---
>  hw/pci-bridge/ioh3420.c | 7 +++
>  1 file changed, 7 insertions(+)
> 

I think you should associate patch 2/3 with patch 3/3 to
one patch, otherwise after patch 2/3 building are broken.

Best regards,
-Gonglei

> diff --git a/hw/pci-bridge/ioh3420.c b/hw/pci-bridge/ioh3420.c
> index 8f6c8b0..c14ea6b 100644
> --- a/hw/pci-bridge/ioh3420.c
> +++ b/hw/pci-bridge/ioh3420.c
> @@ -92,8 +92,10 @@ static void ioh3420_reset(DeviceState *qdev)
> 
>  static int ioh3420_initfn(PCIDevice *d)
>  {
> +PCIBridge* br = PCI_BRIDGE(d);
>  PCIEPort *p = PCIE_PORT(d);
>  PCIESlot *s = PCIE_SLOT(d);
> +uint8_t *exp_cap;
>  int rc;
>  char tmp[100];
>  sprintf(tmp, "pcie_port.%d", s->slot);
> @@ -121,6 +123,10 @@ static int ioh3420_initfn(PCIDevice *d)
>  if (rc < 0) {
>  goto err_msi;
>  }
> +/* Support ARI forwarding */
> +exp_cap = d->config + d->exp.exp_cap;
> +pci_word_test_and_set_mask(exp_cap + PCI_EXP_DEVCAP2,
> PCI_EXP_DEVCAP2_ARI);
> +
>  pcie_cap_deverr_init(d);
>  pcie_cap_slot_init(d, s->slot);
>  pcie_chassis_create(s->chassis);
> @@ -151,6 +157,7 @@ err_bridge:
>  static void ioh3420_exitfn(PCIDevice *d)
>  {
>  PCIESlot *s = PCIE_SLOT(d);
> +PCIBridge* br = PCI_BRIDGE(d);
> 
>  pcie_aer_exit(d);
>  pcie_chassis_del_slot(s);
> --
> 1.9.0
> 

[Qemu-devel] [Bug 1354279] Re: The guest will be destroyed after hot remove the VF from the guest.

[chao zhou]

test on Ivytown_EP
kvm.git + qemu.git: c77dcacb_0e4a7737
kernel version: 3.16.0
after hot remove the VF from the guest, the guest works fine.

** Changed in: qemu
   Status: New => Fix Committed

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1354279

Title:
  The guest will be destroyed after hot remove the VF from the guest.

Status in QEMU:
  Fix Committed

Bug description:
  Environment:
  
  Host OS (ia32/ia32e/IA64):ia32e
  Guest OS (ia32/ia32e/IA64):ia32e
  Guest OS Type (Linux/Windows):Linux
  kvm.git Commit:9f6226a762c7ae02f6a23a3d4fc552dafa57ea23
  qemu.git Commit:5a7348045091a2bc15d85bb177e5956aa6114e5a
  Host Kernel Version:3.16.0-rc1
  Hardware:Romley_EP, Ivytown_EP,Haswell_EP

  
  Bug detailed description:
  --
  hot add the VF to the guest, then hot remove the VF from the guest, the guest 
will be destroyed.

  note:
  1. when hot add the VF with vfio, the hot remove the VF from the guest, the 
guest works fine.
  2. this shoule be a qemu bug:
  kvm   +   qemu= result
  9f6226a7  +  5a734804 = bad
  9f6226a7  +  9f862687 = good


  Reproduce steps:
  
  1. create guest
  qemu-system-x86_64 --enable-kvm -m 1024 -smp 4 -net none rhel6u5.qcow 
-monitor pty
  2. hot add the vf to guest
  echo "device_add pci-assign,host=05:10.0,id=nic" >/dev/pts/x
  cat /dev/pts/x
  3. hot remove the VF froem guest
  echo "device_del nic" >/dev/pts/x

  Current result:
  
  the guest willl be destroyed after hot remove the VF from the guest

  Expected result:
  
  the guest works fine after hot remove the VF from the guest

  
  Basic root-causing log:
  --
  [root@vt-snb9 qemu.git]# qemu-system-x86_64 -enable-kvm -m 1024 -smp 2 -net 
none rhel6u5.qcow -monitor pty
  VNC server running on `::1:5900'
  **
  ERROR:qom/object.c:725:object_unref: assertion failed: (obj->ref > 0)
  Aborted (core dumped)

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1354279/+subscriptions

Re: [Qemu-devel] [PATCH 3/3] ioh3420: Support ARI forwarding

[Knut Omang]

On Wed, 2014-08-20 at 08:34 +, Gonglei (Arei) wrote:
> Hi,
> 
> > Subject: [PATCH 3/3] ioh3420: Support ARI forwarding
> > 
> > 
> > Enable the PCIe capability bit that indicates that this port is able to 
> > support
> > and forward requests to > 8 functions for ARI capable devices.
> > 
> > Signed-off-by: Knut Omang 
> > ---
> >  hw/pci-bridge/ioh3420.c | 7 +++
> >  1 file changed, 7 insertions(+)
> > 
> 
> I think you should associate patch 2/3 with patch 3/3 to
> one patch, otherwise after patch 2/3 building are broken.
> 
> Best regards,
> -Gonglei

Thanks - just noticed - somehow I managed tp break the split of those
two changes since they are independent. 

Knut

> > diff --git a/hw/pci-bridge/ioh3420.c b/hw/pci-bridge/ioh3420.c
> > index 8f6c8b0..c14ea6b 100644
> > --- a/hw/pci-bridge/ioh3420.c
> > +++ b/hw/pci-bridge/ioh3420.c
> > @@ -92,8 +92,10 @@ static void ioh3420_reset(DeviceState *qdev)
> > 
> >  static int ioh3420_initfn(PCIDevice *d)
> >  {
> > +PCIBridge* br = PCI_BRIDGE(d);
> >  PCIEPort *p = PCIE_PORT(d);
> >  PCIESlot *s = PCIE_SLOT(d);
> > +uint8_t *exp_cap;
> >  int rc;
> >  char tmp[100];
> >  sprintf(tmp, "pcie_port.%d", s->slot);
> > @@ -121,6 +123,10 @@ static int ioh3420_initfn(PCIDevice *d)
> >  if (rc < 0) {
> >  goto err_msi;
> >  }
> > +/* Support ARI forwarding */
> > +exp_cap = d->config + d->exp.exp_cap;
> > +pci_word_test_and_set_mask(exp_cap + PCI_EXP_DEVCAP2,
> > PCI_EXP_DEVCAP2_ARI);
> > +
> >  pcie_cap_deverr_init(d);
> >  pcie_cap_slot_init(d, s->slot);
> >  pcie_chassis_create(s->chassis);
> > @@ -151,6 +157,7 @@ err_bridge:
> >  static void ioh3420_exitfn(PCIDevice *d)
> >  {
> >  PCIESlot *s = PCIE_SLOT(d);
> > +PCIBridge* br = PCI_BRIDGE(d);
> > 
> >  pcie_aer_exit(d);
> >  pcie_chassis_del_slot(s);
> > --
> > 1.9.0
> > 
> 

Re: [Qemu-devel] [PATCH 2/3] ioh3420: Provide a unique bus name and an interrupt mapping function

[Paolo Bonzini]

Il 20/08/2014 08:53, Knut Omang ha scritto:
> A unique bus name is necessary to be able to refer to each instance
> from the command line and monitors.

Is it needed?  Can't you just add id= to the -device option?

Paolo

Re: [Qemu-devel] [PATCH bugfix v1 1/3] xen: hvm: Abstract away memory region name ref (fix broken build)

[Stefan Weil]

Am 20.08.2014 um 07:07 schrieb Peter Crosthwaite:
> The mr->name field is removed. This slipped through compile testing.
> Fix.
> 
> Signed-off-by: Peter Crosthwaite 
> ---
> 
>  xen-hvm.c | 9 ++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/xen-hvm.c b/xen-hvm.c
> index 91de2e2..9fd9d6e 100644
> --- a/xen-hvm.c
> +++ b/xen-hvm.c
> @@ -291,6 +291,7 @@ static int xen_add_to_physmap(XenIOState *state,
>  hwaddr pfn, start_gpfn;
>  hwaddr phys_offset = memory_region_get_ram_addr(mr);
>  char path[80], value[17];
> +const char *mr_name;
>  
>  if (get_physmapping(state, start_addr, size)) {
>  return 0;
> @@ -326,11 +327,13 @@ go_physmap:
>  }
>  }
>  
> +mr_name = memory_region_name(mr);
> +
>  physmap = g_malloc(sizeof (XenPhysmap));
>  
>  physmap->start_addr = start_addr;
>  physmap->size = size;
> -physmap->name = (char *)mr->name;
> +physmap->name = (char *)mr_name;

This type cast can be removed. Just add the const in XenPhysmap's name
declaration.

>  physmap->phys_offset = phys_offset;
>  
>  QLIST_INSERT_HEAD(&state->physmap, physmap, list);
> @@ -354,11 +357,11 @@ go_physmap:
>  if (!xs_write(state->xenstore, 0, path, value, strlen(value))) {
>  return -1;
>  }
> -if (mr->name) {
> +if (mr_name) {
>  snprintf(path, sizeof(path),
>  "/local/domain/0/device-model/%d/physmap/%"PRIx64"/name",
>  xen_domid, (uint64_t)phys_offset);
> -if (!xs_write(state->xenstore, 0, path, mr->name, strlen(mr->name))) 
> {
> +if (!xs_write(state->xenstore, 0, path, mr_name, strlen(mr_name))) {
>  return -1;
>  }
>  }
> 


Otherwise ok. Below is my own variant which I tested before I noticed
your patch.

Reviewed-by: Stefan Weil 


@@ -69,11 +69,11 @@ static inline ioreq_t
*xen_vcpu_ioreq(shared_iopage_t *shared_page, int vcpu)
 #define BUFFER_IO_MAX_DELAY  100

 typedef struct XenPhysmap {
 hwaddr start_addr;
 ram_addr_t size;
-char *name;
+const char *name;
 hwaddr phys_offset;

 QLIST_ENTRY(XenPhysmap) list;
 } XenPhysmap;

@@ -328,11 +328,11 @@ go_physmap:

 physmap = g_malloc(sizeof (XenPhysmap));

 physmap->start_addr = start_addr;
 physmap->size = size;
-physmap->name = (char *)mr->name;
+physmap->name = memory_region_name(mr);
 physmap->phys_offset = phys_offset;

 QLIST_INSERT_HEAD(&state->physmap, physmap, list);

 xc_domain_pin_memory_cacheattr(xen_xc, xen_domid,
@@ -352,15 +352,16 @@ go_physmap:
 xen_domid, (uint64_t)phys_offset);
 snprintf(value, sizeof(value), "%"PRIx64, (uint64_t)size);
 if (!xs_write(state->xenstore, 0, path, value, strlen(value))) {
 return -1;
 }
-if (mr->name) {
+if (physmap->name) {
 snprintf(path, sizeof(path),
 "/local/domain/0/device-model/%d/physmap/%"PRIx64"/name",
 xen_domid, (uint64_t)phys_offset);
-if (!xs_write(state->xenstore, 0, path, mr->name,
strlen(mr->name))) {
+if (!xs_write(state->xenstore, 0, path, physmap->name,
+  strlen(physmap->name))) {
 return -1;
 }
 }

 return 0;

Re: [Qemu-devel] [PATCH bugfix v1 1/3] xen: hvm: Abstract away memory region name ref (fix broken build)

[Peter Maydell]

On 20 August 2014 09:53, Stefan Weil  wrote:
> Am 20.08.2014 um 07:07 schrieb Peter Crosthwaite:
>> The mr->name field is removed. This slipped through compile testing.
>> Fix.
>>
>> Signed-off-by: Peter Crosthwaite 
>> ---
>>
>>  xen-hvm.c | 9 ++---
>>  1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/xen-hvm.c b/xen-hvm.c
>> index 91de2e2..9fd9d6e 100644
>> --- a/xen-hvm.c
>> +++ b/xen-hvm.c
>> @@ -291,6 +291,7 @@ static int xen_add_to_physmap(XenIOState *state,
>>  hwaddr pfn, start_gpfn;
>>  hwaddr phys_offset = memory_region_get_ram_addr(mr);
>>  char path[80], value[17];
>> +const char *mr_name;
>>
>>  if (get_physmapping(state, start_addr, size)) {
>>  return 0;
>> @@ -326,11 +327,13 @@ go_physmap:
>>  }
>>  }
>>
>> +mr_name = memory_region_name(mr);
>> +
>>  physmap = g_malloc(sizeof (XenPhysmap));
>>
>>  physmap->start_addr = start_addr;
>>  physmap->size = size;
>> -physmap->name = (char *)mr->name;
>> +physmap->name = (char *)mr_name;
>
> This type cast can be removed. Just add the const in XenPhysmap's name
> declaration.

Yes, I noticed that dodgy cast too. Stefano, is there
a reason why the XenPhysmap field isn't just a
const pointer?

thanks
-- PMM

Re: [Qemu-devel] [PATCH 2/2] pci: add check for pcie root ports and downstream ports

[Marcel Apfelbaum]

On Wed, 2014-08-20 at 03:20 +, Gonglei (Arei) wrote:
> Hi,
> 
> > > Right now, ARI Forwarding dose not support in QEMU.
> > I would replace the above sentence with "ARI Forwarding is not supported".
> > 
> OK.
> 
> > By the way, there is some support for ARI, I don't know if
> > is enabled yet. I'll have a look.
> > 
> MST had pointed out the pcie_ari_init(), but not completed.
> 
> > > According to PCIe spec section 7.3.1, only slot 0 with
> > > the device attached to logic bus representing the link from
> > > downstream ports and root ports.
> > >
> > > So, adding check about slot 0 for PCIe downstream ports and
> > > root ports, which avoid useless operation, both hotplug and
> > > coldplug.
> > >
> > > Signed-off-by: Gonglei 
> > > ---
> > >  hw/pci/pci.c | 41 +
> > >  1 file changed, 41 insertions(+)
> > >
> > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> > > index 351d320..f2d267f 100644
> > > --- a/hw/pci/pci.c
> > > +++ b/hw/pci/pci.c
> > > @@ -773,6 +773,42 @@ static int pci_init_multifunction(PCIBus *bus,
> > PCIDevice *dev)
> > >  return 0;
> > >  }
> > >
> > > +static int pci_check_pcie_port(PCIBus *bus, PCIDevice *dev)
> > > +{
> > > +Object *obj = OBJECT(bus);
> > > +
> > > +if (!strcmp(object_get_typename(obj), TYPE_PCIE_BUS)) {
> > Maybe there is another way to check that this is a PCIe bus?
> > 
> Yes. Paolo has said that object_dynamic_cast() is appropriated.
> 
> > > +DeviceState *parent = qbus_get_parent(BUS(obj));
> > > +const char *name = object_get_typename(OBJECT(parent));
> > > +
> > > +/*
> > > + * Root ports and downstream ports of switches are the hot
> > > + * pluggable ports in a PCI Express hierarchy.
> > > + * PCI Express supports chip-to-chip interconnect, a PCIe link 
> > > can
> > > + * only connect one pci device/Switch/EndPoint or PCI-bridge.
> > > + *
> > > + * 7.3. Configuration Transaction Rules (PCI Express 
> > > specification
> > 3.0)
> > > + * 7.3.1. Device Number
> > > + *
> > > + * Downstream Ports that do not have ARI Forwarding enabled
> > must
> > > + * associate only Device 0 with the device attached to the 
> > > Logical
> > Bus
> > > + * representing the Link from the Port.
> > > + *
> > > + * Right now, ARI Forwarding dose not support. So, only slot 0 is
> > As above, maybe replace it with "ARI Forwarding is not supported"
> > 
> OK.
> 
> > > + * supported, regardless of hotplug or coldplug.
> > > + */
> > > +if (!strcmp(name, "ioh3420") || !strcmp(name,
> > "xio3130-downstream")) {
> > Please use port_type flag from extended configuration space, don't use 
> > device
> > names.
> > If you need help for this, let me know.
> > 
> Yes, please. I appreciate very much that you can help me.
Sure,

I checked and we already have the pcie_cap_get_type function that returns the 
port type.

port_type = pcie_cap_get_type(dev);
if (port_type == PCI_EXP_TYPE_DOWNSTREAM ||
port_type == PCI_EXP_TYPE_ROOT_PORT) {
...
}

Thanks,
Marcel

> 
> > > +if (PCI_SLOT(dev->devfn) != 0) {
> > > +error_report("Unsupported PCI slot %d for %s ports,
> > only "
> > > + "supported slot 0",
> > PCI_SLOT(dev->devfn), name);
> > > +return -1;
> > > +}
> > > +}
> > > +}
> > > +
> > > +return 0;
> > > +}
> > > +
> > >  static void pci_config_alloc(PCIDevice *pci_dev)
> > >  {
> > >  int config_size = pci_config_size(pci_dev);
> > > @@ -871,6 +907,11 @@ static PCIDevice
> > *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
> > >  return NULL;
> > >  }
> > >
> > > +if (pci_check_pcie_port(bus, pci_dev)) {
> > > +do_pci_unregister_device(pci_dev);
> > > +return NULL;
> > > +}
> > It is possible to move the above check earlier in do_pci_register_device
> > function?
> > Maybe move it into the first if statement(s)?
> > 
> Agreed, Thanks, Marcel.
> 
> Best regards,
> -Gonglei

Re: [Qemu-devel] [PATCH 3/3] ioh3420: Support ARI forwarding

[Marcel Apfelbaum]

On Wed, 2014-08-20 at 08:53 +0200, Knut Omang wrote:
> Enable the PCIe capability bit that indicates that this port is able to 
> support
> and forward requests to > 8 functions for ARI capable devices.
> 
> Signed-off-by: Knut Omang 
> ---
>  hw/pci-bridge/ioh3420.c | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/hw/pci-bridge/ioh3420.c b/hw/pci-bridge/ioh3420.c
> index 8f6c8b0..c14ea6b 100644
> --- a/hw/pci-bridge/ioh3420.c
> +++ b/hw/pci-bridge/ioh3420.c
> @@ -92,8 +92,10 @@ static void ioh3420_reset(DeviceState *qdev)
>  
>  static int ioh3420_initfn(PCIDevice *d)
>  {
> +PCIBridge* br = PCI_BRIDGE(d);
>  PCIEPort *p = PCIE_PORT(d);
>  PCIESlot *s = PCIE_SLOT(d);
> +uint8_t *exp_cap;
>  int rc;
>  char tmp[100];
>  sprintf(tmp, "pcie_port.%d", s->slot);
> @@ -121,6 +123,10 @@ static int ioh3420_initfn(PCIDevice *d)
>  if (rc < 0) {
>  goto err_msi;
>  }
> +/* Support ARI forwarding */
> +exp_cap = d->config + d->exp.exp_cap;
> +pci_word_test_and_set_mask(exp_cap + PCI_EXP_DEVCAP2, 
> PCI_EXP_DEVCAP2_ARI);
> +
Hi,

I think is better to move the implementation to a function, say 
pcie_cap_ari_init
and place it into hw/pci/pcie.c so other devices like downstream ports can use 
it.

Thanks,
Marcel

>  pcie_cap_deverr_init(d);
>  pcie_cap_slot_init(d, s->slot);
>  pcie_chassis_create(s->chassis);
> @@ -151,6 +157,7 @@ err_bridge:
>  static void ioh3420_exitfn(PCIDevice *d)
>  {
>  PCIESlot *s = PCIE_SLOT(d);
> +PCIBridge* br = PCI_BRIDGE(d);
>  
>  pcie_aer_exit(d);
>  pcie_chassis_del_slot(s);

Re: [Qemu-devel] [PATCH V3] vhost_net: start/stop guest notifiers properly

[William Dauchy]

On Aug19 12:56, Jason Wang wrote:
> commit a9f98bb5ebe6fb1869321dcc58e72041ae626ad8 vhost: multiqueue
> support changed the order of stopping the device. Previously
> vhost_dev_stop would disable backend and only afterwards, unset guest
> notifiers. We now unset guest notifiers while vhost is still
> active. This can lose interrupts causing guest networking to fail. In
> particular, this has been observed during migration.
> 
> To adapt this, several other changes are needed:
> - remove the hdev->started assertion in vhost.c since we may want to
> start the guest notifiers before vhost starts and stop the guest
> notifiers after vhost is stopped.
> - introduce the vhost_net_set_vq_index() and call it before setting
> guest notifiers. This is used to guarantee vhost_net has the correct
> virtqueue index when setting guest notifiers.
> 
> Reported-by: "Zhangjie (HZ)" 
> Cc: William Dauchy 
> Signed-off-by: Michael S. Tsirkin 
> Signed-off-by: Jason Wang 
> 
> --
> Changes from Michael's patch:
> - Remove the assertion
> Changes from V1:
> - Rebase to latest
> Changes from V2:
> - Introduce vhost_net_set_vq_index() to unbreak multiqueue

indeed I had with v2:
qemu-system-x86_64: hw/virtio/vhost.c:990: vhost_virtqueue_mask: Assertion `n 
>= hdev->vq_index && n < hdev->vq_index + hdev->nvqs' failed.

v3 seems ok to me
-- 
William


signature.asc
Description: Digital signature

[Qemu-devel] [PATCH] libdecnumber: Fix warnings from smatch (missing static, boolean operations)

[Stefan Weil]

Signed-off-by: Stefan Weil 
---
 libdecnumber/decNumber.c |   21 +++--
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/libdecnumber/decNumber.c b/libdecnumber/decNumber.c
index a30632f..58211e7 100644
--- a/libdecnumber/decNumber.c
+++ b/libdecnumber/decNumber.c
@@ -5275,8 +5275,8 @@ static decNumber * decMultiplyOp(decNumber *res, const 
decNumber *lhs,
 /* 4. The working precisions for the static buffers are twice the */
 /*obvious size to allow for calls from decNumberPower.   */
 /* -- */
-decNumber * decExpOp(decNumber *res, const decNumber *rhs,
-decContext *set, uInt *status) {
+static decNumber *decExpOp(decNumber *res, const decNumber *rhs,
+   decContext *set, uInt *status) {
   uInt ignore=0;  /* working status */
   Int h;  /* adjusted exponent for 0. */
   Int p;  /* working precision */
@@ -5563,7 +5563,8 @@ decNumber * decExpOp(decNumber *res, const decNumber *rhs,
 /*  where x is truncated (NB) into the range 10 through 99,  */
 /*  and then c = k>>2 and e = k&3.   */
 /* -- */
-const uShort LNnn[90]={9016,  8652,  8316,  8008,  7724,  7456, 7208,
+static const uShort LNnn[90] = {
+  9016,  8652,  8316,  8008,  7724,  7456,  7208,
   6972, 6748,  6540,  6340,  6148,  5968,  5792,  5628,  5464,  5312,
   5164, 5020,  4884,  4748,  4620,  4496,  4376,  4256,  4144,  4032,
  39233, 38181, 37157, 36157, 35181, 34229, 33297, 32389, 31501, 30629,
@@ -5635,8 +5636,8 @@ const uShort LNnn[90]={9016,  8652,  8316,  8008,  7724,  
7456,7208,
 /* 5. The static buffers are larger than might be expected to allow   */
 /*for calls from decNumberPower. */
 /* -- */
-decNumber * decLnOp(decNumber *res, const decNumber *rhs,
-   decContext *set, uInt *status) {
+static decNumber *decLnOp(decNumber *res, const decNumber *rhs,
+  decContext *set, uInt *status) {
   uInt ignore=0;  /* working status accumulator */
   uInt needbytes; /* for space calculations */
   Int residue;/* rounding residue */
@@ -6052,9 +6053,9 @@ static decNumber * decQuantizeOp(decNumber *res, const 
decNumber *lhs,
 /* The emphasis here is on speed for common cases, and avoiding  */
 /* coefficient comparison if possible.   */
 /* -- */
-decNumber * decCompareOp(decNumber *res, const decNumber *lhs,
-const decNumber *rhs, decContext *set,
-Flag op, uInt *status) {
+static decNumber *decCompareOp(decNumber *res, const decNumber *lhs,
+   const decNumber *rhs, decContext *set,
+   Flag op, uInt *status) {
   #if DECSUBSET
   decNumber *alloclhs=NULL;   /* non-NULL if rounded lhs allocated */
   decNumber *allocrhs=NULL;   /* .., rhs */
@@ -6086,11 +6087,11 @@ decNumber * decCompareOp(decNumber *res, const 
decNumber *lhs,
 
 /* If total ordering then handle differing signs 'up front' */
 if (op==COMPTOTAL) {   /* total ordering */
-  if (decNumberIsNegative(lhs) & !decNumberIsNegative(rhs)) {
+  if (decNumberIsNegative(lhs) && !decNumberIsNegative(rhs)) {
result=-1;
break;
}
-  if (!decNumberIsNegative(lhs) & decNumberIsNegative(rhs)) {
+  if (!decNumberIsNegative(lhs) && decNumberIsNegative(rhs)) {
result=+1;
break;
}
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH 3/3] ioh3420: Support ARI forwarding

[Gonglei (Arei)]

> >  static int ioh3420_initfn(PCIDevice *d)
> >  {
> > +PCIBridge* br = PCI_BRIDGE(d);
> >  PCIEPort *p = PCIE_PORT(d);
> >  PCIESlot *s = PCIE_SLOT(d);
> > +uint8_t *exp_cap;
> >  int rc;
> >  char tmp[100];
> >  sprintf(tmp, "pcie_port.%d", s->slot);
> > @@ -121,6 +123,10 @@ static int ioh3420_initfn(PCIDevice *d)
> >  if (rc < 0) {
> >  goto err_msi;
> >  }
> > +/* Support ARI forwarding */
> > +exp_cap = d->config + d->exp.exp_cap;
> > +pci_word_test_and_set_mask(exp_cap + PCI_EXP_DEVCAP2,
> PCI_EXP_DEVCAP2_ARI);
> > +
> Hi,
> 
> I think is better to move the implementation to a function, say
> pcie_cap_ari_init
> and place it into hw/pci/pcie.c so other devices like downstream ports can use
> it.
> 
Yes, good idea. And I noticed a function ioh3420_init(), but not be called.
Maybe it is useful.

Best regards,
-Gonglei

Re: [Qemu-devel] [PATCH 1/3] pcie: Fix next function setting

[Marcel Apfelbaum]

On Wed, 2014-08-20 at 08:52 +0200, Knut Omang wrote:
>   PCI_ARI_CAP_NFN is for reading next function not writing it
The commit message is not so clear, maybe something like
"Fix incorrect write to ari capability
or similar?

Other than that the patch looks OK to me.
Thanks,
Marcel
> 
> Signed-off-by: Knut Omang 
> ---
>  hw/pci/pcie.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> index a123c01..de0e967 100644
> --- a/hw/pci/pcie.c
> +++ b/hw/pci/pcie.c
> @@ -630,5 +630,5 @@ void pcie_ari_init(PCIDevice *dev, uint16_t offset, 
> uint16_t nextfn)
>  {
>  pcie_add_capability(dev, PCI_EXT_CAP_ID_ARI, PCI_ARI_VER,
>  offset, PCI_ARI_SIZEOF);
> -pci_set_long(dev->config + offset + PCI_ARI_CAP, 
> PCI_ARI_CAP_NFN(nextfn));
> +pci_set_long(dev->config + offset + PCI_ARI_CAP, (nextfn & 0xff) << 8);
>  }

Re: [Qemu-devel] [PATCH 2/2] pci: add check for pcie root ports and downstream ports

[Gonglei (Arei)]

> From: Marcel Apfelbaum [mailto:marce...@redhat.com]
> Subject: Re: [PATCH 2/2] pci: add check for pcie root ports and downstream
> ports
> 
> On Wed, 2014-08-20 at 03:20 +, Gonglei (Arei) wrote:
> > Hi,
> >
> > > > Right now, ARI Forwarding dose not support in QEMU.
> > > I would replace the above sentence with "ARI Forwarding is not supported".
> > >
> > OK.
> >
> > > By the way, there is some support for ARI, I don't know if
> > > is enabled yet. I'll have a look.
> > >
> > MST had pointed out the pcie_ari_init(), but not completed.
> >
> > > > According to PCIe spec section 7.3.1, only slot 0 with
> > > > the device attached to logic bus representing the link from
> > > > downstream ports and root ports.
> > > >
> > > > So, adding check about slot 0 for PCIe downstream ports and
> > > > root ports, which avoid useless operation, both hotplug and
> > > > coldplug.
> > > >
> > > > Signed-off-by: Gonglei 
> > > > ---
> > > >  hw/pci/pci.c | 41 +
> > > >  1 file changed, 41 insertions(+)
> > > >
> > > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> > > > index 351d320..f2d267f 100644
> > > > --- a/hw/pci/pci.c
> > > > +++ b/hw/pci/pci.c
> > > > @@ -773,6 +773,42 @@ static int pci_init_multifunction(PCIBus *bus,
> > > PCIDevice *dev)
> > > >  return 0;
> > > >  }
> > > >
> > > > +static int pci_check_pcie_port(PCIBus *bus, PCIDevice *dev)
> > > > +{
> > > > +Object *obj = OBJECT(bus);
> > > > +
> > > > +if (!strcmp(object_get_typename(obj), TYPE_PCIE_BUS)) {
> > > Maybe there is another way to check that this is a PCIe bus?
> > >
> > Yes. Paolo has said that object_dynamic_cast() is appropriated.
> >
> > > > +DeviceState *parent = qbus_get_parent(BUS(obj));
> > > > +const char *name = object_get_typename(OBJECT(parent));
> > > > +
> > > > +/*
> > > > + * Root ports and downstream ports of switches are the hot
> > > > + * pluggable ports in a PCI Express hierarchy.
> > > > + * PCI Express supports chip-to-chip interconnect, a PCIe link
> can
> > > > + * only connect one pci device/Switch/EndPoint or PCI-bridge.
> > > > + *
> > > > + * 7.3. Configuration Transaction Rules (PCI Express
> specification
> > > 3.0)
> > > > + * 7.3.1. Device Number
> > > > + *
> > > > + * Downstream Ports that do not have ARI Forwarding
> enabled
> > > must
> > > > + * associate only Device 0 with the device attached to the
> Logical
> > > Bus
> > > > + * representing the Link from the Port.
> > > > + *
> > > > + * Right now, ARI Forwarding dose not support. So, only slot 0
> is
> > > As above, maybe replace it with "ARI Forwarding is not supported"
> > >
> > OK.
> >
> > > > + * supported, regardless of hotplug or coldplug.
> > > > + */
> > > > +if (!strcmp(name, "ioh3420") || !strcmp(name,
> > > "xio3130-downstream")) {
> > > Please use port_type flag from extended configuration space, don't use
> device
> > > names.
> > > If you need help for this, let me know.
> > >
> > Yes, please. I appreciate very much that you can help me.
> Sure,
> 
> I checked and we already have the pcie_cap_get_type function that returns the
> port type.
> 
> port_type = pcie_cap_get_type(dev);
> if (port_type == PCI_EXP_TYPE_DOWNSTREAM ||
> port_type == PCI_EXP_TYPE_ROOT_PORT) {
> ...
> }
> 
Great! Thanks a lot. :)

Best regards,
-Gonglei

Re: [Qemu-devel] [PATCH bugfix v1 3/3] qom: object.h: Update object_get_canon_path* doc

[Peter Maydell]

On 20 August 2014 06:08, Peter Crosthwaite  wrote:
> With information about return value ownership.
>
> Signed-off-by: Peter Crosthwaite 
> ---
>
>  include/qom/object.h | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/include/qom/object.h b/include/qom/object.h
> index 8618e49..87de889 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -1029,7 +1029,8 @@ Object *object_get_root(void);
>   * object_get_canonical_path_component:
>   *
>   * Returns: The final component in the object's canonical path.  The 
> canonical
> - * path is the path within the composition tree starting from the root.
> + * path is the path within the composition tree starting from the root. The
> + * returned value may not be modified.
>   */
>  const gchar *object_get_canonical_path_component(Object *obj);

The other thing you need to say is that the returned string is
only valid for as long as the object remains a child property
of its parent. (Is that right? I'm not clear. It also sounds like
a really awkward lifetime management requirement, which
suggests to me that really the "return-a-copy" semantics are
nicer.)

Having object_get_canonical_path_component() and
object_get_canonical_path() having different return value
ownership semantics is likely to be a bit confusing I think.

If we do do this I think I'd put the doc comment change in
the same patch that changes the semantics.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 3/3] ioh3420: Support ARI forwarding

[Knut Omang]

On Wed, 2014-08-20 at 11:57 +0300, Marcel Apfelbaum wrote:
> On Wed, 2014-08-20 at 08:53 +0200, Knut Omang wrote:
> > Enable the PCIe capability bit that indicates that this port is able to 
> > support
> > and forward requests to > 8 functions for ARI capable devices.
> > 
> > Signed-off-by: Knut Omang 
> > ---
> >  hw/pci-bridge/ioh3420.c | 7 +++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/hw/pci-bridge/ioh3420.c b/hw/pci-bridge/ioh3420.c
> > index 8f6c8b0..c14ea6b 100644
> > --- a/hw/pci-bridge/ioh3420.c
> > +++ b/hw/pci-bridge/ioh3420.c
> > @@ -92,8 +92,10 @@ static void ioh3420_reset(DeviceState *qdev)
> >  
> >  static int ioh3420_initfn(PCIDevice *d)
> >  {
> > +PCIBridge* br = PCI_BRIDGE(d);
> >  PCIEPort *p = PCIE_PORT(d);
> >  PCIESlot *s = PCIE_SLOT(d);
> > +uint8_t *exp_cap;
> >  int rc;
> >  char tmp[100];
> >  sprintf(tmp, "pcie_port.%d", s->slot);
> > @@ -121,6 +123,10 @@ static int ioh3420_initfn(PCIDevice *d)
> >  if (rc < 0) {
> >  goto err_msi;
> >  }
> > +/* Support ARI forwarding */
> > +exp_cap = d->config + d->exp.exp_cap;
> > +pci_word_test_and_set_mask(exp_cap + PCI_EXP_DEVCAP2, 
> > PCI_EXP_DEVCAP2_ARI);
> > +
> Hi,
> 
> I think is better to move the implementation to a function, say 
> pcie_cap_ari_init
> and place it into hw/pci/pcie.c so other devices like downstream ports can 
> use it.

Yes, definitely - I'll fix that, thanks.
Knut

> Thanks,
> Marcel
> 
> >  pcie_cap_deverr_init(d);
> >  pcie_cap_slot_init(d, s->slot);
> >  pcie_chassis_create(s->chassis);
> > @@ -151,6 +157,7 @@ err_bridge:
> >  static void ioh3420_exitfn(PCIDevice *d)
> >  {
> >  PCIESlot *s = PCIE_SLOT(d);
> > +PCIBridge* br = PCI_BRIDGE(d);
> >  
> >  pcie_aer_exit(d);
> >  pcie_chassis_del_slot(s);
> 
> 
> 

Re: [Qemu-devel] [PATCH V3] vhost_net: start/stop guest notifiers properly

[Zhangjie (HZ)]

On 2014/8/19 12:56, Jason Wang wrote:
> commit a9f98bb5ebe6fb1869321dcc58e72041ae626ad8 vhost: multiqueue
 call it before setting
> Zhang Jie, please test this patch to see if it fixes the issue.

> +static void vhost_net_set_vq_index(struct vhost_net *net, int vq_index)
> +{
> +net->dev.vq_index = vq_index;
> +}
  int vq_index)
> ...
Because of vhost_net_set_vq_index, VM can be start successfully.
But, after about 80 times of migration under my environment, virtual nic became 
unreachable again.
When I use jprobe to notify tap, the virtual nic becomes reachable again. This 
shows that interrupts missing causes
the problem.
-- 
Best Wishes!
Zhang Jie

Re: [Qemu-devel] [PATCH 2/3] ioh3420: Provide a unique bus name and an interrupt mapping function

[Knut Omang]

On Wed, 2014-08-20 at 10:52 +0200, Paolo Bonzini wrote:
> Il 20/08/2014 08:53, Knut Omang ha scritto:
> > A unique bus name is necessary to be able to refer to each instance
> > from the command line and monitors.
> 
> Is it needed?  Can't you just add id= to the -device option?

Yes, as far as I understand the problem is that the id= would work on
the ioh3420 device itself, while what is needed here is to name the
secondary bus of the ioh3420, which I haven't found a way to name from
the command line.

Maybe an even better solution would be to have default names for
everything, if not specified, from a user friendliness perspective? 

I suppose this is a more general issue of sensible default values
though, but the fact that it is easy to create devices which cannot be
referred has caused me some confusion from time to time.

> Paolo

Thanks,

Knut

Re: [Qemu-devel] [PATCH 1/4] hd-geometry.c: Integrate HDIO_GETGEO in guessing for target-s390x

[Christian Borntraeger]

On 20/08/14 10:19, Paolo Bonzini wrote:
> Il 29/07/2014 14:27, Ekaterina Tumanova ha scritto:
>> The new HDIO_GETGEO logic is required for two use cases:
>> a) Support for geometries of Direct Attached Storage Disks (DASD)
>> on s390x configured as backing of virtio block devices.
> 
> Is this still relevant now that QEMU can emulate 512-byte sectors on top
> of host devices with 4k sectors?

Yes, the geometry and the block size define the layout of the DASD disk format. 
This defines how the ibm disk layout partition table looks like. So partition 
detection of the IBM layout only works if values are correct. (see linux 
block/partitions/ibm.c. it needs these values to correctly locate the data 
structures).

Furthermore, if you do an mkfs in the host and the guest sees a different block 
size all kind of strange things will happen when mounting, no?

> 
>> b) Support for FCP attached SCSI disks that do not yet have a
>> partition table. Without this patch, fdisk -l on the host would
>> return different results then fdisk -l in the guest.
> 
> Can you provide an example?

scsi disk in the host:
[root@host ~]#  sfdisk -g /dev/disk/by-id/wwn-0x6005076305ffc1ae2593
/dev/disk/by-id/wwn-0x6005076305ffc1ae2593: 1011 cylinders, 34 
heads, 61 sectors/track

same scsi disk in the guest as virtio-blk:
scsi disk in the guest:
[root@guest ~]# sfdisk -g /dev/disk/by-id/virtio-d20
/dev/disk/by-id/virtio-d20: 2080 cylinders, 16 heads, 63 sectors/track

16/63 is currently hardcoded by QEMU, no matter what the host thinks. This gets 
overridden as soon as there is a partition table.

command line was:
-drive 
file=/dev/disk/by-id/scsi-36005076305ffc1ae2593,if=none,id=drive-virtio-disk20,format=raw,serial=d20,cache=none,aio=native

> 
> Paolo
> 

Re: [Qemu-devel] [PULL 00/19] target-arm queue

[Peter Maydell]

On 19 August 2014 19:09, Peter Maydell  wrote:
> Flushing my queue of reviewed ARM patches: single step,
> plus a collection of straightforward patches from other
> people.
>
> thanks
> -- PMM
>
>
> The following changes since commit 0e4a77370594c91dd126f9872893ed473374cc72:
>
>   Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into 
> staging (2014-08-19 13:00:57 +0100)
>
> are available in the git repository at:
>
>
>   git://git.linaro.org/people/pmaydell/qemu-arm.git 
> tags/pull-target-arm-20140819
>
> for you to fetch changes up to 14a906f755f77b325666d67e071c572478d06067:
>
>   arm: stellaris: Remove misleading address_space_mem var (2014-08-19 
> 19:02:40 +0100)
>
> 
> target-arm:
>  * fix preferred return address for A64 BRK insn
>  * implement AArch64 single-stepping
>  * support loading gzip compressed AArch64 kernels
>  * use correct PSCI function IDs in the DT when KVM uses PSCI 0.2
>  * minor cleanups
>
> 
> Christoffer Dall (2):
>   target-arm: Rename QEMU PSCI v0.1 definitions
>   arm/virt: Use PSCI v0.2 function IDs in the DT when KVM uses PSCI v0.2
>
> Peter Crosthwaite (3):
>   arm: cortex-a9: Fix cache-line size and associativity
>   arm: armv7m: Rename address_space_mem -> system_memory
>   arm: stellaris: Remove misleading address_space_mem var
>
> Peter Maydell (12):
>   target-arm: Fix return address for A64 BRK instructions
>   target-arm: Collect up the debug cp register definitions
>   target-arm: Allow STATE_BOTH reginfo descriptions for more than cp14
>   target-arm: Provide both 32 and 64 bit versions of debug registers
>   target-arm: Adjust debug ID registers per-CPU
>   target-arm: Don't allow AArch32 to access RES0 CPSR bits
>   target-arm: Correctly handle PSTATE.SS when taking exception to AArch32
>   target-arm: Set PSTATE.SS correctly on exception return from AArch64
>   target-arm: A64: Avoid duplicate exit_tb(0) in non-linked goto_tb
>   target-arm: Implement ARMv8 single-step handling for A64 code
>   target-arm: Implement ARMv8 single-stepping for AArch32 code
>   target-arm: Implement MDSCR_EL1 as having state
>
> Richard W.M. Jones (2):
>   loader: Add load_image_gzipped function.
>   aarch64: Allow -kernel option to take a gzip-compressed kernel.

Applied, thanks.

-- PMM

[Qemu-devel] [RFC PATCH 1/2] VFIO: Drop vfio_container_do_ioctl()

[Gavin Shan]

The patch drops vfio_container_do_ioctl() and merges its logic to
parent function call vfio_container_ioctl() so that the subsequent
patches can reused the found VFIO group in vfio_container_ioctl().

Signed-off-by: Gavin Shan 
---
 hw/misc/vfio.c | 33 +++--
 1 file changed, 15 insertions(+), 18 deletions(-)

diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c
index 7d5f1bb..1a3e7eb 100644
--- a/hw/misc/vfio.c
+++ b/hw/misc/vfio.c
@@ -4419,8 +4419,8 @@ static void register_vfio_pci_dev_type(void)
 
 type_init(register_vfio_pci_dev_type)
 
-static int vfio_container_do_ioctl(AddressSpace *as, int32_t groupid,
-   int req, void *param)
+int vfio_container_ioctl(AddressSpace *as, int32_t groupid,
+ int req, void *param)
 {
 VFIOGroup *group;
 VFIOContainer *container;
@@ -4433,22 +4433,11 @@ static int vfio_container_do_ioctl(AddressSpace *as, 
int32_t groupid,
 }
 
 container = group->container;
-if (group->container) {
-ret = ioctl(container->fd, req, param);
-if (ret < 0) {
-error_report("vfio: failed to ioctl container: ret=%d, %s",
- ret, strerror(errno));
-}
+if (!container) {
+error_report("vfio: no container for group %d\n", groupid);
+goto out;
 }
 
-vfio_put_group(group);
-
-return ret;
-}
-
-int vfio_container_ioctl(AddressSpace *as, int32_t groupid,
- int req, void *param)
-{
 /* We allow only certain ioctls to the container */
 switch (req) {
 case VFIO_CHECK_EXTENSION:
@@ -4458,8 +4447,16 @@ int vfio_container_ioctl(AddressSpace *as, int32_t 
groupid,
 default:
 /* Return an error on unknown requests */
 error_report("vfio: unsupported ioctl %X", req);
-return -1;
+goto out;
 }
 
-return vfio_container_do_ioctl(as, groupid, req, param);
+ret = ioctl(container->fd, req, param);
+if (ret < 0) {
+error_report("vfio: failed to ioctl container: ret=%d, %s",
+ ret, strerror(errno));
+}
+
+out:
+vfio_put_group(group);
+return ret;
 }
-- 
1.8.3.2

[Qemu-devel] [RFC PATCH 2/2] VFIO: Clear stale MSIx table during EEH reset

[Gavin Shan]

The PCI device MSIx table is cleaned out in hardware after EEH PE
reset. However, we still hold the stale MSIx entries in QEMU, which
should be cleared accordingly. Otherwise, we will run into another
(recursive) EEH error and the PCI devices contained in the PE have
to be offlined exceptionally.

The patch clears stale MSIx table before EEH PE reset so that MSIx
table could be restored properly after EEH PE reset.

Signed-off-by: Gavin Shan 
---
 hw/misc/vfio.c | 24 
 1 file changed, 24 insertions(+)

diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c
index 1a3e7eb..3cf7f02 100644
--- a/hw/misc/vfio.c
+++ b/hw/misc/vfio.c
@@ -4424,6 +4424,8 @@ int vfio_container_ioctl(AddressSpace *as, int32_t 
groupid,
 {
 VFIOGroup *group;
 VFIOContainer *container;
+VFIODevice *vdev;
+struct vfio_eeh_pe_op *arg;
 int ret = -1;
 
 group = vfio_get_group(groupid, as);
@@ -4442,7 +,29 @@ int vfio_container_ioctl(AddressSpace *as, int32_t 
groupid,
 switch (req) {
 case VFIO_CHECK_EXTENSION:
 case VFIO_IOMMU_SPAPR_TCE_GET_INFO:
+break;
 case VFIO_EEH_PE_OP:
+arg = (struct vfio_eeh_pe_op *)param;
+switch (arg->op) {
+case VFIO_EEH_PE_RESET_HOT:
+case VFIO_EEH_PE_RESET_FUNDAMENTAL:
+/*
+ * The MSIx table will be cleaned out by reset. We need
+ * disable it so that it can be reenabled properly. Also,
+ * the cached MSIx table should be cleared as it's not
+ * reflecting the contents in hardware.
+ */
+QLIST_FOREACH(vdev, &group->device_list, next) {
+if (msix_enabled(&vdev->pdev)) {
+vfio_disable_msix(vdev);
+}
+
+msix_reset(&vdev->pdev);
+}
+
+break;
+}
+
 break;
 default:
 /* Return an error on unknown requests */
-- 
1.8.3.2

[Qemu-devel] [RFC PATCH 0/2] Fix MSIx lost after PE reset

[Gavin Shan]

The 2 patches fix MSIx lost after PE reset. Otherwise, the MSIx
entries can't be restored successfully after PE reset and the
EEH recovery fails on broadcom tg3 adapter (as tested) in guest.

Note: The patchset "EEH support for guest" isn't merged yet, those
  2 patches are based on Alex Graf's "ppc-next" branch + the
  patchset supporting EEH for guest, which can be checked out
  from below link:

  g...@github.com:gwshan/qemu.git (branch: eeh)

Gavin Shan (2):
  VFIO: Drop vfio_container_do_ioctl()
  VFIO: Clear stale MSIx table during EEH reset

 hw/misc/vfio.c | 57 +++--
 1 file changed, 39 insertions(+), 18 deletions(-)

-- 
1.8.3.2

Re: [Qemu-devel] [PATCH 0/2] block: support block_resize with dataplane

[Kevin Wolf]

Am 18.08.2014 um 15:52 hat Stefan Hajnoczi geschrieben:
> We have the op blocker and AioContext acquire/release infrastructure to safely
> allow block monitor commands on dataplane commands.  It is now easy to make
> block_resize safe.
> 
> Stefan Hajnoczi (2):
>   block: acquire AioContext in qmp_block_resize()
>   virtio-blk: allow block_resize with dataplane
> 
>  blockdev.c  | 13 ++---
>  hw/block/dataplane/virtio-blk.c |  1 +
>  2 files changed, 11 insertions(+), 3 deletions(-)

Thanks, applied all to the block branch.

Kevin

Re: [Qemu-devel] [PATCH v7] virtio-blk: fix reference a pointer which might be freed

[Kevin Wolf]

Am 18.08.2014 um 10:08 hat Paolo Bonzini geschrieben:
> Il 18/08/2014 09:42, zhanghailiang ha scritto:
> > In function virtio_blk_handle_request, it may freed memory pointed by req,
> > So do not access member of req after calling this function.
> > 
> > Reviewed-by: Michael S. Tsirkin 
> > Reviewed-by: Stefan Hajnoczi 
> > Signed-off-by: zhanghailiang 
> 
> Cc: qemu-sta...@nongnu.org

Thanks, added the Cc: line and applied to the block branch.

Kevin

[Qemu-devel] [PATCH 6/6] iscsi: Move iqn generation code to util

[Fam Zheng]

Function qmp_query_uuid, even with a version in libqemustub.a, is not
present in qemu-img, unless we move it to something that is linked with
block-obj-y. Since it's a helper function, move it to util makes sense.

Signed-off-by: Fam Zheng 
---
 block/iscsi.c | 15 +--
 include/qemu-common.h |  3 +++
 util/Makefile.objs|  1 +
 util/iqn.c| 38 ++
 4 files changed, 43 insertions(+), 14 deletions(-)
 create mode 100644 util/iqn.c

diff --git a/block/iscsi.c b/block/iscsi.c
index 2c9cfc1..b09f539 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -37,8 +37,6 @@
 #include "trace.h"
 #include "block/scsi.h"
 #include "qemu/iov.h"
-#include "sysemu/sysemu.h"
-#include "qmp-commands.h"
 
 #include 
 #include 
@@ -1034,8 +1032,6 @@ static char *parse_initiator_name(const char *target)
 QemuOptsList *list;
 QemuOpts *opts;
 const char *name;
-char *iscsi_name;
-UuidInfo *uuid_info;
 
 list = qemu_find_opts("iscsi");
 if (list) {
@@ -1051,16 +1047,7 @@ static char *parse_initiator_name(const char *target)
 }
 }
 
-uuid_info = qmp_query_uuid(NULL);
-if (strcmp(uuid_info->UUID, UUID_NONE) == 0) {
-name = qemu_get_vm_name();
-} else {
-name = uuid_info->UUID;
-}
-iscsi_name = g_strdup_printf("iqn.2008-11.org.linux-kvm%s%s",
- name ? ":" : "", name ? name : "");
-qapi_free_UuidInfo(uuid_info);
-return iscsi_name;
+return iqn_generate("iqn.2008-11.org.linux-kvm");
 }
 
 static void iscsi_nop_timed_event(void *opaque)
diff --git a/include/qemu-common.h b/include/qemu-common.h
index bcf7a6a..ba7700f 100644
--- a/include/qemu-common.h
+++ b/include/qemu-common.h
@@ -420,6 +420,9 @@ int uleb128_decode_small(const uint8_t *in, uint32_t *n);
 /* unicode.c */
 int mod_utf8_codepoint(const char *s, size_t n, char **end);
 
+/* iqn.c */
+char *iqn_generate(const char *prefix);
+
 /*
  * Hexdump a buffer to a file. An optional string prefix is added to every line
  */
diff --git a/util/Makefile.objs b/util/Makefile.objs
index 65a36f6..5c043e9 100644
--- a/util/Makefile.objs
+++ b/util/Makefile.objs
@@ -14,3 +14,4 @@ util-obj-y += throttle.o
 util-obj-y += getauxval.o
 util-obj-y += readline.o
 util-obj-y += rfifolock.o
+util-obj-y += iqn.o
diff --git a/util/iqn.c b/util/iqn.c
new file mode 100644
index 000..c8d1eda
--- /dev/null
+++ b/util/iqn.c
@@ -0,0 +1,38 @@
+/*
+ * iqn generat function
+ *
+ * Copyright Red Hat, Inc., 2014
+ *
+ * Author: Paolo Bonzini 
+ * Fam Zheng 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * later.  See the COPYING file in the top-level directory.
+ */
+
+#include 
+#include "qemu/error-report.h"
+#include "qemu-common.h"
+#include "sysemu/sysemu.h"
+#include "qmp-commands.h"
+
+char *iqn_generate(const char *prefix)
+{
+const char *name;
+char *iqn;
+UuidInfo *uuid_info;
+
+uuid_info = qmp_query_uuid(NULL);
+if (strcmp(uuid_info->UUID, UUID_NONE) == 0) {
+name = qemu_get_vm_name();
+} else {
+name = uuid_info->UUID;
+}
+iqn = g_strdup_printf("%s%s%s",
+ prefix,
+ name ? ":" : "",
+ name ? : "");
+qapi_free_UuidInfo(uuid_info);
+
+return iqn;
+}
-- 
2.0.3

Re: [Qemu-devel] [PATCH 1/4] qcow2: Add qcow2_signal_corruption()

[Kevin Wolf]

Am 16.08.2014 um 23:16 hat Max Reitz geschrieben:
> Add a helper function for easily marking an image corrupt while
> outputting an informative message to stderr and via QAPI.
> 
> Signed-off-by: Max Reitz 
> ---
>  block/qcow2.c | 28 
>  block/qcow2.h |  4 
>  2 files changed, 32 insertions(+)
> 
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 435e0e1..ef2c931 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -30,6 +30,8 @@
>  #include "qemu/error-report.h"
>  #include "qapi/qmp/qerror.h"
>  #include "qapi/qmp/qbool.h"
> +#include "qapi/qmp/types.h"
> +#include "qapi-event.h"
>  #include "trace.h"
>  #include "qemu/option_int.h"
>  
> @@ -2378,6 +2380,32 @@ static int qcow2_amend_options(BlockDriverState *bs, 
> QemuOpts *opts)
>  return 0;
>  }
>  
> +void qcow2_signal_corruption(BlockDriverState *bs, int64_t offset, int64_t 
> size,
> + const char *message_format, ...)
> +{
> +char *message;
> +va_list ap;
> +
> +va_start(ap, message_format);
> +message = g_strdup_vprintf(message_format, ap);
> +va_end(ap);
> +
> +if (bs->read_only) {
> +fprintf(stderr, "qcow2: Image is corrupt: %s\n", message);

The BDS isn't made unusable in read-only mode, so we can produce quite a
lot of these messages and fill up the log. Perhaps it would be better to
print the message only the first time (or the first n times) and then
tell the user that further errors won't be logged.

Also, including the block device or file name (or both) wouldn't hurt.

> +} else {
> +fprintf(stderr, "qcow2: Marking image as corrupt: %s\n", message);
> +qapi_event_send_block_image_corrupted(bdrv_get_device_name(bs), 
> message,
> +  offset >= 0, offset,
> +  size >= 0, size, &error_abort);
> +}
> +g_free(message);
> +
> +if (!bs->read_only) {
> +qcow2_mark_corrupt(bs);
> +bs->drv = NULL; /* make BDS unusable */
> +}
> +}

Kevin

Re: [Qemu-devel] [PATCH 3/4] iotests: Fix output of 060

[Kevin Wolf]

Am 16.08.2014 um 23:16 hat Max Reitz geschrieben:
> With qcow2_pre_write_overlap_check() relying on
> qcow2_signal_corruption(), the output in case of a corruption changes.
> Therefore, 060's output has to be adapted accordingly.
> 
> Signed-off-by: Max Reitz 

Squash this into patch 2 so that the test isn't broken at any point
during the series.

Kevin

Re: [Qemu-devel] [PATCH V3] vhost_net: start/stop guest notifiers properly

[Michael S. Tsirkin]

On Wed, Aug 20, 2014 at 05:23:21PM +0800, Zhangjie (HZ) wrote:
> On 2014/8/19 12:56, Jason Wang wrote:
> > commit a9f98bb5ebe6fb1869321dcc58e72041ae626ad8 vhost: multiqueue
>  call it before setting
> > Zhang Jie, please test this patch to see if it fixes the issue.
> 
> > +static void vhost_net_set_vq_index(struct vhost_net *net, int vq_index)
> > +{
> > +net->dev.vq_index = vq_index;
> > +}
>   int vq_index)
> > ...
> Because of vhost_net_set_vq_index, VM can be start successfully.
> But, after about 80 times of migration under my environment, virtual nic 
> became unreachable again.
> When I use jprobe to notify tap, the virtual nic becomes reachable again. 
> This shows that interrupts missing causes
> the problem.

Could you please clarify what do you mean by "notify tap" here?
Thanks!

> -- 
> Best Wishes!
> Zhang Jie

Re: [Qemu-devel] [PATCH 3/3] ioh3420: Support ARI forwarding

[Knut Omang]

On Wed, 2014-08-20 at 09:02 +, Gonglei (Arei) wrote:
> > >  static int ioh3420_initfn(PCIDevice *d)
> > >  {
> > > +PCIBridge* br = PCI_BRIDGE(d);
> > >  PCIEPort *p = PCIE_PORT(d);
> > >  PCIESlot *s = PCIE_SLOT(d);
> > > +uint8_t *exp_cap;
> > >  int rc;
> > >  char tmp[100];
> > >  sprintf(tmp, "pcie_port.%d", s->slot);
> > > @@ -121,6 +123,10 @@ static int ioh3420_initfn(PCIDevice *d)
> > >  if (rc < 0) {
> > >  goto err_msi;
> > >  }
> > > +/* Support ARI forwarding */
> > > +exp_cap = d->config + d->exp.exp_cap;
> > > +pci_word_test_and_set_mask(exp_cap + PCI_EXP_DEVCAP2,
> > PCI_EXP_DEVCAP2_ARI);
> > > +
> > Hi,
> > 
> > I think is better to move the implementation to a function, say
> > pcie_cap_ari_init
> > and place it into hw/pci/pcie.c so other devices like downstream ports can 
> > use
> > it.
> > 
> Yes, good idea. And I noticed a function ioh3420_init(), but not be called.
> Maybe it is useful.

Yes, I noticed too - I would guess this is something left over from an
earlier stage of the qdev APIs - Isaku, should I add a patch to remove
it while at this?

> Best regards,
> -Gonglei
> 

Re: [Qemu-devel] [PATCH v7 3/4] s390x: Migrate to new NMI interface

[Alexey Kardashevskiy]

On 07/17/2014 04:30 PM, Alexey Kardashevskiy wrote:
> On 07/15/2014 06:29 AM, Paolo Bonzini wrote:
>> Il 14/07/2014 05:17, Alexey Kardashevskiy ha scritto:
> No, you should politely bother them again.
>>>
>>> That does not seem helping though :-/
>>
>> Sorry. :)
>>
>> Well, patch 1 is the same as v6 and for the others Cornelia and Alex can
>> ack it.  It looks good to me though.
> 
> 
> Let me clarify things here.
> 
> Alex rb'ed to 0/4, Cornelia rb'ed 2/4 and 3/4, so I put Alex's rb to
> 1&2&3&4&5, Cornelia's rb to 2&3, rebase it on top of the current upstream,
> repost after 2.1 is released and resume annoying people again. Is that 
> correct?
> 
> This is my current set:
> 5/5 pc_piix: Migrate to new NMI interface
> 4/5 spapr: Add support for new NMI interface
> 3/5 s390x: Migrate to new NMI interface
> 2/5 s390x: Convert QEMUMachine to MachineClass
> 1/5 cpus: Define callback for QEMU "nmi" command


Alex, ping.


ps. anecdot.
"what is your problem?"
"doctor, everybody ignores me"
"ah I see. next patient, please"
:)





-- 
Alexey

[Qemu-devel] Recent SeaBIOS too big for QEMU -initrd

[Stefan Hajnoczi]

The QEMU -initrd option loads the initrd at the top of RAM.  There is
a 64 KB safety region for ACPI tables in hw/i386/pc.c:load_linux():

  initrd_max = max_ram_size-ACPI_DATA_SIZE-1;

QEMU's bios-256k.bin SeaBIOS build reserves 128 KB at the top of
memory so the 64 KB ACPI data size has become too small.

The guest Linux kernel rejects the initrd:
BIOS-provided physical RAM map:
 BIOS-e820: 0001 - 0009fc00 (usable)
 BIOS-e820: 0009fc00 - 000a (reserved)
 BIOS-e820: 000f - 0010 (reserved)
 BIOS-e820: 0010 - 3ffe (usable)
 BIOS-e820: 3ffe - 4000 (reserved)
 BIOS-e820: feffc000 - ff00 (reserved)
 BIOS-e820: fffc - 0001 (reserved)
...
initrd extends beyond end of memory (0x3ffef79f > 0x3ffe)
disabling initrd

It is easy enough to "fix" the problem by bumping ACPI_DATA_SIZE up to
0x2 in QEMU.  Perhaps this should only be done for bios-256k.bin
guests and not bios-128k.bin guests (QEMU 1.7 and older machine
types).

Perhaps QEMU -> SeaBIOS -> linuxboot.bin can be simplified so QEMU
doesn't have to guess what e820 region SeaBIOS will reserve.
linuxboot.bin would probably be the place to do it unless SeaBIOS has
Linux loading functionality that could be reused.

Any ideas?

Stefan

[Qemu-devel] [PATCH 5/6] build-sys: Change libqemuutil.a to qemuutil.o and link whole object

[Fam Zheng]

When an executable is being generated, unused functions from
libqemuutil.a are not linked. This is the linker's convention on
archives (libqemuutil.a).

Now that we have dynamically loaded modules, which may reference
function from libqemuutil.a but not linked in the executable, because
the executable itself didn't reference this symbol. That is a problem
for module build.

We can't link both executable and the dynamic shared object to
libqemuutil.a, because of the risk of inconsistent views of program
variables: DSO module sees a copy of some data because it is linked
against libqemuutil.a, whereas the executable sees another copy. In
other words, they each maintains a copy but with a same name. In this
case, it can be very tricky to notice such a duplication, and make a bug
hard to reason. So it's good to avoid it from the beginning.

This patch solves the above issue by fully linking. Specifically, it
fixes block-iscsi.mo: in block/iscsi.c, util/bitmap.c functions are
used, but qemu-img doesn't link it.

The solution is to link everything in libqemuutil.a. We do this by
changing it to qemuutil.o, which includes all the util objects. This is
easier and also expected to be more portable than "--whole-archive".

Because qemuutil.o is now fully linked and hence make executables
references more symbols than before, some test executables now need
libqemustub.a, so add them as necessary too.

Signed-off-by: Fam Zheng 
---
 Makefile| 17 +---
 Makefile.objs   |  2 +-
 Makefile.target |  2 +-
 tests/Makefile  | 60 -
 4 files changed, 42 insertions(+), 39 deletions(-)

diff --git a/Makefile b/Makefile
index b33aaac..7cf5e51 100644
--- a/Makefile
+++ b/Makefile
@@ -187,7 +187,7 @@ subdir-dtc:dtc/libfdt dtc/tests
 dtc/%:
mkdir -p $@
 
-$(SUBDIR_RULES): libqemuutil.a libqemustub.a $(common-obj-y)
+$(SUBDIR_RULES): qemuutil.o libqemustub.a $(common-obj-y)
 
 ROMSUBDIR_RULES=$(patsubst %,romsubdir-%, $(ROMS))
 romsubdir-%:
@@ -208,7 +208,10 @@ Makefile: $(version-obj-y) $(version-lobj-y)
 # Build libraries
 
 libqemustub.a: $(stub-obj-y)
-libqemuutil.a: $(util-obj-y)
+
+qemuutil.o: CC_REL_FLAGS := -Wl,-r
+qemuutil.o: $(util-obj-y)
+   $(call quiet-command,$(CC) -nostdlib $(CC_REL_FLAGS) -o $@ $^,"  LD -r  
$(TARGET_DIR)$@")
 
 block-modules = $(foreach o,$(block-obj-m),"$(basename $(subst /,-,$o))",) NULL
 util/module.o-cflags = -D'CONFIG_BLOCK_MODULES=$(block-modules)'
@@ -217,13 +220,13 @@ util/module.o-cflags = 
-D'CONFIG_BLOCK_MODULES=$(block-modules)'
 
 qemu-img.o: qemu-img-cmds.h
 
-qemu-img$(EXESUF): qemu-img.o $(block-obj-y) libqemuutil.a libqemustub.a
-qemu-nbd$(EXESUF): qemu-nbd.o $(block-obj-y) libqemuutil.a libqemustub.a
-qemu-io$(EXESUF): qemu-io.o $(block-obj-y) libqemuutil.a libqemustub.a
+qemu-img$(EXESUF): qemu-img.o $(block-obj-y) qemuutil.o libqemustub.a
+qemu-nbd$(EXESUF): qemu-nbd.o $(block-obj-y) qemuutil.o libqemustub.a
+qemu-io$(EXESUF): qemu-io.o $(block-obj-y) qemuutil.o libqemustub.a
 
 qemu-bridge-helper$(EXESUF): qemu-bridge-helper.o
 
-fsdev/virtfs-proxy-helper$(EXESUF): fsdev/virtfs-proxy-helper.o 
fsdev/virtio-9p-marshal.o libqemuutil.a libqemustub.a
+fsdev/virtfs-proxy-helper$(EXESUF): fsdev/virtfs-proxy-helper.o 
fsdev/virtio-9p-marshal.o qemuutil.o libqemustub.a
 fsdev/virtfs-proxy-helper$(EXESUF): LIBS += -lcap
 
 qemu-img-cmds.h: $(SRC_PATH)/qemu-img-cmds.hx
@@ -280,7 +283,7 @@ $(qapi-modules) $(SRC_PATH)/scripts/qapi-commands.py 
$(qapi-py)
 QGALIB_GEN=$(addprefix qga/qapi-generated/, qga-qapi-types.h qga-qapi-visit.h 
qga-qmp-commands.h)
 $(qga-obj-y) qemu-ga.o: $(QGALIB_GEN)
 
-qemu-ga$(EXESUF): $(qga-obj-y) libqemuutil.a libqemustub.a
+qemu-ga$(EXESUF): $(qga-obj-y) qemuutil.o libqemustub.a
$(call LINK, $^)
 
 clean:
diff --git a/Makefile.objs b/Makefile.objs
index 97db978..4847da7 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -109,6 +109,6 @@ target-obj-y += trace/
 # guest agent
 
 # FIXME: a few definitions from qapi-types.o/qapi-visit.o are needed
-# by libqemuutil.a.  These should be moved to a separate .json schema.
+# by qemuutil.o.  These should be moved to a separate .json schema.
 qga-obj-y = qga/
 qga-vss-dll-obj-y = qga/
diff --git a/Makefile.target b/Makefile.target
index 1e8d7ab..48a3089 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -176,7 +176,7 @@ all-obj-y += $(target-obj-y)
 all-obj-$(CONFIG_SOFTMMU) += $(block-obj-y)
 
 # build either PROG or PROGW
-$(QEMU_PROG_BUILD): $(all-obj-y) ../libqemuutil.a ../libqemustub.a
+$(QEMU_PROG_BUILD): $(all-obj-y) ../qemuutil.o ../libqemustub.a
$(call LINK,$^)
 
 gdbstub-xml.c: $(TARGET_XML_FILES) $(SRC_PATH)/scripts/feature_to_c.sh
diff --git a/tests/Makefile b/tests/Makefile
index 837e9c8..b79a299 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -226,22 +226,22 @@ qom-core-obj = qom/object.o qom/qom-qobject.o 
qom/container.o
 
 tests/test-x86-cpuid.o: QEMU_INCLUDES += -I$(SRC_PATH)/target-i386
 
-tests/ch

[Qemu-devel] [PATCH 1/6] build-sys: Move fifio8 to hw/

[Fam Zheng]

Since it has a dependency on vmstate and is only used by device
emulation, moving out from util will make the archive more independent.

Signed-off-by: Fam Zheng 
---
 hw/Makefile.objs   |   1 +
 hw/fifo8.c | 125 +
 util/Makefile.objs |   1 -
 util/fifo8.c   | 125 -
 4 files changed, 126 insertions(+), 126 deletions(-)
 create mode 100644 hw/fifo8.c
 delete mode 100644 util/fifo8.c

diff --git a/hw/Makefile.objs b/hw/Makefile.objs
index 52a1464..bc77dd3 100644
--- a/hw/Makefile.objs
+++ b/hw/Makefile.objs
@@ -33,3 +33,4 @@ devices-dirs-$(CONFIG_MEM_HOTPLUG) += mem/
 devices-dirs-y += core/
 common-obj-y += $(devices-dirs-y)
 obj-y += $(devices-dirs-y)
+obj-y += fifo8.o
diff --git a/hw/fifo8.c b/hw/fifo8.c
new file mode 100644
index 000..0ea5ad9
--- /dev/null
+++ b/hw/fifo8.c
@@ -0,0 +1,125 @@
+/*
+ * Generic FIFO component, implemented as a circular buffer.
+ *
+ * Copyright (c) 2012 Peter A. G. Crosthwaite
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see .
+ */
+
+#include "qemu-common.h"
+#include "qemu/fifo8.h"
+
+void fifo8_create(Fifo8 *fifo, uint32_t capacity)
+{
+fifo->data = g_new(uint8_t, capacity);
+fifo->capacity = capacity;
+fifo->head = 0;
+fifo->num = 0;
+}
+
+void fifo8_destroy(Fifo8 *fifo)
+{
+g_free(fifo->data);
+}
+
+void fifo8_push(Fifo8 *fifo, uint8_t data)
+{
+if (fifo->num == fifo->capacity) {
+abort();
+}
+fifo->data[(fifo->head + fifo->num) % fifo->capacity] = data;
+fifo->num++;
+}
+
+void fifo8_push_all(Fifo8 *fifo, const uint8_t *data, uint32_t num)
+{
+uint32_t start, avail;
+
+if (fifo->num + num > fifo->capacity) {
+abort();
+}
+
+start = (fifo->head + fifo->num) % fifo->capacity;
+
+if (start + num <= fifo->capacity) {
+memcpy(&fifo->data[start], data, num);
+} else {
+avail = fifo->capacity - start;
+memcpy(&fifo->data[start], data, avail);
+memcpy(&fifo->data[0], &data[avail], num - avail);
+}
+
+fifo->num += num;
+}
+
+uint8_t fifo8_pop(Fifo8 *fifo)
+{
+uint8_t ret;
+
+if (fifo->num == 0) {
+abort();
+}
+ret = fifo->data[fifo->head++];
+fifo->head %= fifo->capacity;
+fifo->num--;
+return ret;
+}
+
+const uint8_t *fifo8_pop_buf(Fifo8 *fifo, uint32_t max, uint32_t *num)
+{
+uint8_t *ret;
+
+if (max == 0 || max > fifo->num) {
+abort();
+}
+*num = MIN(fifo->capacity - fifo->head, max);
+ret = &fifo->data[fifo->head];
+fifo->head += *num;
+fifo->head %= fifo->capacity;
+fifo->num -= *num;
+return ret;
+}
+
+void fifo8_reset(Fifo8 *fifo)
+{
+fifo->num = 0;
+fifo->head = 0;
+}
+
+bool fifo8_is_empty(Fifo8 *fifo)
+{
+return (fifo->num == 0);
+}
+
+bool fifo8_is_full(Fifo8 *fifo)
+{
+return (fifo->num == fifo->capacity);
+}
+
+uint32_t fifo8_num_free(Fifo8 *fifo)
+{
+return fifo->capacity - fifo->num;
+}
+
+uint32_t fifo8_num_used(Fifo8 *fifo)
+{
+return fifo->num;
+}
+
+const VMStateDescription vmstate_fifo8 = {
+.name = "Fifo8",
+.version_id = 1,
+.minimum_version_id = 1,
+.fields = (VMStateField[]) {
+VMSTATE_VBUFFER_UINT32(data, Fifo8, 1, NULL, 0, capacity),
+VMSTATE_UINT32(head, Fifo8),
+VMSTATE_UINT32(num, Fifo8),
+VMSTATE_END_OF_LIST()
+}
+};
diff --git a/util/Makefile.objs b/util/Makefile.objs
index 6b3c83b..65a36f6 100644
--- a/util/Makefile.objs
+++ b/util/Makefile.objs
@@ -3,7 +3,6 @@ util-obj-$(CONFIG_WIN32) += oslib-win32.o qemu-thread-win32.o 
event_notifier-win
 util-obj-$(CONFIG_POSIX) += oslib-posix.o qemu-thread-posix.o 
event_notifier-posix.o qemu-openpty.o
 util-obj-y += envlist.o path.o host-utils.o module.o
 util-obj-y += bitmap.o bitops.o hbitmap.o
-util-obj-y += fifo8.o
 util-obj-y += acl.o
 util-obj-y += error.o qemu-error.o
 util-obj-$(CONFIG_POSIX) += compatfd.o
diff --git a/util/fifo8.c b/util/fifo8.c
deleted file mode 100644
index 0ea5ad9..000
--- a/util/fifo8.c
+++ /dev/null
@@ -1,125 +0,0 @@
-/*
- * Generic FIFO component, implemented as a circular buffer.
- *
- * Copyright (c) 2012 Peter A. G. Crosthwaite
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License
- * as published by the Free Software Foundation; either version
- * 2 of the License, or (at your option) any later version.
- *
- * You should have received a copy of the GNU General Public License along
- * with this program; if not, see .
- */
-
-

[Qemu-devel] [PATCH 4/6] stubs: Add timer.c

[Fam Zheng]

Add timer functions that are used in util/throttle.c.

Signed-off-by: Fam Zheng 
---
 stubs/Makefile.objs |  1 +
 stubs/timer.c   | 44 
 2 files changed, 45 insertions(+)
 create mode 100644 stubs/timer.c

diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index f5d2cdd..1d433f3 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -42,3 +42,4 @@ stub-obj-y += kvm.o
 stub-obj-y += qmp_pc_dimm_device_list.o
 stub-obj-y += iohandler.o
 stub-obj-y += openpty.o
+stub-obj-y += timer.o
diff --git a/stubs/timer.c b/stubs/timer.c
new file mode 100644
index 000..17e25c9
--- /dev/null
+++ b/stubs/timer.c
@@ -0,0 +1,44 @@
+/*
+ * timer stub functions
+ *
+ * Copyright Red Hat, Inc., 2014
+ *
+ * Author: Fam Zheng 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * later.  See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/timer.h"
+
+void timer_del(QEMUTimer *ts)
+{
+abort();
+}
+
+void timer_init(QEMUTimer *ts,
+QEMUTimerList *timer_list, int scale,
+QEMUTimerCB *cb, void *opaque)
+{
+abort();
+}
+
+void timer_free(QEMUTimer *ts)
+{
+abort();
+}
+
+int64_t qemu_clock_get_ns(QEMUClockType type)
+{
+abort();
+}
+
+void timer_mod(QEMUTimer *ts, int64_t expire_time)
+{
+abort();
+}
+
+bool timer_pending(QEMUTimer *ts)
+{
+abort();
+}
-- 
2.0.3

[Qemu-devel] [PATCH 3/6] stubs: Add openpty.c

[Fam Zheng]

Add function "openpty" which is used by util/qemu-openpty.c.

Signed-off-by: Fam Zheng 
---
 stubs/Makefile.objs |  1 +
 stubs/openpty.c | 21 +
 2 files changed, 22 insertions(+)
 create mode 100644 stubs/openpty.c

diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index d9cad1b..f5d2cdd 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -41,3 +41,4 @@ stub-obj-y += cpus.o
 stub-obj-y += kvm.o
 stub-obj-y += qmp_pc_dimm_device_list.o
 stub-obj-y += iohandler.o
+stub-obj-y += openpty.o
diff --git a/stubs/openpty.c b/stubs/openpty.c
new file mode 100644
index 000..31e6ff5
--- /dev/null
+++ b/stubs/openpty.c
@@ -0,0 +1,21 @@
+/*
+ * openpty stub
+ *
+ * Copyright Red Hat, Inc., 2014
+ *
+ * Author: Fam Zheng 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * later.  See the COPYING file in the top-level directory.
+ */
+
+#include 
+#include 
+
+int openpty(int *amaster, int *aslave, char *name,
+const struct termios *termp,
+const struct winsize *winp)
+{
+abort();
+}
+
-- 
2.0.3

Re: [Qemu-devel] qemu 9p mapped-xattr model does not translate d_type in readdir

[Aneesh Kumar K.V]

Michael Tokarev  writes:

> There's another bugreport filed against debian qemu package,
> http://bugs.debian.org/755738 .  It talks about security_model=mapped-xattr
> case, where file type is stored elsewhere, and all files are turned into
> regular files.  http://bugs.debian.org/755738#10 is the proposed patch.
>

Ok the patch looks good, we could also try to get the d_type correct by
looking at the xattr mode bits, but i guess that will slow down readdir

Can you send a proper patch with signed-off-by to the list. I will pick
it from there.

-aneesh

[Qemu-devel] [PATCH 2/6] stubs: Add iohandler.c

[Fam Zheng]

Add stub function "qemu_set_fd_handler" which is used by
util/event_notifier-posix.c.

Signed-off-by: Fam Zheng 
---
 stubs/Makefile.objs |  1 +
 stubs/iohandler.c   | 20 
 2 files changed, 21 insertions(+)
 create mode 100644 stubs/iohandler.c

diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index 5e347d0..d9cad1b 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -40,3 +40,4 @@ stub-obj-$(CONFIG_WIN32) += fd-register.o
 stub-obj-y += cpus.o
 stub-obj-y += kvm.o
 stub-obj-y += qmp_pc_dimm_device_list.o
+stub-obj-y += iohandler.o
diff --git a/stubs/iohandler.c b/stubs/iohandler.c
new file mode 100644
index 000..97c0ce5
--- /dev/null
+++ b/stubs/iohandler.c
@@ -0,0 +1,20 @@
+/*
+ * iohandler stub functions
+ *
+ * Copyright Red Hat, Inc., 2014
+ *
+ * Author: Fam Zheng 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * later.  See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/main-loop.h"
+
+int qemu_set_fd_handler(int fd,
+IOHandler *fd_read,
+IOHandler *fd_write,
+void *opaque)
+{
+abort();
+}
-- 
2.0.3

Re: [Qemu-devel] [PATCH 4/4] qcow2: Check L1/L2/reftable entries for alignment

[Kevin Wolf]

Am 16.08.2014 um 23:16 hat Max Reitz geschrieben:
> Offsets taken from the L1, L2 and refcount tables are generally assumed
> to be correctly aligned. However, this cannot be guaranteed if the image
> has been written to by something different than qemu, thus check all
> offsets taken from these tables for correct cluster alignment.
> 
> Signed-off-by: Max Reitz 
> ---
>  block/qcow2-cluster.c  | 27 ++-
>  block/qcow2-refcount.c | 36 ++--
>  2 files changed, 60 insertions(+), 3 deletions(-)

Can you extend qemu-iotests 060 to check each of these cases?

> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index 5b36018..2cc41b2 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -486,6 +486,12 @@ int qcow2_get_cluster_offset(BlockDriverState *bs, 
> uint64_t offset,
>  goto out;
>  }
>  
> +if (offset_into_cluster(s, l2_offset)) {
> +qcow2_signal_corruption(bs, -1, -1, "L2 table offset %#" PRIx64
> +" unaligned", l2_offset);

Should we include l1_index in the message? If you want to debug the
image with a hex editor or something, this is important information.

> +return -EIO;
> +}
> +
>  /* load the l2 table in memory */
>  
>  ret = l2_load(bs, l2_offset, &l2_table);
> @@ -525,6 +531,12 @@ int qcow2_get_cluster_offset(BlockDriverState *bs, 
> uint64_t offset,
>  c = count_contiguous_clusters(nb_clusters, s->cluster_size,
>  &l2_table[l2_index], QCOW_OFLAG_ZERO);
>  *cluster_offset &= L2E_OFFSET_MASK;
> +if (offset_into_cluster(s, *cluster_offset)) {
> +qcow2_signal_corruption(bs, -1, -1, "Data cluster offset %#" 
> PRIx64
> +" unaligned", *cluster_offset);

The same thing here would be offset.

> +qcow2_cache_put(bs, s->l2_table_cache, (void **)&l2_table);
> +return -EIO;
> +}

I wonder whether a goto fail would start to make sense now, zero
clusters in v2 images have the same qcow2_cache_put/return -EIO code.

And actually, that's a corruption case as well, so we might call
qcow2_signal_corruption() there.

>  break;
>  default:
>  abort();
> @@ -576,6 +588,11 @@ static int get_cluster_table(BlockDriverState *bs, 
> uint64_t offset,
>  
>  assert(l1_index < s->l1_size);
>  l2_offset = s->l1_table[l1_index] & L1E_OFFSET_MASK;
> +if (offset_into_cluster(s, l2_offset)) {
> +qcow2_signal_corruption(bs, -1, -1, "L2 table offset %#" PRIx64
> +" unaligned", l2_offset);

l1_index again.

> +return -EIO;
> +}
>  
>  /* seek the l2 table of the given l2 offset */
>  
> @@ -948,6 +965,14 @@ static int handle_copied(BlockDriverState *bs, uint64_t 
> guest_offset,
>  bool offset_matches =
>  (cluster_offset & L2E_OFFSET_MASK) == *host_offset;
>  
> +if (offset_into_cluster(s, cluster_offset & L2E_OFFSET_MASK)) {
> +qcow2_signal_corruption(bs, -1, -1, "Data cluster offset %#llx "
> +"unaligned",
> +cluster_offset & L2E_OFFSET_MASK);

Worth adding guest_offset.

> +ret = -EIO;
> +goto out;
> +}
> +
>  if (*host_offset != 0 && !offset_matches) {
>  *bytes = 0;
>  ret = 0;
> @@ -979,7 +1004,7 @@ out:
>  
>  /* Only return a host offset if we actually made progress. Otherwise we
>   * would make requirements for handle_alloc() that it can't fulfill */
> -if (ret) {
> +if (ret > 0) {
>  *host_offset = (cluster_offset & L2E_OFFSET_MASK)
>   + offset_into_cluster(s, guest_offset);
>  }
> diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
> index 0ac1339..fac2963 100644
> --- a/block/qcow2-refcount.c
> +++ b/block/qcow2-refcount.c
> @@ -108,6 +108,12 @@ static int get_refcount(BlockDriverState *bs, int64_t 
> cluster_index)
>  if (!refcount_block_offset)
>  return 0;
>  
> +if (offset_into_cluster(s, refcount_block_offset)) {
> +qcow2_signal_corruption(bs, -1, -1, "Refblock offset %#" PRIx64
> +" unaligned", refcount_block_offset);

Add refcount_table_index.

> +return -EIO;
> +}
> +
>  ret = qcow2_cache_get(bs, s->refcount_block_cache, refcount_block_offset,
>  (void**) &refcount_block);
>  if (ret < 0) {
> @@ -181,6 +187,12 @@ static int alloc_refcount_block(BlockDriverState *bs,
>  
>  /* If it's already there, we're done */
>  if (refcount_block_offset) {
> +if (offset_into_cluster(s, refcount_block_offset)) {
> +qcow2_signal_corruption(bs, -1, -1, "Refblock offset %#" 
> PRIx64
> +" unaligned", refcount_block_offset);
> +return -EIO;
> +  

Re: [Qemu-devel] [PATCH v7 3/4] s390x: Migrate to new NMI interface

[Alexander Graf]

On 20.08.14 12:20, Alexey Kardashevskiy wrote:
> On 07/17/2014 04:30 PM, Alexey Kardashevskiy wrote:
>> On 07/15/2014 06:29 AM, Paolo Bonzini wrote:
>>> Il 14/07/2014 05:17, Alexey Kardashevskiy ha scritto:
>> No, you should politely bother them again.

 That does not seem helping though :-/
>>>
>>> Sorry. :)
>>>
>>> Well, patch 1 is the same as v6 and for the others Cornelia and Alex can
>>> ack it.  It looks good to me though.
>>
>>
>> Let me clarify things here.
>>
>> Alex rb'ed to 0/4, Cornelia rb'ed 2/4 and 3/4, so I put Alex's rb to
>> 1&2&3&4&5, Cornelia's rb to 2&3, rebase it on top of the current upstream,
>> repost after 2.1 is released and resume annoying people again. Is that 
>> correct?
>>
>> This is my current set:
>> 5/5 pc_piix: Migrate to new NMI interface
>> 4/5 spapr: Add support for new NMI interface
>> 3/5 s390x: Migrate to new NMI interface
>> 2/5 s390x: Convert QEMUMachine to MachineClass
>> 1/5 cpus: Define callback for QEMU "nmi" command
> 
> 
> Alex, ping.
>


Uh, what exactly do you expect from me? :)


Alex

Re: [Qemu-devel] [PATCH] virt/kvm/assigned-dev.c: Set 'dev->irq_source_id' to '-1' after free it

[Chen Gang]

On 08/20/2014 08:01 AM, Chen Gang wrote:
> 
> By the way, at present, I use Qemu as user mode program, is there common
> test with both Qemu and KVM/Xen? And is a PC enough for the common test?
>

Oh, I find Qemu have "make check" just like gcc/binutils, so for each of
my patch, next, shall run "./configure && make && make check" at least.

And also welcome any additional ideas, suggestions or completions about
test for kvm/xen/qemu.


Thanks.
 
> 
> On 08/20/2014 07:58 AM, Chen Gang wrote:
>> On 08/19/2014 11:49 PM, Paolo Bonzini wrote:
>>> Il 19/08/2014 17:44, Chen Gang ha scritto:
> Hello maintainers:
>
> Please help check this patch, when you have time.
>>> Hi, it's already on its way to 3.17-rc2, but I first have to run a bunch
>>> of tests.
>>
>> OK, thanks. Also can let me try the test, although I am not quite
>> familiar with KVM. Since I plan to focus on KVM/Xen next, I shall
>> construct related environments for its' common test, at least.
>>
>> I am just constructing the gcc common test environments under a new PC,
>> is a PC also enough for KVM/Xen common test?
>>
>> Welcome any ideas, suggestions or completions about it (especially the
>> information about KVM/Xen common test).
>>
>>
>> Thanks.
>>
> 
> 


-- 
Chen Gang

Open share and attitude like air water and life which God blessed

Re: [Qemu-devel] Recent SeaBIOS too big for QEMU -initrd

[Michael S. Tsirkin]

On Wed, Aug 20, 2014 at 11:27:41AM +0100, Stefan Hajnoczi wrote:
> The QEMU -initrd option loads the initrd at the top of RAM.  There is
> a 64 KB safety region for ACPI tables in hw/i386/pc.c:load_linux():
> 
>   initrd_max = max_ram_size-ACPI_DATA_SIZE-1;
> 
> QEMU's bios-256k.bin SeaBIOS build reserves 128 KB at the top of
> memory so the 64 KB ACPI data size has become too small.
> 
> The guest Linux kernel rejects the initrd:
> BIOS-provided physical RAM map:
>  BIOS-e820: 0001 - 0009fc00 (usable)
>  BIOS-e820: 0009fc00 - 000a (reserved)
>  BIOS-e820: 000f - 0010 (reserved)
>  BIOS-e820: 0010 - 3ffe (usable)
>  BIOS-e820: 3ffe - 4000 (reserved)
>  BIOS-e820: feffc000 - ff00 (reserved)
>  BIOS-e820: fffc - 0001 (reserved)
> ...
> initrd extends beyond end of memory (0x3ffef79f > 0x3ffe)
> disabling initrd
> 
> It is easy enough to "fix" the problem by bumping ACPI_DATA_SIZE up to
> 0x2 in QEMU.  Perhaps this should only be done for bios-256k.bin
> guests and not bios-128k.bin guests (QEMU 1.7 and older machine
> types).
> 
> Perhaps QEMU -> SeaBIOS -> linuxboot.bin can be simplified so QEMU
> doesn't have to guess what e820 region SeaBIOS will reserve.
> linuxboot.bin would probably be the place to do it unless SeaBIOS has
> Linux loading functionality that could be reused.
> 
> Any ideas?
> 
> Stefan

I would say 2.1 and up, -M pc-i440fx-2.0 works fine, or am I mistaken?

Re: [Qemu-devel] vhost-net issue with multiples interfaces using MQ

[William Dauchy]

On Aug19 15:02, William Dauchy wrote:
> I'm using readconfig option to read the three vif interfaces.
> thanks for testing I will re-triple test to see what's wrong

so indeed sorry for the noise, I was wrong in my tests.
-- 
William


signature.asc
Description: Digital signature

Re: [Qemu-devel] [PATCH 2/3] ioh3420: Provide a unique bus name and an interrupt mapping function

[Michael S. Tsirkin]

On Wed, Aug 20, 2014 at 11:30:55AM +0200, Knut Omang wrote:
> On Wed, 2014-08-20 at 10:52 +0200, Paolo Bonzini wrote:
> > Il 20/08/2014 08:53, Knut Omang ha scritto:
> > > A unique bus name is necessary to be able to refer to each instance
> > > from the command line and monitors.
> > 
> > Is it needed?  Can't you just add id= to the -device option?
> 
> Yes, as far as I understand the problem is that the id= would work on
> the ioh3420 device itself, while what is needed here is to name the
> secondary bus of the ioh3420, which I haven't found a way to name from
> the command line.

Did you try using the device name?

For pci bridges, unless you set bus_name, bus name will
match device itself. See this code:

 * If we don't specify the name, the bus will be addressed as
 * .0, where id is the device id.
 * Since PCI Bridge devices have a single bus each, we don't need
 * the index:
 * let users address the bus using the device name.
 */
if (!br->bus_name && dev->qdev.id && *dev->qdev.id) {
br->bus_name = dev->qdev.id;
}



> Maybe an even better solution would be to have default names for
> everything, if not specified, from a user friendliness perspective? 
>
> I suppose this is a more general issue of sensible default values
> though, but the fact that it is easy to create devices which cannot be
> referred has caused me some confusion from time to time.
> 
> > Paolo
> 
> Thanks,
> 
> Knut

Re: [Qemu-devel] [PATCH v7 3/4] s390x: Migrate to new NMI interface

[Alexey Kardashevskiy]

On 08/20/2014 09:20 PM, Alexander Graf wrote:
> 
> 
> On 20.08.14 12:20, Alexey Kardashevskiy wrote:
>> On 07/17/2014 04:30 PM, Alexey Kardashevskiy wrote:
>>> On 07/15/2014 06:29 AM, Paolo Bonzini wrote:
 Il 14/07/2014 05:17, Alexey Kardashevskiy ha scritto:
>>> No, you should politely bother them again.
>
> That does not seem helping though :-/

 Sorry. :)

 Well, patch 1 is the same as v6 and for the others Cornelia and Alex can
 ack it.  It looks good to me though.
>>>
>>>
>>> Let me clarify things here.
>>>
>>> Alex rb'ed to 0/4, Cornelia rb'ed 2/4 and 3/4, so I put Alex's rb to
>>> 1&2&3&4&5, Cornelia's rb to 2&3, rebase it on top of the current upstream,
>>> repost after 2.1 is released and resume annoying people again. Is that 
>>> correct?
>>>
>>> This is my current set:
>>> 5/5 pc_piix: Migrate to new NMI interface
>>> 4/5 spapr: Add support for new NMI interface
>>> 3/5 s390x: Migrate to new NMI interface
>>> 2/5 s390x: Convert QEMUMachine to MachineClass
>>> 1/5 cpus: Define callback for QEMU "nmi" command
>>
>>
>> Alex, ping.
>>
> 
> 
> Uh, what exactly do you expect from me? :)

No idea :) You could take patches to your tree. I do not understand what to
do with this set now. Please help. Thanks.



-- 
Alexey

Re: [Qemu-devel] [PATCH 1/4] block: Correct bs->growable

[Kevin Wolf]

Am 12.07.2014 um 00:23 hat Max Reitz geschrieben:
> Currently, the field "growable" in a BDS is set iff the BDS is opened in
> protocol mode (with O_BDRV_PROTOCOL). However, not every protocol block
> driver allows growing: NBD, for instance, does not. On the other hand,
> a non-protocol block driver may allow growing: The raw driver does.
> 
> Fix this by correcting the "growable" field in the driver-specific open
> function for the BDS, if necessary.
> 
> Signed-off-by: Max Reitz 

I'm not sure I agree with bs->growable = true for raw. It's certainly
true that the backend can technically provide the functionality that
writes beyond EOF grow the file. That's not the point of bs->growable,
though.

The point of it was to _forbid_ it to grow even when it's technically
possible (non-file protocols weren't really a thing back then, apart
from vvfat, so the assumption was that it's always technically
possible). growable was introduced with bdrv_check_request(), which is
supposed to reject guest requests after the end of the virtual disk (and
this fixed a CVE, see commit 71d0770c). You're now disabling this check
for raw.

I think we need to make sure that bs->growable is only set if it is
opened for an image that has drv->requires_growing_file set and
therefore not directly used by a guest.

Well, except that with node-name a guest will be able to use any image
in the chain... Might this mean that it's really a BlockBackend
property?

Kevin

Re: [Qemu-devel] [PATCH v10 00/14] qemu-img: Implement commit like QMP

[Kevin Wolf]

Am 15.08.2014 um 17:22 hat Max Reitz geschrieben:
> On 22.07.2014 23:29, Max Reitz wrote:
> >qemu-img should use QMP commands whenever possible in order to ensure
> >feature completeness of both online and offline image operations. For
> >the "commit" command, this is relatively easy, so implement it first
> >(in the hope that indeed others will follow).
> >
> >As qemu-img does not have access to QMP (due to QMP being intertwined
> >with basically everything in qemu), we cannot directly use QMP, but at
> >least use the functions the corresponding QMP commands are using (which
> >would be "block-commit", in this case).
> 
> Ping

This doesn't apply cleanly any more.

Kevin

Re: [Qemu-devel] [PATCH 2/3] ioh3420: Provide a unique bus name and an interrupt mapping function

[Andreas Färber]

Hi Knut,

Am 20.08.2014 08:53, schrieb Knut Omang:
> 
> A unique bus name is necessary to be able to refer to each instance
> from the command line and monitors.
> 
> Signed-off-by: Knut Omang 
> ---
>  hw/pci-bridge/ioh3420.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/hw/pci-bridge/ioh3420.c b/hw/pci-bridge/ioh3420.c
> index 7cd87fc..8f6c8b0 100644
> --- a/hw/pci-bridge/ioh3420.c
> +++ b/hw/pci-bridge/ioh3420.c
> @@ -95,6 +95,9 @@ static int ioh3420_initfn(PCIDevice *d)
>  PCIEPort *p = PCIE_PORT(d);
>  PCIESlot *s = PCIE_SLOT(d);
>  int rc;
> +char tmp[100];
> +sprintf(tmp, "pcie_port.%d", s->slot);
> +pci_bridge_map_irq(br, g_strdup(tmp), pci_swizzle_map_irq_fn);

Style issues apart (white line being dropped), this is a rather
convoluted way of providing a bus name to something. ;)
That makes me think something else is going wrong, or APIs may need to
be improved. Could you elaborate for those not intimately familiar with
this bridge what name you're seeing before, given a particular command
line, and explain why IRQ mapping affects the bus name?

Thanks,
Andreas

>  
>  rc = pci_bridge_initfn(d, TYPE_PCIE_BUS);
>  if (rc < 0) {
> @@ -154,6 +157,7 @@ static void ioh3420_exitfn(PCIDevice *d)
>  pcie_cap_exit(d);
>  msi_uninit(d);
>  pci_bridge_exitfn(d);
> +g_free((char*)br->bus_name);
>  }
>  
>  PCIESlot *ioh3420_init(PCIBus *bus, int devfn, bool multifunction,

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH v7 3/4] s390x: Migrate to new NMI interface

[Alexander Graf]

On 20.08.14 13:38, Alexey Kardashevskiy wrote:
> On 08/20/2014 09:20 PM, Alexander Graf wrote:
>>
>>
>> On 20.08.14 12:20, Alexey Kardashevskiy wrote:
>>> On 07/17/2014 04:30 PM, Alexey Kardashevskiy wrote:
 On 07/15/2014 06:29 AM, Paolo Bonzini wrote:
> Il 14/07/2014 05:17, Alexey Kardashevskiy ha scritto:
 No, you should politely bother them again.
>>
>> That does not seem helping though :-/
>
> Sorry. :)
>
> Well, patch 1 is the same as v6 and for the others Cornelia and Alex can
> ack it.  It looks good to me though.


 Let me clarify things here.

 Alex rb'ed to 0/4, Cornelia rb'ed 2/4 and 3/4, so I put Alex's rb to
 1&2&3&4&5, Cornelia's rb to 2&3, rebase it on top of the current upstream,
 repost after 2.1 is released and resume annoying people again. Is that 
 correct?

 This is my current set:
 5/5 pc_piix: Migrate to new NMI interface
 4/5 spapr: Add support for new NMI interface
 3/5 s390x: Migrate to new NMI interface
 2/5 s390x: Convert QEMUMachine to MachineClass
 1/5 cpus: Define callback for QEMU "nmi" command
>>>
>>>
>>> Alex, ping.
>>>
>>
>>
>> Uh, what exactly do you expect from me? :)
> 
> No idea :) You could take patches to your tree. I do not understand what to
> do with this set now. Please help. Thanks.

The way I understood your last email you wanted to rebase and repost
them at which point someone could pick them up :)


Alex

Re: [Qemu-devel] [PATCH 2/3] ioh3420: Provide a unique bus name and an interrupt mapping function

[Markus Armbruster]

Knut Omang  writes:

> On Wed, 2014-08-20 at 10:52 +0200, Paolo Bonzini wrote:
>> Il 20/08/2014 08:53, Knut Omang ha scritto:
>> > A unique bus name is necessary to be able to refer to each instance
>> > from the command line and monitors.
>> 
>> Is it needed?  Can't you just add id= to the -device option?
>
> Yes, as far as I understand the problem is that the id= would work on
> the ioh3420 device itself, while what is needed here is to name the
> secondary bus of the ioh3420, which I haven't found a way to name from
> the command line.

Bus names in qdev are a mess.  Here are the rules:

1. If code provides a name, that's the name.

2. Else, if the device has an ID, the name is ID.N, where N counts the
device's buses from zero.

3. Else, the name is BUS-TYPE-NAME.N, where N counts the buses of this
type from zero.

This results in a usable bus name unless device IDs collide with bus
type names, or the code provides names that collide.

The user needs to take care to use IDs that don't collide with bus
names.  Adding new bus names may screw some users.

The user needs to take further care to use IDs whenever the code
provides a bus name that collides.  Adding code that provides bus names
may screw some users.

Broken by design.

The problem here is "code provides names that collide": device
q35-pcihost provides the name "pcie.0".  Bound to collide with the first
PCIE bus named under rule 3.  For instance, if you next add an ioh3420
without ID, its bus is also named "pcie.0".

Rule 1 should be taken out and shot.  Unfortunately, that'll break ABI
left & right.  Instead, we can try to reduce its use.  The appended does
exactly that for q35-pcihost.  With it applied, the bus provided by
q35-pcihost still gets the same name "pcie.0", but under rule 3 instead
of rule 1.  Rule 3 then names further PCIE buses "pcie.1", "pcie.2", ...
instead of "pcie.0", "pcie.1", ...  Better, but it's still an ABI break.

> Maybe an even better solution would be to have default names for
> everything, if not specified, from a user friendliness perspective? 

Buses *have* a default name!  You're confusing this with device IDs,
which exist only when the user sets one.

Changes in this area are difficult, because the names are all ABI.
Names that cannot be used are fair game, of course.

> I suppose this is a more general issue of sensible default values
> though, but the fact that it is easy to create devices which cannot be
> referred has caused me some confusion from time to time.

Picking default qdev IDs risks collisions with the user's IDs.  We
shouldn't do that.  We do it anyway in a few places, for historical
reasons.

QOM paths might be a sane way to let users refer to devices without IDs.


While writing the above, I stumbled another rule 1 screwup: pci_bridge.c
attempts to "improve" the boring standard bus names chosen via rule 2 or
3.

pci_bridge_initfn() provides a bus name of its own (commit 8a3d80f
pci_bridge: user-friendly default bus name):

a. If pci_bridge_map_irq() set a bus name, that's the name.

b. Else, if the device has an ID, that's the name.  Thus, ID.N is
"improved" to just ID, at the cost of a special case: now users have to
avoid not just IDs of the form BUS-TYPE-NAME.N, but also plain
BUS-TYPE-NAME.

Callers of pci_bridge_map_irq() generally provide a name.  Some names
contain spaces, thus can't collide (but would be bloody inconvenient on
the command line or in the monitor).  Others don't, but thankfully the
ones I checked are in dead code.  Craptastic.


diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index 37f228e..469aafd 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -47,7 +47,7 @@ static void q35_host_realize(DeviceState *dev, Error **errp)
 sysbus_add_io(sbd, MCH_HOST_BRIDGE_CONFIG_DATA, &pci->data_mem);
 sysbus_init_ioports(sbd, MCH_HOST_BRIDGE_CONFIG_DATA, 4);
 
-pci->bus = pci_bus_new(DEVICE(s), "pcie.0",
+pci->bus = pci_bus_new(DEVICE(s), NULL,
s->mch.pci_address_space, s->mch.address_space_io,
0, TYPE_PCIE_BUS);
 qdev_set_parent_bus(DEVICE(&s->mch), BUS(pci->bus));

Re: [Qemu-devel] [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only supply _EJ0 methods for PCIslots that support hotplug by runtime patching

[Fabio Fantoni]

Il 12/05/2014 16:32, Ross Philipson ha scritto:

On 05/12/2014 05:05 AM, Ian Campbell wrote:

On Fri, 2014-05-09 at 13:32 -0400, Ross Philipson wrote:

On 05/09/2014 12:34 PM, Paul Durrant wrote:

-Original Message-
From: Ian Campbell
Sent: 09 May 2014 17:12
To: Konrad Rzeszutek Wilk
Cc: Ross Philipson; ke...@koconnor.net; Huangweidong (C); Hanweidong
(Randy); m...@redhat.com; qemu-devel@nongnu.org; xen-
de...@lists.xen.org; fabio.fant...@m2r.biz;
johannes.kra...@googlemail.com; Gonglei (Arei); Stefano Stabellini;
Gaowei (UVP); Jan Beulich; Anthony Perard; Paul Durrant
Subject: Re: [Xen-devel] [PATCH v4] Hvmloader: Modify ACPI to only 
supply

_EJ0 methods for PCIslots that support hotplug by runtime patching


Ping...
Are there any news about this patch?

Thanks for any reply.



On Fri, 2014-05-09 at 12:00 -0400, Konrad Rzeszutek Wilk wrote:

So we could just then gat the _EJ0 functionality based on values 
that

are present (or not) in the SSDT ?


AIUI the very presence of _EJ0 is what marks the device as being
ejectable (e.g. in the Windows device manager).

It would be possible to make _EJ0 conditionally turn itself into a 
NOP

without resorting to an SSDT, but I don't think that solves the issue
they are trying to solve, which is that the user can even try to 
eject

an non-hotplug device. (grep for UAR1 in our dsdt.asl and
acpi_info->com1_present in hvmloader/acpi/build.c for an example 
of this

sort of conditional thing)



Going back to the SSDT idea. A little poking around and what not and I
came up with something like this that I build into an SSDT:

DefinitionBlock ("SSDTX.aml", "SSDT", 2, "Xen", "HVM", 0)
{
  /* S00 device is defined in DSDT, this allows me to
   * refrence it in this SSDT
   */
  External (\_SB.PCI0.S00, DeviceObj)

  ...

  /* Extend the functionality of S00 */
  Scope ( \_SB.PCI0.S00 ) {
  Method(_EJ0, 1, NotSerialized)
  {
  /* Do stuffs here */
  }
  }
}


Thanks, this looks like the sort of thing I was naively imagining would
be possible.


So I did find some examples of this after all in my pile of ACPI
firmware snapshots from all our supported platforms.


Thanks (none of the machines I looked at had PCI hotplug apparently). I
was curious to know how Real Firmware Engineers(tm) dealt with this sort
of issue.

I was worried how real life OSPMs might interpret this method being in
an SSDT instead of the DSDT. In theory it shouldn't matter, and the fact
that real firmware does this seem to suggest that at least Windows
treats it that way (which is a relief).


I did actually find SSDTs that were specifically adding an _EJ0 to a 
device scope for a device defined externally. I attached an example 
from a Fujitsu system I have. The PRT1 device on SAT0 is external:


External (\_SB_.PCI0.SAT0.PRT1, DeviceObj)

And _EJ0 is added to the scope.




  I think this would
work allowing you to just add or not add _EJ0 methods to the PCI 
devices

you want by either using different SSDTs or doing something to generate
or munge the SSDT at runtime (which would be simpler than messing with
the DSDT I think.


Without filling out the body of _EJ0 (which I tried but failed to do)
your stub compiles to 60 bytes of AML, I suppose that even having filled
in _EJ0 in the result would be less than, say, 128 bytes.

Given that there are 32 PCI slots we would be talking about a total of
4k of space in hvmloader to provide a precompiled SSDT for each slot,
which can be inserted at runtime depending on each slots configuration.

I wouldn't be especially surprised if the code to generate a suitable
SSDT dynamically was a reasonable proportion of that size, so unless
there is the possibility of needing other variants it seems like just
generating each of them would be the say to go.


  I did not try it (actually I did but ran into other
problems on our platform :).


;-)

Ian.

Re: [Qemu-devel] [PATCH v7 3/4] s390x: Migrate to new NMI interface

[Alexander Graf]

On 20.08.14 14:11, Alexey Kardashevskiy wrote:
> On 08/20/2014 10:05 PM, Alexander Graf wrote:
>>
>>
>> On 20.08.14 13:38, Alexey Kardashevskiy wrote:
>>> On 08/20/2014 09:20 PM, Alexander Graf wrote:


 On 20.08.14 12:20, Alexey Kardashevskiy wrote:
> On 07/17/2014 04:30 PM, Alexey Kardashevskiy wrote:
>> On 07/15/2014 06:29 AM, Paolo Bonzini wrote:
>>> Il 14/07/2014 05:17, Alexey Kardashevskiy ha scritto:
>> No, you should politely bother them again.

 That does not seem helping though :-/
>>>
>>> Sorry. :)
>>>
>>> Well, patch 1 is the same as v6 and for the others Cornelia and Alex can
>>> ack it.  It looks good to me though.
>>
>>
>> Let me clarify things here.
>>
>> Alex rb'ed to 0/4, Cornelia rb'ed 2/4 and 3/4, so I put Alex's rb to
>> 1&2&3&4&5, Cornelia's rb to 2&3, rebase it on top of the current 
>> upstream,
>> repost after 2.1 is released and resume annoying people again. Is that 
>> correct?
>>
>> This is my current set:
>> 5/5 pc_piix: Migrate to new NMI interface
>> 4/5 spapr: Add support for new NMI interface
>> 3/5 s390x: Migrate to new NMI interface
>> 2/5 s390x: Convert QEMUMachine to MachineClass
>> 1/5 cpus: Define callback for QEMU "nmi" command
>
>
> Alex, ping.
>


 Uh, what exactly do you expect from me? :)
>>>
>>> No idea :) You could take patches to your tree. I do not understand what to
>>> do with this set now. Please help. Thanks.
>>
>> The way I understood your last email you wanted to rebase and repost
>> them at which point someone could pick them up :)
> 
> I wanted answer to "is that correct?" :)
> And I tried replaying the patchset on top of your ppc-next, it applied. But
> I'll repost to make sure that we have the same thing.

Oh, cool. I'll apply them as soon as I finished to recover my HD then :)


Alex

[Qemu-devel] [PATCH v8 2/5] s390x: Convert QEMUMachine to MachineClass

[Alexey Kardashevskiy]

This converts s390-virtio and s390-ccw-virtio machines to QOM MachineClass.
This brings ability to add interfaces to the machine classes. The first
interface for addition will be NMI.

The patch is mechanical so no change in behavior is expected.

Signed-off-by: Alexey Kardashevskiy 
Reviewed-by: Cornelia Huck 
Reviewed-by: Alexander Graf 
---
 hw/s390x/s390-virtio-ccw.c | 43 +++
 hw/s390x/s390-virtio.c | 44 +++-
 2 files changed, 54 insertions(+), 33 deletions(-)

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 42f5cec..05311b8 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -18,6 +18,8 @@
 #include "css.h"
 #include "virtio-ccw.h"
 
+#define TYPE_S390_CCW_MACHINE   "s390-ccw-machine"
+
 void io_subsystem_reset(void)
 {
 DeviceState *css, *sclp, *flic;
@@ -134,24 +136,33 @@ static void ccw_init(MachineState *machine)
 s390_create_virtio_net(BUS(css_bus), "virtio-net-ccw");
 }
 
-static QEMUMachine ccw_machine = {
-.name = "s390-ccw-virtio",
-.alias = "s390-ccw",
-.desc = "VirtIO-ccw based S390 machine",
-.init = ccw_init,
-.block_default_type = IF_VIRTIO,
-.no_cdrom = 1,
-.no_floppy = 1,
-.no_serial = 1,
-.no_parallel = 1,
-.no_sdcard = 1,
-.use_sclp = 1,
-.max_cpus = 255,
+static void ccw_machine_class_init(ObjectClass *oc, void *data)
+{
+MachineClass *mc = MACHINE_CLASS(oc);
+
+mc->name = "s390-ccw-virtio";
+mc->alias = "s390-ccw";
+mc->desc = "VirtIO-ccw based S390 machine";
+mc->init = ccw_init;
+mc->block_default_type = IF_VIRTIO;
+mc->no_cdrom = 1;
+mc->no_floppy = 1;
+mc->no_serial = 1;
+mc->no_parallel = 1;
+mc->no_sdcard = 1;
+mc->use_sclp = 1,
+mc->max_cpus = 255;
+}
+
+static const TypeInfo ccw_machine_info = {
+.name  = TYPE_S390_CCW_MACHINE,
+.parent= TYPE_MACHINE,
+.class_init= ccw_machine_class_init,
 };
 
-static void ccw_machine_init(void)
+static void ccw_machine_register_types(void)
 {
-qemu_register_machine(&ccw_machine);
+type_register_static(&ccw_machine_info);
 }
 
-machine_init(ccw_machine_init)
+type_init(ccw_machine_register_types)
diff --git a/hw/s390x/s390-virtio.c b/hw/s390x/s390-virtio.c
index 93c7ace..f1e0dbc 100644
--- a/hw/s390x/s390-virtio.c
+++ b/hw/s390x/s390-virtio.c
@@ -51,6 +51,7 @@
 
 #define MAX_BLK_DEVS10
 #define ZIPL_FILENAME   "s390-zipl.rom"
+#define TYPE_S390_MACHINE   "s390-machine"
 
 static VirtIOS390Bus *s390_bus;
 static S390CPU **ipi_states;
@@ -279,25 +280,34 @@ static void s390_init(MachineState *machine)
 s390_create_virtio_net((BusState *)s390_bus, "virtio-net-s390");
 }
 
-static QEMUMachine s390_machine = {
-.name = "s390-virtio",
-.alias = "s390",
-.desc = "VirtIO based S390 machine",
-.init = s390_init,
-.block_default_type = IF_VIRTIO,
-.no_cdrom = 1,
-.no_floppy = 1,
-.no_serial = 1,
-.no_parallel = 1,
-.no_sdcard = 1,
-.use_virtcon = 1,
-.max_cpus = 255,
-.is_default = 1,
+static void s390_machine_class_init(ObjectClass *oc, void *data)
+{
+MachineClass *mc = MACHINE_CLASS(oc);
+
+mc->name = "s390-virtio";
+mc->alias = "s390";
+mc->desc = "VirtIO based S390 machine";
+mc->init = s390_init;
+mc->block_default_type = IF_VIRTIO;
+mc->max_cpus = 255;
+mc->no_serial = 1;
+mc->no_parallel = 1;
+mc->use_virtcon = 1;
+mc->no_floppy = 1;
+mc->no_cdrom = 1;
+mc->no_sdcard = 1;
+mc->is_default = 1;
+}
+
+static const TypeInfo s390_machine_info = {
+.name  = TYPE_S390_MACHINE,
+.parent= TYPE_MACHINE,
+.class_init= s390_machine_class_init,
 };
 
-static void s390_machine_init(void)
+static void s390_machine_register_types(void)
 {
-qemu_register_machine(&s390_machine);
+type_register_static(&s390_machine_info);
 }
 
-machine_init(s390_machine_init);
+type_init(s390_machine_register_types)
-- 
2.0.0

[Qemu-devel] [PATCH v8 5/5] pc_piix: Migrate to new NMI interface

[Alexey Kardashevskiy]

This implements an NMI interface for i386 PC machines.

This removes #ifdef I386 branch in qmp_inject_nmi so new i386's nmi()
callback is going to be used for NMI.

This changes code to inject NMI on the current CPU instead of injecting
it on every CPU. However that does not seem to be an issue.

Since kvm_apic_external_nmi() takes care of preforming operations in
the specific CPU thread so no extra measure is required here.

Signed-off-by: Alexey Kardashevskiy 
Reviewed-by: Alexander Graf 
---
Changes:
v6:
* make use of NMI interface

v5:
* make use of NMI interface

v4:
* s/\/nmi_monitor_handler/

v3:
* now contains both old code removal and new code insertion, easier to
track changes
* fixed compile for linux-user
---
 cpus.c| 14 --
 hw/i386/pc_piix.c | 42 ++
 2 files changed, 42 insertions(+), 14 deletions(-)

diff --git a/cpus.c b/cpus.c
index eb1ac85..f57fe5f 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1525,21 +1525,7 @@ exit:
 
 void qmp_inject_nmi(Error **errp)
 {
-#if defined(TARGET_I386)
-CPUState *cs;
-
-CPU_FOREACH(cs) {
-X86CPU *cpu = X86_CPU(cs);
-
-if (!cpu->apic_state) {
-cpu_interrupt(cs, CPU_INTERRUPT_NMI);
-} else {
-apic_deliver_nmi(cpu->apic_state);
-}
-}
-#else
 nmi_monitor_handle(monitor_get_cpu_index(), errp);
-#endif
 }
 
 void dump_drift_info(FILE *f, fprintf_function cpu_fprintf)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 4f22be8..125e609 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -49,11 +49,13 @@
 #include "hw/acpi/acpi.h"
 #include "cpu.h"
 #include "qemu/error-report.h"
+#include "hw/nmi.h"
 #ifdef CONFIG_XEN
 #  include 
 #endif
 
 #define MAX_IDE_BUS 2
+#define TYPE_NMI_X86"x86-nmi"
 
 static const int ide_iobase[MAX_IDE_BUS] = { 0x1f0, 0x170 };
 static const int ide_iobase2[MAX_IDE_BUS] = { 0x3f6, 0x376 };
@@ -297,6 +299,9 @@ static void pc_init1(MachineState *machine,
 if (pci_enabled) {
 pc_pci_device_init(pci_bus);
 }
+
+object_property_add_child(OBJECT(machine), "nmi",
+  object_new(TYPE_NMI_X86), NULL);
 }
 
 static void pc_init_pci(MachineState *machine)
@@ -926,3 +931,40 @@ static void pc_machine_init(void)
 }
 
 machine_init(pc_machine_init);
+
+static void x86_nmi(NMIState *n, int cpu_index, Error **errp)
+{
+CPUState *cs = qemu_get_cpu(cpu_index);
+X86CPU *cpu = X86_CPU(cs);
+
+if (!cpu->apic_state) {
+cpu_interrupt(cs, CPU_INTERRUPT_NMI);
+#ifndef CONFIG_USER_ONLY
+} else {
+apic_deliver_nmi(cpu->apic_state);
+#endif
+}
+}
+
+static void x86_nmi_class_init(ObjectClass *oc, void *data)
+{
+NMIClass *nc = NMI_CLASS(oc);
+nc->nmi_monitor_handler = x86_nmi;
+}
+
+static const TypeInfo x86_nmi_info = {
+.name  = TYPE_NMI_X86,
+.parent= TYPE_OBJECT,
+.class_init= x86_nmi_class_init,
+.interfaces = (InterfaceInfo[]) {
+{ TYPE_NMI },
+{ }
+},
+};
+
+static void x86_nmi_register_types(void)
+{
+type_register_static(&x86_nmi_info);
+}
+
+type_init(x86_nmi_register_types)
-- 
2.0.0

[Qemu-devel] [PATCH v8 3/5] s390x: Migrate to new NMI interface

[Alexey Kardashevskiy]

This implements an NMI interface for s390 and s390-ccw machines.

This removes #ifdef s390 branch in qmp_inject_nmi so new s390's
nmi_monitor_handler() callback is going to be used for NMI.

Since nmi_monitor_handler()-calling code is platform independent,
CPUState::cpu_index is used instead of S390CPU::env.cpu_num.
There should not be any change in behaviour as both @cpu_index and
@cpu_num are global CPU numbers.

Note that s390_cpu_restart() already takes care of the specified cpu,
so we don't need to schedule via async_run_on_cpu().

Since the only error s390_cpu_restart() can return is ENOSYS, convert
it to QERR_UNSUPPORTED.

Signed-off-by: Alexey Kardashevskiy 
Reviewed-by: Alexander Graf 
Reviewed-by: Cornelia Huck 
---
Changes:
v8:
* adjusted commit log

v7:
* since now s390' are QOM'ed, add interface to them and do not create
new object

v6:
* supported NMI interface

v5:
* added ENOSYS -> QERR_UNSUPPORTED, qapi/qmp/qerror.h was added for this

v4:
* s/\/nmi_monitor_handler/

v3:
* now contains both old code removal and new code insertion, easier to
track changes
---
 cpus.c | 14 --
 hw/s390x/s390-virtio-ccw.c |  6 ++
 hw/s390x/s390-virtio.c | 15 +++
 hw/s390x/s390-virtio.h |  3 +++
 4 files changed, 24 insertions(+), 14 deletions(-)

diff --git a/cpus.c b/cpus.c
index 37d90f4..eb1ac85 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1537,20 +1537,6 @@ void qmp_inject_nmi(Error **errp)
 apic_deliver_nmi(cpu->apic_state);
 }
 }
-#elif defined(TARGET_S390X)
-CPUState *cs;
-S390CPU *cpu;
-
-CPU_FOREACH(cs) {
-cpu = S390_CPU(cs);
-if (cpu->env.cpu_num == monitor_get_cpu_index()) {
-if (s390_cpu_restart(S390_CPU(cs)) == -1) {
-error_set(errp, QERR_UNSUPPORTED);
-return;
-}
-break;
-}
-}
 #else
 nmi_monitor_handle(monitor_get_cpu_index(), errp);
 #endif
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 05311b8..004b2c2 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -139,6 +139,7 @@ static void ccw_init(MachineState *machine)
 static void ccw_machine_class_init(ObjectClass *oc, void *data)
 {
 MachineClass *mc = MACHINE_CLASS(oc);
+NMIClass *nc = NMI_CLASS(oc);
 
 mc->name = "s390-ccw-virtio";
 mc->alias = "s390-ccw";
@@ -152,12 +153,17 @@ static void ccw_machine_class_init(ObjectClass *oc, void 
*data)
 mc->no_sdcard = 1;
 mc->use_sclp = 1,
 mc->max_cpus = 255;
+nc->nmi_monitor_handler = s390_nmi;
 }
 
 static const TypeInfo ccw_machine_info = {
 .name  = TYPE_S390_CCW_MACHINE,
 .parent= TYPE_MACHINE,
 .class_init= ccw_machine_class_init,
+.interfaces = (InterfaceInfo[]) {
+{ TYPE_NMI },
+{ }
+},
 };
 
 static void ccw_machine_register_types(void)
diff --git a/hw/s390x/s390-virtio.c b/hw/s390x/s390-virtio.c
index f1e0dbc..1a75a1c 100644
--- a/hw/s390x/s390-virtio.c
+++ b/hw/s390x/s390-virtio.c
@@ -280,9 +280,19 @@ static void s390_init(MachineState *machine)
 s390_create_virtio_net((BusState *)s390_bus, "virtio-net-s390");
 }
 
+void s390_nmi(NMIState *n, int cpu_index, Error **errp)
+{
+CPUState *cs = qemu_get_cpu(cpu_index);
+
+if (s390_cpu_restart(S390_CPU(cs))) {
+error_set(errp, QERR_UNSUPPORTED);
+}
+}
+
 static void s390_machine_class_init(ObjectClass *oc, void *data)
 {
 MachineClass *mc = MACHINE_CLASS(oc);
+NMIClass *nc = NMI_CLASS(oc);
 
 mc->name = "s390-virtio";
 mc->alias = "s390";
@@ -297,12 +307,17 @@ static void s390_machine_class_init(ObjectClass *oc, void 
*data)
 mc->no_cdrom = 1;
 mc->no_sdcard = 1;
 mc->is_default = 1;
+nc->nmi_monitor_handler = s390_nmi;
 }
 
 static const TypeInfo s390_machine_info = {
 .name  = TYPE_S390_MACHINE,
 .parent= TYPE_MACHINE,
 .class_init= s390_machine_class_init,
+.interfaces = (InterfaceInfo[]) {
+{ TYPE_NMI },
+{ }
+},
 };
 
 static void s390_machine_register_types(void)
diff --git a/hw/s390x/s390-virtio.h b/hw/s390x/s390-virtio.h
index 5c405e7..33847ae 100644
--- a/hw/s390x/s390-virtio.h
+++ b/hw/s390x/s390-virtio.h
@@ -12,6 +12,8 @@
 #ifndef HW_S390_VIRTIO_H
 #define HW_S390_VIRTIO_H 1
 
+#include "hw/nmi.h"
+
 #define KVM_S390_VIRTIO_NOTIFY  0
 #define KVM_S390_VIRTIO_RESET   1
 #define KVM_S390_VIRTIO_SET_STATUS  2
@@ -26,4 +28,5 @@ void s390_init_ipl_dev(const char *kernel_filename,
const char *initrd_filename,
const char *firmware);
 void s390_create_virtio_net(BusState *bus, const char *name);
+void s390_nmi(NMIState *n, int cpu_index, Error **errp);
 #endif
-- 
2.0.0

Re: [Qemu-devel] QEMU block layer todo list

[Benoît Canet]

The Wednesday 20 Aug 2014 à 09:57:39 (+0200), Christian Borntraeger wrote :
> On 19/08/14 16:20, Stefan Hajnoczi wrote:
> > The following wiki page contains a list of proposed tasks for the QEMU
> > block layer:
> > 
> > http://qemu-project.org/Features/Block/Todo
> > 
> > Benoit requested that we make the todo list viewable/editable.  This
> > is a good idea as it helps us focus and see what other developers are
> > thinking about.
> > 
> > There is no requirement to put tasks on the wiki.  Just sending
> > patches or discussing on the mailing list is fine.
> > 
> > The todo list is useful for large tasks or anything which cannot be
> > completed within a couple of weeks.  That way the tasks will not be
> > forgotten.
> > 
> > You can subscribe to the wiki changes RSS feed to follow edits:
> > http://qemu-project.org/index.php?title=Special:RecentChanges&feed=rss
> > 
> > Stefan
> > 
> 
> I would like to add geometry and block size passthrough for host devices. See
> the patches from Ekaterina Tumanova.
> https://lists.nongnu.org/archive/html/qemu-devel/2014-07/msg03791.html
> 
> On s390 we have a stricter requirement of passing through geometry and block 
> size for DASD disk devices as this geometry is more than a imaginary number 
> triplet. It is required for proper partition detection and is actually a 
> property of the real (or better emulated on storage servers like DS8x000) 
> disk.

I added this to the list.

Best regards

Benoît

> 
> Christian
> 
> 

Re: [Qemu-devel] [PATCH v7 3/4] s390x: Migrate to new NMI interface

[Alexey Kardashevskiy]

On 08/20/2014 10:05 PM, Alexander Graf wrote:
> 
> 
> On 20.08.14 13:38, Alexey Kardashevskiy wrote:
>> On 08/20/2014 09:20 PM, Alexander Graf wrote:
>>>
>>>
>>> On 20.08.14 12:20, Alexey Kardashevskiy wrote:
 On 07/17/2014 04:30 PM, Alexey Kardashevskiy wrote:
> On 07/15/2014 06:29 AM, Paolo Bonzini wrote:
>> Il 14/07/2014 05:17, Alexey Kardashevskiy ha scritto:
> No, you should politely bother them again.
>>>
>>> That does not seem helping though :-/
>>
>> Sorry. :)
>>
>> Well, patch 1 is the same as v6 and for the others Cornelia and Alex can
>> ack it.  It looks good to me though.
>
>
> Let me clarify things here.
>
> Alex rb'ed to 0/4, Cornelia rb'ed 2/4 and 3/4, so I put Alex's rb to
> 1&2&3&4&5, Cornelia's rb to 2&3, rebase it on top of the current upstream,
> repost after 2.1 is released and resume annoying people again. Is that 
> correct?
>
> This is my current set:
> 5/5 pc_piix: Migrate to new NMI interface
> 4/5 spapr: Add support for new NMI interface
> 3/5 s390x: Migrate to new NMI interface
> 2/5 s390x: Convert QEMUMachine to MachineClass
> 1/5 cpus: Define callback for QEMU "nmi" command


 Alex, ping.

>>>
>>>
>>> Uh, what exactly do you expect from me? :)
>>
>> No idea :) You could take patches to your tree. I do not understand what to
>> do with this set now. Please help. Thanks.
> 
> The way I understood your last email you wanted to rebase and repost
> them at which point someone could pick them up :)

I wanted answer to "is that correct?" :)
And I tried replaying the patchset on top of your ppc-next, it applied. But
I'll repost to make sure that we have the same thing.



-- 
Alexey

[Qemu-devel] [PATCH v8 4/5] spapr: Add support for new NMI interface

[Alexey Kardashevskiy]

This implements an NMI interface POWERPC SPAPR machine.
This enables an "nmi" HMP/QMP command supported on SPAPR.

This calls POWERPC_EXCP_RESET (vector 0x100) in the guest to deliver NMI
to every CPU. The expected result is XMON (in-kernel debugger) invocation.

Signed-off-by: Alexey Kardashevskiy 
Reviewed-by: Alexander Graf 
---
Changes:
v7:
* s/ppc_cpu_do_nmi/ppc_cpu_do_system_reset/

v6:
* support NMI interface

v4:
* s/\/nmi_monitor_handler/
* added note about XMON into commit log

v3:
* ppc_cpu_do_nmi() is exported from excp_helper.c instead of powerpc_excp()
---
 hw/ppc/spapr.c   | 21 +
 target-ppc/cpu-qom.h |  1 +
 target-ppc/excp_helper.c |  8 
 3 files changed, 30 insertions(+)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 5c92707..c619c3b 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -55,6 +55,7 @@
 #include "qemu/config-file.h"
 #include "qemu/error-report.h"
 #include "trace.h"
+#include "hw/nmi.h"
 
 #include 
 
@@ -1639,10 +1640,28 @@ static void spapr_machine_initfn(Object *obj)
 spapr_get_kvm_type, spapr_set_kvm_type, NULL);
 }
 
+static void ppc_cpu_do_nmi_on_cpu(void *arg)
+{
+CPUState *cs = arg;
+
+cpu_synchronize_state(cs);
+ppc_cpu_do_system_reset(cs);
+}
+
+static void spapr_nmi(NMIState *n, int cpu_index, Error **errp)
+{
+CPUState *cs;
+
+CPU_FOREACH(cs) {
+async_run_on_cpu(cs, ppc_cpu_do_nmi_on_cpu, cs);
+}
+}
+
 static void spapr_machine_class_init(ObjectClass *oc, void *data)
 {
 MachineClass *mc = MACHINE_CLASS(oc);
 FWPathProviderClass *fwc = FW_PATH_PROVIDER_CLASS(oc);
+NMIClass *nc = NMI_CLASS(oc);
 
 mc->name = "pseries";
 mc->desc = "pSeries Logical Partition (PAPR compliant)";
@@ -1656,6 +1675,7 @@ static void spapr_machine_class_init(ObjectClass *oc, 
void *data)
 mc->kvm_type = spapr_kvm_type;
 
 fwc->get_dev_path = spapr_get_fw_dev_path;
+nc->nmi_monitor_handler = spapr_nmi;
 }
 
 static const TypeInfo spapr_machine_info = {
@@ -1666,6 +1686,7 @@ static const TypeInfo spapr_machine_info = {
 .class_init= spapr_machine_class_init,
 .interfaces = (InterfaceInfo[]) {
 { TYPE_FW_PATH_PROVIDER },
+{ TYPE_NMI },
 { }
 },
 };
diff --git a/target-ppc/cpu-qom.h b/target-ppc/cpu-qom.h
index 0fee36f..a379f79 100644
--- a/target-ppc/cpu-qom.h
+++ b/target-ppc/cpu-qom.h
@@ -127,6 +127,7 @@ int ppc64_cpu_write_elf64_qemunote(WriteCoreDumpFunction f,
 int ppc64_cpu_write_elf64_note(WriteCoreDumpFunction f, CPUState *cs,
int cpuid, void *opaque);
 #ifndef CONFIG_USER_ONLY
+void ppc_cpu_do_system_reset(CPUState *cs);
 extern const struct VMStateDescription vmstate_ppc_cpu;
 
 typedef struct PPCTimebase {
diff --git a/target-ppc/excp_helper.c b/target-ppc/excp_helper.c
index be71590..922e86d 100644
--- a/target-ppc/excp_helper.c
+++ b/target-ppc/excp_helper.c
@@ -810,6 +810,14 @@ void ppc_hw_interrupt(CPUPPCState *env)
 }
 }
 }
+
+void ppc_cpu_do_system_reset(CPUState *cs)
+{
+PowerPCCPU *cpu = POWERPC_CPU(cs);
+CPUPPCState *env = &cpu->env;
+
+powerpc_excp(cpu, env->excp_model, POWERPC_EXCP_RESET);
+}
 #endif /* !CONFIG_USER_ONLY */
 
 #if defined(DEBUG_OP)
-- 
2.0.0

[Qemu-devel] [PATCH v8 1/5] cpus: Define callback for QEMU "nmi" command

[Alexey Kardashevskiy]

This introduces an NMI (Non Maskable Interrupt) interface with
a single nmi_monitor_handler() method. A machine or a device can
implement it. This searches for an QOM object with this interface
and if it is implemented, calls it. The callback implements an action
required to cause debug crash dump on in-kernel debugger invocation.
The callback returns Error**.

This adds a nmi_monitor_handle() helper which walks through
all objects to find the interface. The interface method is called
for all found instances.

This adds support for it in qmp_inject_nmi(). Since no architecture
supports it at the moment, there is no change in behaviour.

This changes inject-nmi command description for HMP and QMP.

Signed-off-by: Alexey Kardashevskiy 
Reviewed-by: Alexander Graf 
---
Changes:
v8:
* added missing "static" to nmi_children() definition

v7:
* s/NMI/NMIState/
* added copyrighs to new files
* s/an Non/a Non/
* s/nmi()/nmi_monitor_handle()/

v6:
* NMI is an interface again

v5:
* s/given guest's (CPU|VCPU)/default CPU/
* nmi_monitor_handler() now returns Error**

v4:
* s/\/nmi_monitor_handler/

v3:
* actual nmi() enablement moved from last patch to first patch
* changed description for QMP command too
---
 cpus.c|  3 +-
 hmp-commands.hx   |  6 ++--
 hw/core/Makefile.objs |  1 +
 hw/core/nmi.c | 84 +++
 include/hw/nmi.h  | 49 ++
 qapi-schema.json  |  4 +--
 qmp-commands.hx   |  3 +-
 7 files changed, 141 insertions(+), 9 deletions(-)
 create mode 100644 hw/core/nmi.c
 create mode 100644 include/hw/nmi.h

diff --git a/cpus.c b/cpus.c
index 2b5c0bd..37d90f4 100644
--- a/cpus.c
+++ b/cpus.c
@@ -40,6 +40,7 @@
 #include "qemu/bitmap.h"
 #include "qemu/seqlock.h"
 #include "qapi-event.h"
+#include "hw/nmi.h"
 
 #ifndef _WIN32
 #include "qemu/compatfd.h"
@@ -1551,7 +1552,7 @@ void qmp_inject_nmi(Error **errp)
 }
 }
 #else
-error_set(errp, QERR_UNSUPPORTED);
+nmi_monitor_handle(monitor_get_cpu_index(), errp);
 #endif
 }
 
diff --git a/hmp-commands.hx b/hmp-commands.hx
index d0943b1..f859f8d 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -832,19 +832,17 @@ The values that can be specified here depend on the 
machine type, but are
 the same that can be specified in the @code{-boot} command line option.
 ETEXI
 
-#if defined(TARGET_I386) || defined(TARGET_S390X)
 {
 .name   = "nmi",
 .args_type  = "",
 .params = "",
-.help   = "inject an NMI on all guest's CPUs",
+.help   = "inject an NMI",
 .mhandler.cmd = hmp_inject_nmi,
 },
-#endif
 STEXI
 @item nmi @var{cpu}
 @findex nmi
-Inject an NMI (x86) or RESTART (s390x) on the given CPU.
+Inject an NMI on the default CPU (x86/s390) or all CPUs (ppc64).
 
 ETEXI
 
diff --git a/hw/core/Makefile.objs b/hw/core/Makefile.objs
index 5377d05..17845df 100644
--- a/hw/core/Makefile.objs
+++ b/hw/core/Makefile.objs
@@ -4,6 +4,7 @@ common-obj-y += fw-path-provider.o
 # irq.o needed for qdev GPIO handling:
 common-obj-y += irq.o
 common-obj-y += hotplug.o
+common-obj-y += nmi.o
 
 common-obj-$(CONFIG_EMPTY_SLOT) += empty_slot.o
 common-obj-$(CONFIG_XILINX_AXI) += stream.o
diff --git a/hw/core/nmi.c b/hw/core/nmi.c
new file mode 100644
index 000..3dff020
--- /dev/null
+++ b/hw/core/nmi.c
@@ -0,0 +1,84 @@
+/*
+ *  NMI monitor handler class and helpers.
+ *
+ *  Copyright IBM Corp., 2014
+ *
+ *  Author: Alexey Kardashevskiy 
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License,
+ *  or (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, see .
+ */
+
+#include "hw/nmi.h"
+#include "qapi/qmp/qerror.h"
+
+struct do_nmi_s {
+int cpu_index;
+Error *errp;
+bool handled;
+};
+
+static void nmi_children(Object *o, struct do_nmi_s *ns);
+
+static int do_nmi(Object *o, void *opaque)
+{
+struct do_nmi_s *ns = opaque;
+NMIState *n = (NMIState *) object_dynamic_cast(o, TYPE_NMI);
+
+if (n) {
+NMIClass *nc = NMI_GET_CLASS(n);
+
+ns->handled = true;
+nc->nmi_monitor_handler(n, ns->cpu_index, &ns->errp);
+if (ns->errp) {
+return -1;
+}
+}
+nmi_children(o, ns);
+
+return 0;
+}
+
+static void nmi_children(Object *o, struct do_nmi_s *ns)
+{
+object_child_foreach(o, do_nmi, ns);
+}
+
+void nmi_monitor_handle(int cpu_index, Error **errp)
+{
+struct do_nmi_s ns = {
+

[Qemu-devel] [PATCH v8 0/5] cpus: Add generic "nmi" monitor command support

[Alexey Kardashevskiy]

This adds an "nmi" monitor command handler per CPUs.
x86, s390 and ppc CPUS are supported.

Please comment. Thanks.

Changes:
v8:
* adjusted commit log for s390 patch
* fixed missing "static" in 1/4

v7:
* fixed typenames, function names, copyrights
* s390x QOM'ed for later addition of the NMI interface
* Since none of x86 machines is QOM'ed, postpone migration to new interface for 
x86

v6:
* back 5 steps and make it an interface again

v5:
* added Error** to the callback
* fixed some comments

v4:
* now it is not nmi() but nmi_monitor_handler() to avoid confusion

v3:
* patches reorganized
* comments from v2 addressed, more details are in individual commit logs

v2:
* moved from machine interface to CPUClass callback
* s390 and x86 moved to target-s390/target-i386
* x86 handler delivers to the current CPU only now




Alexey Kardashevskiy (5):
  cpus: Define callback for QEMU "nmi" command
  s390x: Convert QEMUMachine to MachineClass
  s390x: Migrate to new NMI interface
  spapr: Add support for new NMI interface
  pc_piix: Migrate to new NMI interface

 cpus.c | 31 ++---
 hmp-commands.hx|  6 ++--
 hw/core/Makefile.objs  |  1 +
 hw/core/nmi.c  | 84 ++
 hw/i386/pc_piix.c  | 42 +++
 hw/ppc/spapr.c | 21 
 hw/s390x/s390-virtio-ccw.c | 49 ++-
 hw/s390x/s390-virtio.c | 59 ++--
 hw/s390x/s390-virtio.h |  3 ++
 include/hw/nmi.h   | 49 +++
 qapi-schema.json   |  4 +--
 qmp-commands.hx|  3 +-
 target-ppc/cpu-qom.h   |  1 +
 target-ppc/excp_helper.c   |  8 +
 14 files changed, 291 insertions(+), 70 deletions(-)
 create mode 100644 hw/core/nmi.c
 create mode 100644 include/hw/nmi.h

-- 
2.0.0

Re: [Qemu-devel] [PATCH 2/3] ioh3420: Provide a unique bus name and an interrupt mapping function

[Michael S. Tsirkin]

On Wed, Aug 20, 2014 at 02:06:38PM +0200, Markus Armbruster wrote:
> Knut Omang  writes:
> 
> > On Wed, 2014-08-20 at 10:52 +0200, Paolo Bonzini wrote:
> >> Il 20/08/2014 08:53, Knut Omang ha scritto:
> >> > A unique bus name is necessary to be able to refer to each instance
> >> > from the command line and monitors.
> >> 
> >> Is it needed?  Can't you just add id= to the -device option?
> >
> > Yes, as far as I understand the problem is that the id= would work on
> > the ioh3420 device itself, while what is needed here is to name the
> > secondary bus of the ioh3420, which I haven't found a way to name from
> > the command line.
> 
> Bus names in qdev are a mess.  Here are the rules:
> 
> 1. If code provides a name, that's the name.
> 
> 2. Else, if the device has an ID, the name is ID.N, where N counts the
> device's buses from zero.
> 
> 3. Else, the name is BUS-TYPE-NAME.N, where N counts the buses of this
> type from zero.
> 
> This results in a usable bus name unless device IDs collide with bus
> type names, or the code provides names that collide.
> 
> The user needs to take care to use IDs that don't collide with bus
> names.  Adding new bus names may screw some users.
> 
> The user needs to take further care to use IDs whenever the code
> provides a bus name that collides.  Adding code that provides bus names
> may screw some users.
> 
> Broken by design.
> 
> The problem here is "code provides names that collide": device
> q35-pcihost provides the name "pcie.0".  Bound to collide with the first
> PCIE bus named under rule 3.  For instance, if you next add an ioh3420
> without ID, its bus is also named "pcie.0".
> 
> Rule 1 should be taken out and shot.  Unfortunately, that'll break ABI
> left & right.  Instead, we can try to reduce its use.  The appended does
> exactly that for q35-pcihost.  With it applied, the bus provided by
> q35-pcihost still gets the same name "pcie.0", but under rule 3 instead
> of rule 1.  Rule 3 then names further PCIE buses "pcie.1", "pcie.2", ...
> instead of "pcie.0", "pcie.1", ...  Better, but it's still an ABI break.
> 
> > Maybe an even better solution would be to have default names for
> > everything, if not specified, from a user friendliness perspective? 
> 
> Buses *have* a default name!  You're confusing this with device IDs,
> which exist only when the user sets one.
> 
> Changes in this area are difficult, because the names are all ABI.
> Names that cannot be used are fair game, of course.
> 
> > I suppose this is a more general issue of sensible default values
> > though, but the fact that it is easy to create devices which cannot be
> > referred has caused me some confusion from time to time.
> 
> Picking default qdev IDs risks collisions with the user's IDs.  We
> shouldn't do that.  We do it anyway in a few places, for historical
> reasons.
> 
> QOM paths might be a sane way to let users refer to devices without IDs.
> 
> 
> While writing the above, I stumbled another rule 1 screwup: pci_bridge.c
> attempts to "improve" the boring standard bus names chosen via rule 2 or
> 3.
> 
> pci_bridge_initfn() provides a bus name of its own (commit 8a3d80f
> pci_bridge: user-friendly default bus name):
> a. If pci_bridge_map_irq() set a bus name, that's the name.
> 
> b. Else, if the device has an ID, that's the name.  Thus, ID.N is
> "improved" to just ID, at the cost of a special case: now users have to
> avoid not just IDs of the form BUS-TYPE-NAME.N, but also plain
> BUS-TYPE-NAME.
> 
> Callers of pci_bridge_map_irq() generally provide a name.  Some names
> contain spaces, thus can't collide (but would be bloody inconvenient on
> the command line or in the monitor).  Others don't, but thankfully the
> ones I checked are in dead code.  Craptastic.
> 
> 
> diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
> index 37f228e..469aafd 100644
> --- a/hw/pci-host/q35.c
> +++ b/hw/pci-host/q35.c
> @@ -47,7 +47,7 @@ static void q35_host_realize(DeviceState *dev, Error **errp)
>  sysbus_add_io(sbd, MCH_HOST_BRIDGE_CONFIG_DATA, &pci->data_mem);
>  sysbus_init_ioports(sbd, MCH_HOST_BRIDGE_CONFIG_DATA, 4);
>  
> -pci->bus = pci_bus_new(DEVICE(s), "pcie.0",
> +pci->bus = pci_bus_new(DEVICE(s), NULL,
> s->mch.pci_address_space, s->mch.address_space_io,
> 0, TYPE_PCIE_BUS);
>  qdev_set_parent_bus(DEVICE(&s->mch), BUS(pci->bus));

This is for the root bus, I think it won't help Knut who's trying to
add devices behind root ports.

[Qemu-devel] [PATCH] kvm: run cpu state synchronization on target vcpu thread

[David Hildenbrand]

As already done for kvm_cpu_synchronize_state(), let's trigger
kvm_arch_put_registers() via run_on_cpu() for kvm_cpu_synchronize_post_reset()
and kvm_cpu_synchronize_post_init().

This way, we make sure that the register synchronizing ioctls are
called from the proper vcpu thread; this avoids calls to
synchronize_rcu() in the kernel.

Reviewed-by: Cornelia Huck 
Signed-off-by: David Hildenbrand 
---
 kvm-all.c | 18 --
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index 1402f4f..b240bf8 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -1669,18 +1669,32 @@ void kvm_cpu_synchronize_state(CPUState *cpu)
 }
 }
 
-void kvm_cpu_synchronize_post_reset(CPUState *cpu)
+static void do_kvm_cpu_synchronize_post_reset(void *arg)
 {
+CPUState *cpu = arg;
+
 kvm_arch_put_registers(cpu, KVM_PUT_RESET_STATE);
 cpu->kvm_vcpu_dirty = false;
 }
 
-void kvm_cpu_synchronize_post_init(CPUState *cpu)
+void kvm_cpu_synchronize_post_reset(CPUState *cpu)
+{
+run_on_cpu(cpu, do_kvm_cpu_synchronize_post_reset, cpu);
+}
+
+static void do_kvm_cpu_synchronize_post_init(void *arg)
 {
+CPUState *cpu = arg;
+
 kvm_arch_put_registers(cpu, KVM_PUT_FULL_STATE);
 cpu->kvm_vcpu_dirty = false;
 }
 
+void kvm_cpu_synchronize_post_init(CPUState *cpu)
+{
+run_on_cpu(cpu, do_kvm_cpu_synchronize_post_init, cpu);
+}
+
 int kvm_cpu_exec(CPUState *cpu)
 {
 struct kvm_run *run = cpu->kvm_run;
-- 
1.8.5.5

Re: [Qemu-devel] [Bug] QEMU crashed with latest qemu master tree code

[Gonglei]

Hi,

Anyone encounter the same issue with me? Thanks. 

Best regards,
-Gonglei

> -Original Message-
> Subject: [Qemu-devel] [Bug] QEMU crashed with latest qemu master tree code
> 
> Hi,
> 
> Command line:
> 
> #./qemu-system-x86_64 -enable-kvm -m 2048 -machine q35 -device
> ide-drive,bus=ide.2,drive=MacHDD \
> -drive id=MacHDD,if=none,file=/mnt/sdb/gonglei/image/redhat_q35.img
> -monitor stdio -vnc :10 -readconfig ../docs/q35-chipset.cfg
> (qemu) Segmentation fault (core dumped)
> 
> Some clews:
> 
> 1. the bad commit is 0e4a77370594c91dd126f9872893ed473374cc72
> 2. if I do not add command line argument '-readconfig ../docs/q35-chipset.cfg 
> ',
> it works well.
> 3. backtrace as below:
> 
> Program received signal SIGSEGV, Segmentation fault.
> [Switching to Thread 0x7257c700 (LWP 27649)]
> 0x558b4ad3 in object_class_dynamic_cast (class=0x56523720,
> typename=0x559ba852 "cpu") at qom/object.c:502
> 502 if (type->name == typename) {
> (gdb) p *type
> Cannot access memory at address 0x0
> (gdb) bt
> #0  0x558b4ad3 in object_class_dynamic_cast
> (class=0x56523720, typename=0x559ba852 "cpu") at
> qom/object.c:502
> #1  0x558b485a in object_dynamic_cast (obj=0x5643abe0,
> typename=0x559ba852 "cpu") at qom/object.c:443
> #2  0x556a4ee3 in acpi_add_cpu_info (o=0x5643abe0,
> opaque=0x7257b930) at /home/new/qemu/hw/i386/acpi-build.c:133
> #3  0x558b50fa in object_child_foreach (obj=0x563e8770,
> fn=0x556a4eac , opaque=0x7257b930)
> at qom/object.c:676
> #4  0x556a4f4b in acpi_add_cpu_info (o=0x563e8770,
> opaque=0x7257b930) at /home/new/qemu/hw/i386/acpi-build.c:140
> #5  0x558b50fa in object_child_foreach (obj=0x562efa40,
> fn=0x556a4eac , opaque=0x7257b930)
> at qom/object.c:676
> #6  0x556a4f4b in acpi_add_cpu_info (o=0x562efa40,
> opaque=0x7257b930) at /home/new/qemu/hw/i386/acpi-build.c:140
> #7  0x558b50fa in object_child_foreach (obj=0x562d26a0,
> fn=0x556a4eac , opaque=0x7257b930)
> at qom/object.c:676
> #8  0x556a4f4b in acpi_add_cpu_info (o=0x562d26a0,
> opaque=0x7257b930) at /home/new/qemu/hw/i386/acpi-build.c:140
> #9  0x558b50fa in object_child_foreach (obj=0x562d5770,
> fn=0x556a4eac , opaque=0x7257b930)
> at qom/object.c:676
> #10 0x556a5024 in acpi_get_cpu_info (cpu=0x7257b930) at
> /home/new/qemu/hw/i386/acpi-build.c:149
> #11 0x556a8bb7 in acpi_build (guest_info=0x56309880,
> tables=0x7257b980) at /home/new/qemu/hw/i386/acpi-build.c:1486
> #12 0x556a90cd in acpi_build_update
> (build_opaque=0x564353b0, offset=0) at
> /home/new/qemu/hw/i386/acpi-build.c:1623
> #13 0x55826373 in fw_cfg_read (s=0x5630d010) at
> hw/nvram/fw_cfg.c:255
> #14 0x55826565 in fw_cfg_comb_read (opaque=0x5630d010,
> addr=1, size=1) at hw/nvram/fw_cfg.c:291
> #15 0x55645f52 in memory_region_read_accessor
> (mr=0x5630f4f0, addr=1, value=0x7257bb10, size=1, shift=0,
> mask=255)
> at /home/new/qemu/memory.c:410
> #16 0x5564624c in access_with_adjusted_size (addr=1,
> value=0x7257bb10, size=1, access_size_min=1, access_size_max=4,
> access=0x55645ef4 ,
> mr=0x5630f4f0) at /home/new/qemu/memory.c:480
> #17 0x55649651 in memory_region_dispatch_read1
> (mr=0x5630f4f0, addr=1, size=1) at /home/new/qemu/memory.c:1096
> #18 0x55649709 in memory_region_dispatch_read
> (mr=0x5630f4f0, addr=1, pval=0x7257bc00, size=1)
> at /home/new/qemu/memory.c:1118
> #19 0x5564d325 in io_mem_read (mr=0x5630f4f0, addr=1,
> pval=0x7257bc00, size=1) at /home/new/qemu/memory.c:1961
> #20 0x555f7bfe in address_space_rw (as=0x55df6d20
> , addr=1297, buf=0x77ff3000 "&", len=1,
> is_write=false) at /home/new/qemu/exec.c:2086
> #21 0x556429d6 in kvm_handle_io (port=1297, data=0x77ff3000,
> direction=0, size=1, count=1024)
> at /home/new/qemu/kvm-all.c:1597
> #22 0x55642f7a in kvm_cpu_exec (cpu=0x562f3060) at
> /home/new/qemu/kvm-all.c:1734
> #23 0x5562a4d8 in qemu_kvm_cpu_thread_fn (arg=0x562f3060)
> at /home/new/qemu/cpus.c:939
> #24 0x7572a7f6 in start_thread () from /lib64/libpthread.so.0
> #25 0x7548609d in clone () from /lib64/libc.so.6
> #26 0x in ?? ()
> (gdb) p *class
> $2 = {type = 0x0, interfaces = 0x21 = {
> (gdb)
> 
> Best regards,
> -Gonglei
> 
> 

Re: [Qemu-devel] Recent SeaBIOS too big for QEMU -initrd

[Stefan Hajnoczi]

On Wed, Aug 20, 2014 at 12:27 PM, Michael S. Tsirkin  wrote:
> On Wed, Aug 20, 2014 at 11:27:41AM +0100, Stefan Hajnoczi wrote:
>> The QEMU -initrd option loads the initrd at the top of RAM.  There is
>> a 64 KB safety region for ACPI tables in hw/i386/pc.c:load_linux():
>>
>>   initrd_max = max_ram_size-ACPI_DATA_SIZE-1;
>>
>> QEMU's bios-256k.bin SeaBIOS build reserves 128 KB at the top of
>> memory so the 64 KB ACPI data size has become too small.
>>
>> The guest Linux kernel rejects the initrd:
>> BIOS-provided physical RAM map:
>>  BIOS-e820: 0001 - 0009fc00 (usable)
>>  BIOS-e820: 0009fc00 - 000a (reserved)
>>  BIOS-e820: 000f - 0010 (reserved)
>>  BIOS-e820: 0010 - 3ffe (usable)
>>  BIOS-e820: 3ffe - 4000 (reserved)
>>  BIOS-e820: feffc000 - ff00 (reserved)
>>  BIOS-e820: fffc - 0001 (reserved)
>> ...
>> initrd extends beyond end of memory (0x3ffef79f > 0x3ffe)
>> disabling initrd
>>
>> It is easy enough to "fix" the problem by bumping ACPI_DATA_SIZE up to
>> 0x2 in QEMU.  Perhaps this should only be done for bios-256k.bin
>> guests and not bios-128k.bin guests (QEMU 1.7 and older machine
>> types).
>>
>> Perhaps QEMU -> SeaBIOS -> linuxboot.bin can be simplified so QEMU
>> doesn't have to guess what e820 region SeaBIOS will reserve.
>> linuxboot.bin would probably be the place to do it unless SeaBIOS has
>> Linux loading functionality that could be reused.
>>
>> Any ideas?
>>
>> Stefan
>
> I would say 2.1 and up, -M pc-i440fx-2.0 works fine, or am I mistaken?

pc-i440fx-2.0 works fine.  pc-i440fx-2.1 does not.

Stefan

Re: [Qemu-devel] [PATCH v5 00/15] TriCore architecture guest implementation

[Bastian Koppelmann]

ping, anyone?

Patchwork links:

01: https://patchwork.ozlabs.org/patch/379594/
02: https://patchwork.ozlabs.org/patch/379598/
03: https://patchwork.ozlabs.org/patch/379600/
04: https://patchwork.ozlabs.org/patch/379588/
05: https://patchwork.ozlabs.org/patch/379589/
06: https://patchwork.ozlabs.org/patch/379591/
07: https://patchwork.ozlabs.org/patch/379587/
08: https://patchwork.ozlabs.org/patch/379596/
09: https://patchwork.ozlabs.org/patch/379597/
10: https://patchwork.ozlabs.org/patch/379592/
11: https://patchwork.ozlabs.org/patch/379593/
12: https://patchwork.ozlabs.org/patch/379595/
13: https://patchwork.ozlabs.org/patch/379586/
14: https://patchwork.ozlabs.org/patch/379590/
15: https://patchwork.ozlabs.org/patch/379599/

Thanks,

Bastian

On 08/13/2014 01:07 PM, Bastian Koppelmann wrote:

Hi,

my aim is to add Infineon's TriCore architecture to QEMU. This series of 
patches adds the target stubs, a basic testboard and a softmmu for system mode 
emulation. Furthermore it adds all the 16 bit long instructions of the 
architecture grouped by opcode format.

After this series of patches. Another one will follow, which adds a lot of the 
32 bit long instructions.

All the best

Bastian

v4 -> v5:
 - Change documentation of S, SV bits to use bit 31.
 - psw_read/_write now uses only bit 31 of S, SV bits.
 - gen_intermediate_code_internal: Move calculation of next_pc after the 
ifs for singlestep and long tbs.
 - gen_shaci: Change case of shift_count == 32 to shift_count == -32 and 
add the clear of V bit.
 - gen_shaci: Move creation and freeing of t_max, t_min to case shift_count 
> 0.
 - gen_shaci: Add cast to int32_t to the creation of t_min.
 - gen_shaci: Now msk is 1 bit longer and uses a positive value of 
shift_count in the else case.
 - gen_shaci: now computes V bit in bit 31.
 - gen_shaci: Move computation of shift after the computation of PSW bits 
to handle the case of ret = r1.
 - gen_shaci: Add clear of V bit for cases shift_count = 0 / = -32 / < 0.
 - gen_add_d now saves result of addition into tcg temp to handle ret = r1 
cases.
 - gen_cond_add now sets SV, SAV bits conditionally.
 - gen_cond_add now writes result after PSW bit computations, to handle ret 
= r1 cases.
 - Change int cond to TCGCond on function gen_cond_add, gen_condi_add.
 - Switched source and destination register of SRC_ADD_A15 and SRC_ADD_15A 
insns.
 - Negate conditions of 16_SRC_CMOV and 16_SRC_CMOVN insns.
 - SRC_MOV_A loads const4 zero extended. (Googled alot to find a manual 
saying it should be sign extended, but couldn't. 1.3 and 1.6 manual state sign 
extension)
 - RSUB now computes V bit in bit 31.
 - gen_sub_d now saves result of substraction into tcg temp to handle ret = 
r1 cases.
 - gen_mul_i32s now calculates V, SV bits in bit 31.
 - SSOV makro now computes V, SV bits in bit 31.
 - Negate conditions of 16_SRR_CMOV and 16_SRR_CMOVN insns.
 - MOV_AA: Switch r1 and r2 arguments.
 - decode_16Bit_opc: Add if to handle ADDSC.A opcode being 6 bit instead of 
7 bit long.
 - Change int cond to TCGCond in functions gen_branch_cond, 
gen_branch_condi.
 - gen_intermediate_code_internal: Add gen_goto_tb so qemu finds new tb on 
singlestep and after long tbs without branch.
 - Switched sat_neg and arg in first movcond in function gen_saturate.
 - SR_NOT: Remove if checking OP2.


Bastian Koppelmann (15):
   target-tricore: Add target stubs and qom-cpu
   target-tricore: Add board for systemmode
   target-tricore: Add softmmu support
   target-tricore: Add initialization for translation and activate target
   target-tricore: Add masks and opcodes for decoding
   target-tricore: Add instructions of SRC opcode format
   target-tricore: Add instructions of SRR opcode format
   target-tricore: Add instructions of SSR opcode format
   target-tricore: Add instructions of SRRS and SLRO opcode format
   target-tricore: Add instructions of SB opcode format
   target-tricore: Add instructions of SBC and SBRN opcode format
   target-tricore: Add instructions of SBR opcode format
   target-tricore: Add instructions of SC opcode format
   target-tricore: Add instructions of SLR, SSRO and SRO opcode format
   target-tricore: Add instructions of SR opcode format

  arch_init.c |2 +
  configure   |5 +
  cpu-exec.c  |   11 +-
  cpus.c  |6 +
  default-configs/tricore-softmmu.mak |3 +
  hw/tricore/Makefile.objs|1 +
  hw/tricore/tricore_testboard.c  |  129 
  include/elf.h   |2 +
  include/hw/tricore/tricore.h|   54 ++
  include/sysemu/arch_init.h  |1 +
  target-tricore/Makefile.objs|1 +
  target-tricore/cpu-qom.h|   71 ++
  target-tricore/cpu.c|  191 ++

Re: [Qemu-devel] [PATCH 2/3] ioh3420: Provide a unique bus name and an interrupt mapping function

[Markus Armbruster]

"Michael S. Tsirkin"  writes:

> On Wed, Aug 20, 2014 at 02:06:38PM +0200, Markus Armbruster wrote:
>> Knut Omang  writes:
>> 
>> > On Wed, 2014-08-20 at 10:52 +0200, Paolo Bonzini wrote:
>> >> Il 20/08/2014 08:53, Knut Omang ha scritto:
>> >> > A unique bus name is necessary to be able to refer to each instance
>> >> > from the command line and monitors.
>> >> 
>> >> Is it needed?  Can't you just add id= to the -device option?
>> >
>> > Yes, as far as I understand the problem is that the id= would work on
>> > the ioh3420 device itself, while what is needed here is to name the
>> > secondary bus of the ioh3420, which I haven't found a way to name from
>> > the command line.
>> 
>> Bus names in qdev are a mess.  Here are the rules:
>> 
>> 1. If code provides a name, that's the name.
>> 
>> 2. Else, if the device has an ID, the name is ID.N, where N counts the
>> device's buses from zero.
>> 
>> 3. Else, the name is BUS-TYPE-NAME.N, where N counts the buses of this
>> type from zero.
>> 
>> This results in a usable bus name unless device IDs collide with bus
>> type names, or the code provides names that collide.
>> 
>> The user needs to take care to use IDs that don't collide with bus
>> names.  Adding new bus names may screw some users.
>> 
>> The user needs to take further care to use IDs whenever the code
>> provides a bus name that collides.  Adding code that provides bus names
>> may screw some users.
>> 
>> Broken by design.
>> 
>> The problem here is "code provides names that collide": device
>> q35-pcihost provides the name "pcie.0".  Bound to collide with the first
>> PCIE bus named under rule 3.  For instance, if you next add an ioh3420
>> without ID, its bus is also named "pcie.0".
>> 
>> Rule 1 should be taken out and shot.  Unfortunately, that'll break ABI
>> left & right.  Instead, we can try to reduce its use.  The appended does
>> exactly that for q35-pcihost.  With it applied, the bus provided by
>> q35-pcihost still gets the same name "pcie.0", but under rule 3 instead
>> of rule 1.  Rule 3 then names further PCIE buses "pcie.1", "pcie.2", ...
>> instead of "pcie.0", "pcie.1", ...  Better, but it's still an ABI break.
>> 
>> > Maybe an even better solution would be to have default names for
>> > everything, if not specified, from a user friendliness perspective? 
>> 
>> Buses *have* a default name!  You're confusing this with device IDs,
>> which exist only when the user sets one.
>> 
>> Changes in this area are difficult, because the names are all ABI.
>> Names that cannot be used are fair game, of course.
>> 
>> > I suppose this is a more general issue of sensible default values
>> > though, but the fact that it is easy to create devices which cannot be
>> > referred has caused me some confusion from time to time.
>> 
>> Picking default qdev IDs risks collisions with the user's IDs.  We
>> shouldn't do that.  We do it anyway in a few places, for historical
>> reasons.
>> 
>> QOM paths might be a sane way to let users refer to devices without IDs.
>> 
>> 
>> While writing the above, I stumbled another rule 1 screwup: pci_bridge.c
>> attempts to "improve" the boring standard bus names chosen via rule 2 or
>> 3.
>> 
>> pci_bridge_initfn() provides a bus name of its own (commit 8a3d80f
>> pci_bridge: user-friendly default bus name):
>> a. If pci_bridge_map_irq() set a bus name, that's the name.
>> 
>> b. Else, if the device has an ID, that's the name.  Thus, ID.N is
>> "improved" to just ID, at the cost of a special case: now users have to
>> avoid not just IDs of the form BUS-TYPE-NAME.N, but also plain
>> BUS-TYPE-NAME.
>> 
>> Callers of pci_bridge_map_irq() generally provide a name.  Some names
>> contain spaces, thus can't collide (but would be bloody inconvenient on
>> the command line or in the monitor).  Others don't, but thankfully the
>> ones I checked are in dead code.  Craptastic.
>> 
>> 
>> diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
>> index 37f228e..469aafd 100644
>> --- a/hw/pci-host/q35.c
>> +++ b/hw/pci-host/q35.c
>> @@ -47,7 +47,7 @@ static void q35_host_realize(DeviceState *dev, Error 
>> **errp)
>>  sysbus_add_io(sbd, MCH_HOST_BRIDGE_CONFIG_DATA, &pci->data_mem);
>>  sysbus_init_ioports(sbd, MCH_HOST_BRIDGE_CONFIG_DATA, 4);
>>  
>> -pci->bus = pci_bus_new(DEVICE(s), "pcie.0",
>> +pci->bus = pci_bus_new(DEVICE(s), NULL,
>> s->mch.pci_address_space, 
>> s->mch.address_space_io,
>> 0, TYPE_PCIE_BUS);
>>  qdev_set_parent_bus(DEVICE(&s->mch), BUS(pci->bus));
>
> This is for the root bus, I think it won't help Knut who's trying to
> add devices behind root ports.

Read again, more slowly :)

Yes, I null the name of the root bus.  That makes the qdev machinery
derive the very same "pcie.0" name via rule 3 instead of rule 1, with
the side effect that future (non-root) PCIE buses get different names.
In particular, the next one named via rule 3 will be called "pcie.1"
instead of "pcie.0", making it

Re: [Qemu-devel] [PATCH 2/3] ioh3420: Provide a unique bus name and an interrupt mapping function

[Knut Omang]

On Wed, 2014-08-20 at 13:36 +0200, Michael S. Tsirkin wrote:
> On Wed, Aug 20, 2014 at 11:30:55AM +0200, Knut Omang wrote:
> > On Wed, 2014-08-20 at 10:52 +0200, Paolo Bonzini wrote:
> > > Il 20/08/2014 08:53, Knut Omang ha scritto:
> > > > A unique bus name is necessary to be able to refer to each instance
> > > > from the command line and monitors.
> > > 
> > > Is it needed?  Can't you just add id= to the -device option?
> > 
> > Yes, as far as I understand the problem is that the id= would work on
> > the ioh3420 device itself, while what is needed here is to name the
> > secondary bus of the ioh3420, which I haven't found a way to name from
> > the command line.
> 
> Did you try using the device name?

I believe I did, I tried a lot back and forth back then...

> For pci bridges, unless you set bus_name, bus name will
> match device itself. See this code:
> 
>  * If we don't specify the name, the bus will be addressed as
>  * .0, where id is the device id.
>  * Since PCI Bridge devices have a single bus each, we don't need
>  * the index:
>  * let users address the bus using the device name.
>  */
> if (!br->bus_name && dev->qdev.id && *dev->qdev.id) {
> br->bus_name = dev->qdev.id;
> }

but my testing of this may well have been before your patch with this
logic - the cost of being so slow with my patches - it won't happen
again...
 
Both with the somewhat counterintuitive second re-provision of the bus
name in

-pci_bridge_map_irq(br, g_strdup(tmp), pci_swizzle_map_irq_fn);
+pci_bridge_map_irq(br, br->bus_name, pci_swizzle_map_irq_fn);

and actually with removing it due to Alex's commit 659fefee which was
also not in when this was conceived, I am now able to instantiate two
devices on two ports by means of:

-device ioh3420,slot=0,id=pcie_port.0
-device ioh3420,slot=1,id=pcie_port.1
-device ,bus=pcie_port.0
-device ,bus=pcie_port.1

In light of Markus' enlightening description of the mess here, unless
there are more issues, I'll repost the rest of the changes without this
commit,

Thanks,

Knut

> > Maybe an even better solution would be to have default names for
> > everything, if not specified, from a user friendliness perspective? 
> >
> > I suppose this is a more general issue of sensible default values
> > though, but the fact that it is easy to create devices which cannot be
> > referred has caused me some confusion from time to time.
> > 
> > > Paolo
> > 
> > Thanks,
> > 
> > Knut

Re: [Qemu-devel] [PATCH 1/4] hd-geometry.c: Integrate HDIO_GETGEO in guessing for target-s390x

[Paolo Bonzini]

Il 20/08/2014 11:35, Christian Borntraeger ha scritto:
> On 20/08/14 10:19, Paolo Bonzini wrote:
>> Il 29/07/2014 14:27, Ekaterina Tumanova ha scritto:
>>> The new HDIO_GETGEO logic is required for two use cases:
>>> a) Support for geometries of Direct Attached Storage Disks (DASD)
>>> on s390x configured as backing of virtio block devices.
>>
>> Is this still relevant now that QEMU can emulate 512-byte sectors on top
>> of host devices with 4k sectors?
> 
> Yes, the geometry and the block size define the layout of the DASD disk 
> format. This defines how the ibm disk layout partition table looks like. So 
> partition detection of the IBM layout only works if values are correct. (see 
> linux block/partitions/ibm.c. it needs these values to correctly locate the 
> data structures).
> 
> Furthermore, if you do an mkfs in the host and the guest sees a different 
> block size all kind of strange things will happen when mounting, no?
> 
>>
>>> b) Support for FCP attached SCSI disks that do not yet have a
>>> partition table. Without this patch, fdisk -l on the host would
>>> return different results then fdisk -l in the guest.
>>
>> Can you provide an example?
> 
> scsi disk in the host:
> [root@host ~]#  sfdisk -g 
> /dev/disk/by-id/wwn-0x6005076305ffc1ae2593
> /dev/disk/by-id/wwn-0x6005076305ffc1ae2593: 1011 cylinders, 34 
> heads, 61 sectors/track
> 
> same scsi disk in the guest as virtio-blk:
> scsi disk in the guest:
> [root@guest ~]# sfdisk -g /dev/disk/by-id/virtio-d20
> /dev/disk/by-id/virtio-d20: 2080 cylinders, 16 heads, 63 sectors/track
> 
> 16/63 is currently hardcoded by QEMU, no matter what the host thinks. This 
> gets overridden as soon as there is a partition table.
> 
> command line was:
> -drive 
> file=/dev/disk/by-id/scsi-36005076305ffc1ae2593,if=none,id=drive-virtio-disk20,format=raw,serial=d20,cache=none,aio=native

But this risks changing whenever you move data from a disk to another
disk, or if you move a virtual DASD disk from physical DASD to physical
SCSI.

If s390 is so sensitive to the head count and number of sectors/track
(on x86 everything is done with LBAs nowadays, even in the firmware), I
think whatever management layer you use should always specify them
explicitly.

I'm not saying this patch shouldn't be included---but it should be
treated as a handy thing for developers rather than as a definitive fix.

Paolo

Re: [Qemu-devel] [PATCH v7 3/4] s390x: Migrate to new NMI interface

[Paolo Bonzini]

Il 20/08/2014 14:12, Alexander Graf ha scritto:
> 
> 
> On 20.08.14 14:11, Alexey Kardashevskiy wrote:
>> On 08/20/2014 10:05 PM, Alexander Graf wrote:
>>>
>>>
>>> On 20.08.14 13:38, Alexey Kardashevskiy wrote:
 On 08/20/2014 09:20 PM, Alexander Graf wrote:
>
>
> On 20.08.14 12:20, Alexey Kardashevskiy wrote:
>> On 07/17/2014 04:30 PM, Alexey Kardashevskiy wrote:
>>> On 07/15/2014 06:29 AM, Paolo Bonzini wrote:
 Il 14/07/2014 05:17, Alexey Kardashevskiy ha scritto:
>>> No, you should politely bother them again.
>
> That does not seem helping though :-/

 Sorry. :)

 Well, patch 1 is the same as v6 and for the others Cornelia and Alex 
 can
 ack it.  It looks good to me though.
>>>
>>>
>>> Let me clarify things here.
>>>
>>> Alex rb'ed to 0/4, Cornelia rb'ed 2/4 and 3/4, so I put Alex's rb to
>>> 1&2&3&4&5, Cornelia's rb to 2&3, rebase it on top of the current 
>>> upstream,
>>> repost after 2.1 is released and resume annoying people again. Is that 
>>> correct?
>>>
>>> This is my current set:
>>> 5/5 pc_piix: Migrate to new NMI interface
>>> 4/5 spapr: Add support for new NMI interface
>>> 3/5 s390x: Migrate to new NMI interface
>>> 2/5 s390x: Convert QEMUMachine to MachineClass
>>> 1/5 cpus: Define callback for QEMU "nmi" command
>>
>>
>> Alex, ping.
>>
>
>
> Uh, what exactly do you expect from me? :)

 No idea :) You could take patches to your tree. I do not understand what to
 do with this set now. Please help. Thanks.
>>>
>>> The way I understood your last email you wanted to rebase and repost
>>> them at which point someone could pick them up :)
>>
>> I wanted answer to "is that correct?" :)
>> And I tried replaying the patchset on top of your ppc-next, it applied. But
>> I'll repost to make sure that we have the same thing.
> 
> Oh, cool. I'll apply them as soon as I finished to recover my HD then :)

I can apply them to uq/master too.  Let me know.

Paolo

Re: [Qemu-devel] [PATCH v5 00/15] TriCore architecture guest implementation

[Andreas Färber]

Hi,

Am 13.08.2014 14:07, schrieb Bastian Koppelmann:
>  arch_init.c |2 +
>  configure   |5 +
>  cpu-exec.c  |   11 +-
>  cpus.c  |6 +
>  default-configs/tricore-softmmu.mak |3 +
>  hw/tricore/Makefile.objs|1 +
>  hw/tricore/tricore_testboard.c  |  129 
>  include/elf.h   |2 +
>  include/hw/tricore/tricore.h|   54 ++
>  include/sysemu/arch_init.h  |1 +
>  target-tricore/Makefile.objs|1 +
>  target-tricore/cpu-qom.h|   71 ++
>  target-tricore/cpu.c|  191 +
>  target-tricore/cpu.h|  400 ++
>  target-tricore/helper.c |  144 
>  target-tricore/helper.h |   25 +
>  target-tricore/op_helper.c  |  392 ++
>  target-tricore/translate.c  | 1259 +++
>  target-tricore/tricore-defs.h   |   28 +
>  target-tricore/tricore-opcodes.h| 1406 
> +++
>  20 files changed, 4130 insertions(+), 1 deletion(-)
>  create mode 100644 default-configs/tricore-softmmu.mak
>  create mode 100644 hw/tricore/Makefile.objs
>  create mode 100644 hw/tricore/tricore_testboard.c
>  create mode 100644 include/hw/tricore/tricore.h
>  create mode 100644 target-tricore/Makefile.objs
>  create mode 100644 target-tricore/cpu-qom.h
>  create mode 100644 target-tricore/cpu.c
>  create mode 100644 target-tricore/cpu.h
>  create mode 100644 target-tricore/helper.c
>  create mode 100644 target-tricore/helper.h
>  create mode 100644 target-tricore/op_helper.c
>  create mode 100644 target-tricore/translate.c
>  create mode 100644 target-tricore/tricore-defs.h
>  create mode 100644 target-tricore/tricore-opcodes.h

I still haven't really looked at the CPU QOM parts yet (and won't get to
that shortly, no blocker), in this diffstat I am missing a MAINTAINERS
entry that names and CCs someone (you?) to take care of all those new
target-tricore/, hw/tricore/ and hw/include/tricore/ files in the future.

Have you tested your new machine using "make check V=1"? Just wondering
whether qom-test passes. (Sorry if I already asked earlier.)

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 2/3] ioh3420: Provide a unique bus name and an interrupt mapping function

[Paolo Bonzini]

Il 20/08/2014 15:03, Markus Armbruster ha scritto:
>> >
>> > This is for the root bus, I think it won't help Knut who's trying to
>> > add devices behind root ports.
> Read again, more slowly :)
> 
> Yes, I null the name of the root bus.  That makes the qdev machinery
> derive the very same "pcie.0" name via rule 3 instead of rule 1, with
> the side effect that future (non-root) PCIE buses get different names.
> In particular, the next one named via rule 3 will be called "pcie.1"
> instead of "pcie.0", making it actually accessible.

I agree that this is a big improvement.

Paolo

Re: [Qemu-devel] [PATCH 2/3] ioh3420: Provide a unique bus name and an interrupt mapping function

[Paolo Bonzini]

Il 20/08/2014 13:36, Michael S. Tsirkin ha scritto:
> 
> For pci bridges, unless you set bus_name, bus name will
> match device itself. See this code:
> 
>  * If we don't specify the name, the bus will be addressed as
>  * .0, where id is the device id.
>  * Since PCI Bridge devices have a single bus each, we don't need
>  * the index:
>  * let users address the bus using the device name.
>  */
> if (!br->bus_name && dev->qdev.id && *dev->qdev.id) {
> br->bus_name = dev->qdev.id;
> }

Is libvirt using this rule?  If not, I'd rather slash it since the
.0 name is shared with all other buses and not PCI-bridge-specific.

Paolo

Re: [Qemu-devel] [PATCH v8] slirp/misc: Use the GLib memory allocation APIs

[Benoît Canet]

The Tuesday 19 Aug 2014 à 16:30:17 (+0800), zhanghailiang wrote :
> Here we don't check the return value of malloc() which may fail.
> Use the g_new() instead, which will abort the program when
> there is not enough memory.
> 
> Also, use g_strdup instead of strdup and remove the unnecessary
> strdup function.
> 
> Signed-off-by: zhanghailiang 
> Reviewed-by: Alex Bennée 
> ---
>  slirp/misc.c | 20 +++-
>  1 file changed, 3 insertions(+), 17 deletions(-)
> 
> diff --git a/slirp/misc.c b/slirp/misc.c
> index b8eb74c..6543dc7 100644
> --- a/slirp/misc.c
> +++ b/slirp/misc.c
> @@ -54,11 +54,11 @@ int add_exec(struct ex_list **ex_ptr, int do_pty, char 
> *exec,
>   }
>  
>   tmp_ptr = *ex_ptr;
> - *ex_ptr = (struct ex_list *)malloc(sizeof(struct ex_list));
> + *ex_ptr = g_new(struct ex_list, 1);
>   (*ex_ptr)->ex_fport = port;
>   (*ex_ptr)->ex_addr = addr;
>   (*ex_ptr)->ex_pty = do_pty;
> - (*ex_ptr)->ex_exec = (do_pty == 3) ? exec : strdup(exec);
> + (*ex_ptr)->ex_exec = (do_pty == 3) ? exec : g_strdup(exec);
>   (*ex_ptr)->ex_next = tmp_ptr;
>   return 0;
>  }
> @@ -187,7 +187,7 @@ fork_exec(struct socket *so, const char *ex, int do_pty)
>  bptr++;
>   c = *bptr;
>   *bptr++ = (char)0;
> - argv[i++] = strdup(curarg);
> + argv[i++] = g_strdup(curarg);
>  } while (c);
>  
>  argv[i] = NULL;
> @@ -228,20 +228,6 @@ fork_exec(struct socket *so, const char *ex, int do_pty)
>  }
>  #endif
>  
> -#ifndef HAVE_STRDUP
> -char *
> -strdup(str)
> - const char *str;
> -{
> - char *bptr;
> -
> - bptr = (char *)malloc(strlen(str)+1);
> - strcpy(bptr, str);
> -
> - return bptr;
> -}
> -#endif
> -
>  void slirp_connection_info(Slirp *slirp, Monitor *mon)
>  {
>  const char * const tcpstates[] = {
> -- 
> 1.7.12.4
> 
> 
> 
Reviewed-by: Benoît Canet 

Re: [Qemu-devel] [PATCH 2/6] stubs: Add iohandler.c

[Paolo Bonzini]

Il 20/08/2014 12:01, Fam Zheng ha scritto:
> Add stub function "qemu_set_fd_handler" which is used by
> util/event_notifier-posix.c.
> 
> Signed-off-by: Fam Zheng 
> ---
>  stubs/Makefile.objs |  1 +
>  stubs/iohandler.c   | 20 
>  2 files changed, 21 insertions(+)
>  create mode 100644 stubs/iohandler.c
> 
> diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
> index 5e347d0..d9cad1b 100644
> --- a/stubs/Makefile.objs
> +++ b/stubs/Makefile.objs
> @@ -40,3 +40,4 @@ stub-obj-$(CONFIG_WIN32) += fd-register.o
>  stub-obj-y += cpus.o
>  stub-obj-y += kvm.o
>  stub-obj-y += qmp_pc_dimm_device_list.o
> +stub-obj-y += iohandler.o
> diff --git a/stubs/iohandler.c b/stubs/iohandler.c
> new file mode 100644
> index 000..97c0ce5
> --- /dev/null
> +++ b/stubs/iohandler.c
> @@ -0,0 +1,20 @@
> +/*
> + * iohandler stub functions
> + *
> + * Copyright Red Hat, Inc., 2014
> + *
> + * Author: Fam Zheng 
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or
> + * later.  See the COPYING file in the top-level directory.
> + */
> +
> +#include "qemu/main-loop.h"
> +
> +int qemu_set_fd_handler(int fd,
> +IOHandler *fd_read,
> +IOHandler *fd_write,
> +void *opaque)
> +{
> +abort();
> +}
> 

Please merge with stubs/set-fd-handler.c.

Paolo

Re: [Qemu-devel] [PATCH 3/6] stubs: Add openpty.c

[Paolo Bonzini]

Il 20/08/2014 12:01, Fam Zheng ha scritto:
> Add function "openpty" which is used by util/qemu-openpty.c.
> 
> Signed-off-by: Fam Zheng 
> ---
>  stubs/Makefile.objs |  1 +
>  stubs/openpty.c | 21 +
>  2 files changed, 22 insertions(+)
>  create mode 100644 stubs/openpty.c
> 
> diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
> index d9cad1b..f5d2cdd 100644
> --- a/stubs/Makefile.objs
> +++ b/stubs/Makefile.objs
> @@ -41,3 +41,4 @@ stub-obj-y += cpus.o
>  stub-obj-y += kvm.o
>  stub-obj-y += qmp_pc_dimm_device_list.o
>  stub-obj-y += iohandler.o
> +stub-obj-y += openpty.o
> diff --git a/stubs/openpty.c b/stubs/openpty.c
> new file mode 100644
> index 000..31e6ff5
> --- /dev/null
> +++ b/stubs/openpty.c
> @@ -0,0 +1,21 @@
> +/*
> + * openpty stub
> + *
> + * Copyright Red Hat, Inc., 2014
> + *
> + * Author: Fam Zheng 
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or
> + * later.  See the COPYING file in the top-level directory.
> + */
> +
> +#include 
> +#include 
> +
> +int openpty(int *amaster, int *aslave, char *name,
> +const struct termios *termp,
> +const struct winsize *winp)
> +{
> +abort();
> +}
> +
> 

Please move -lutil from libs_softmmu to LIBS instead.

Paolo

Re: [Qemu-devel] [PATCH 4/6] stubs: Add timer.c

[Paolo Bonzini]

Il 20/08/2014 12:01, Fam Zheng ha scritto:
> Add timer functions that are used in util/throttle.c.
> 
> Signed-off-by: Fam Zheng 
> ---
>  stubs/Makefile.objs |  1 +
>  stubs/timer.c   | 44 
>  2 files changed, 45 insertions(+)
>  create mode 100644 stubs/timer.c
> 
> diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
> index f5d2cdd..1d433f3 100644
> --- a/stubs/Makefile.objs
> +++ b/stubs/Makefile.objs
> @@ -42,3 +42,4 @@ stub-obj-y += kvm.o
>  stub-obj-y += qmp_pc_dimm_device_list.o
>  stub-obj-y += iohandler.o
>  stub-obj-y += openpty.o
> +stub-obj-y += timer.o
> diff --git a/stubs/timer.c b/stubs/timer.c
> new file mode 100644
> index 000..17e25c9
> --- /dev/null
> +++ b/stubs/timer.c
> @@ -0,0 +1,44 @@
> +/*
> + * timer stub functions
> + *
> + * Copyright Red Hat, Inc., 2014
> + *
> + * Author: Fam Zheng 
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or
> + * later.  See the COPYING file in the top-level directory.
> + */
> +
> +#include "qemu/timer.h"
> +
> +void timer_del(QEMUTimer *ts)
> +{
> +abort();
> +}
> +
> +void timer_init(QEMUTimer *ts,
> +QEMUTimerList *timer_list, int scale,
> +QEMUTimerCB *cb, void *opaque)
> +{
> +abort();
> +}
> +
> +void timer_free(QEMUTimer *ts)
> +{
> +abort();
> +}
> +
> +int64_t qemu_clock_get_ns(QEMUClockType type)
> +{
> +abort();
> +}
> +
> +void timer_mod(QEMUTimer *ts, int64_t expire_time)
> +{
> +abort();
> +}
> +
> +bool timer_pending(QEMUTimer *ts)
> +{
> +abort();
> +}
> 

Who uses these?  The tests?

Perhaps util/throttle.c can be moved to the root and added to block-obj-y.

Paolo

Re: [Qemu-devel] [PATCH 5/6] build-sys: Change libqemuutil.a to qemuutil.o and link whole object

[Paolo Bonzini]

Il 20/08/2014 12:01, Fam Zheng ha scritto:
> +
> +qemuutil.o: CC_REL_FLAGS := -Wl,-r

Why the target-specific rule?

> +qemuutil.o: $(util-obj-y)
> + $(call quiet-command,$(CC) -nostdlib $(CC_REL_FLAGS) -o $@ $^,"  LD -r  
> $(TARGET_DIR)$@")
>  


I think either you have

LD_REL := $(CC) -nostdlib -Wl,-r

(as opposed to "LD_REL := $(LD) -r" for example) or you do not need to
put -Wl,-r in a separate variable.

Paolo

Re: [Qemu-devel] [PATCH 6/6] iscsi: Move iqn generation code to util

[Paolo Bonzini]

Il 20/08/2014 12:01, Fam Zheng ha scritto:
> Function qmp_query_uuid, even with a version in libqemustub.a, is not
> present in qemu-img, unless we move it to something that is linked with
> block-obj-y. Since it's a helper function, move it to util makes sense.
> 
> Signed-off-by: Fam Zheng 
> ---
>  block/iscsi.c | 15 +--
>  include/qemu-common.h |  3 +++
>  util/Makefile.objs|  1 +
>  util/iqn.c| 38 ++
>  4 files changed, 43 insertions(+), 14 deletions(-)
>  create mode 100644 util/iqn.c
> 
> diff --git a/block/iscsi.c b/block/iscsi.c
> index 2c9cfc1..b09f539 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -37,8 +37,6 @@
>  #include "trace.h"
>  #include "block/scsi.h"
>  #include "qemu/iov.h"
> -#include "sysemu/sysemu.h"
> -#include "qmp-commands.h"
>  
>  #include 
>  #include 
> @@ -1034,8 +1032,6 @@ static char *parse_initiator_name(const char *target)
>  QemuOptsList *list;
>  QemuOpts *opts;
>  const char *name;
> -char *iscsi_name;
> -UuidInfo *uuid_info;
>  
>  list = qemu_find_opts("iscsi");
>  if (list) {
> @@ -1051,16 +1047,7 @@ static char *parse_initiator_name(const char *target)
>  }
>  }
>  
> -uuid_info = qmp_query_uuid(NULL);
> -if (strcmp(uuid_info->UUID, UUID_NONE) == 0) {
> -name = qemu_get_vm_name();
> -} else {
> -name = uuid_info->UUID;
> -}
> -iscsi_name = g_strdup_printf("iqn.2008-11.org.linux-kvm%s%s",
> - name ? ":" : "", name ? name : "");
> -qapi_free_UuidInfo(uuid_info);
> -return iscsi_name;
> +return iqn_generate("iqn.2008-11.org.linux-kvm");
>  }
>  
>  static void iscsi_nop_timed_event(void *opaque)
> diff --git a/include/qemu-common.h b/include/qemu-common.h
> index bcf7a6a..ba7700f 100644
> --- a/include/qemu-common.h
> +++ b/include/qemu-common.h
> @@ -420,6 +420,9 @@ int uleb128_decode_small(const uint8_t *in, uint32_t *n);
>  /* unicode.c */
>  int mod_utf8_codepoint(const char *s, size_t n, char **end);
>  
> +/* iqn.c */
> +char *iqn_generate(const char *prefix);
> +
>  /*
>   * Hexdump a buffer to a file. An optional string prefix is added to every 
> line
>   */
> diff --git a/util/Makefile.objs b/util/Makefile.objs
> index 65a36f6..5c043e9 100644
> --- a/util/Makefile.objs
> +++ b/util/Makefile.objs
> @@ -14,3 +14,4 @@ util-obj-y += throttle.o
>  util-obj-y += getauxval.o
>  util-obj-y += readline.o
>  util-obj-y += rfifolock.o
> +util-obj-y += iqn.o
> diff --git a/util/iqn.c b/util/iqn.c
> new file mode 100644
> index 000..c8d1eda
> --- /dev/null
> +++ b/util/iqn.c
> @@ -0,0 +1,38 @@
> +/*
> + * iqn generat function
> + *
> + * Copyright Red Hat, Inc., 2014
> + *
> + * Author: Paolo Bonzini 

Why me? :)

> + * Fam Zheng 
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or
> + * later.  See the COPYING file in the top-level directory.
> + */
> +
> +#include 
> +#include "qemu/error-report.h"
> +#include "qemu-common.h"
> +#include "sysemu/sysemu.h"
> +#include "qmp-commands.h"
> +
> +char *iqn_generate(const char *prefix)
> +{
> +const char *name;
> +char *iqn;
> +UuidInfo *uuid_info;
> +
> +uuid_info = qmp_query_uuid(NULL);
> +if (strcmp(uuid_info->UUID, UUID_NONE) == 0) {
> +name = qemu_get_vm_name();
> +} else {
> +name = uuid_info->UUID;
> +}
> +iqn = g_strdup_printf("%s%s%s",
> + prefix,
> + name ? ":" : "",
> + name ? : "");
> +qapi_free_UuidInfo(uuid_info);
> +
> +return iqn;
> +}
> 

Ouch, this is ugly... I cannot think of any other way to do it, but
perhaps Benoit's QMP-in-tools series could help here?

In any case, if we need it I think there's no need to keep the argument
to iqn_generate.

Paolo

Re: [Qemu-devel] [PATCH 1/6] build-sys: Move fifio8 to hw/

[Paolo Bonzini]

Typo in the subject, and please set up git to do rename detection
correctly.  I still could review the patch by checking the hashes in the
"index" lines, so:

Reviewed-by: Paolo Bonzini 

Il 20/08/2014 12:01, Fam Zheng ha scritto:
> Since it has a dependency on vmstate and is only used by device
> emulation, moving out from util will make the archive more independent.
> 
> Signed-off-by: Fam Zheng 
> ---
>  hw/Makefile.objs   |   1 +
>  hw/fifo8.c | 125 
> +
>  util/Makefile.objs |   1 -
>  util/fifo8.c   | 125 
> -
>  4 files changed, 126 insertions(+), 126 deletions(-)
>  create mode 100644 hw/fifo8.c
>  delete mode 100644 util/fifo8.c
> 
> diff --git a/hw/Makefile.objs b/hw/Makefile.objs
> index 52a1464..bc77dd3 100644
> --- a/hw/Makefile.objs
> +++ b/hw/Makefile.objs
> @@ -33,3 +33,4 @@ devices-dirs-$(CONFIG_MEM_HOTPLUG) += mem/
>  devices-dirs-y += core/
>  common-obj-y += $(devices-dirs-y)
>  obj-y += $(devices-dirs-y)
> +obj-y += fifo8.o
> diff --git a/hw/fifo8.c b/hw/fifo8.c
> new file mode 100644
> index 000..0ea5ad9
> --- /dev/null
> +++ b/hw/fifo8.c
> @@ -0,0 +1,125 @@
> +/*
> + * Generic FIFO component, implemented as a circular buffer.
> + *
> + * Copyright (c) 2012 Peter A. G. Crosthwaite
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version
> + * 2 of the License, or (at your option) any later version.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see .
> + */
> +
> +#include "qemu-common.h"
> +#include "qemu/fifo8.h"
> +
> +void fifo8_create(Fifo8 *fifo, uint32_t capacity)
> +{
> +fifo->data = g_new(uint8_t, capacity);
> +fifo->capacity = capacity;
> +fifo->head = 0;
> +fifo->num = 0;
> +}
> +
> +void fifo8_destroy(Fifo8 *fifo)
> +{
> +g_free(fifo->data);
> +}
> +
> +void fifo8_push(Fifo8 *fifo, uint8_t data)
> +{
> +if (fifo->num == fifo->capacity) {
> +abort();
> +}
> +fifo->data[(fifo->head + fifo->num) % fifo->capacity] = data;
> +fifo->num++;
> +}
> +
> +void fifo8_push_all(Fifo8 *fifo, const uint8_t *data, uint32_t num)
> +{
> +uint32_t start, avail;
> +
> +if (fifo->num + num > fifo->capacity) {
> +abort();
> +}
> +
> +start = (fifo->head + fifo->num) % fifo->capacity;
> +
> +if (start + num <= fifo->capacity) {
> +memcpy(&fifo->data[start], data, num);
> +} else {
> +avail = fifo->capacity - start;
> +memcpy(&fifo->data[start], data, avail);
> +memcpy(&fifo->data[0], &data[avail], num - avail);
> +}
> +
> +fifo->num += num;
> +}
> +
> +uint8_t fifo8_pop(Fifo8 *fifo)
> +{
> +uint8_t ret;
> +
> +if (fifo->num == 0) {
> +abort();
> +}
> +ret = fifo->data[fifo->head++];
> +fifo->head %= fifo->capacity;
> +fifo->num--;
> +return ret;
> +}
> +
> +const uint8_t *fifo8_pop_buf(Fifo8 *fifo, uint32_t max, uint32_t *num)
> +{
> +uint8_t *ret;
> +
> +if (max == 0 || max > fifo->num) {
> +abort();
> +}
> +*num = MIN(fifo->capacity - fifo->head, max);
> +ret = &fifo->data[fifo->head];
> +fifo->head += *num;
> +fifo->head %= fifo->capacity;
> +fifo->num -= *num;
> +return ret;
> +}
> +
> +void fifo8_reset(Fifo8 *fifo)
> +{
> +fifo->num = 0;
> +fifo->head = 0;
> +}
> +
> +bool fifo8_is_empty(Fifo8 *fifo)
> +{
> +return (fifo->num == 0);
> +}
> +
> +bool fifo8_is_full(Fifo8 *fifo)
> +{
> +return (fifo->num == fifo->capacity);
> +}
> +
> +uint32_t fifo8_num_free(Fifo8 *fifo)
> +{
> +return fifo->capacity - fifo->num;
> +}
> +
> +uint32_t fifo8_num_used(Fifo8 *fifo)
> +{
> +return fifo->num;
> +}
> +
> +const VMStateDescription vmstate_fifo8 = {
> +.name = "Fifo8",
> +.version_id = 1,
> +.minimum_version_id = 1,
> +.fields = (VMStateField[]) {
> +VMSTATE_VBUFFER_UINT32(data, Fifo8, 1, NULL, 0, capacity),
> +VMSTATE_UINT32(head, Fifo8),
> +VMSTATE_UINT32(num, Fifo8),
> +VMSTATE_END_OF_LIST()
> +}
> +};
> diff --git a/util/Makefile.objs b/util/Makefile.objs
> index 6b3c83b..65a36f6 100644
> --- a/util/Makefile.objs
> +++ b/util/Makefile.objs
> @@ -3,7 +3,6 @@ util-obj-$(CONFIG_WIN32) += oslib-win32.o qemu-thread-win32.o 
> event_notifier-win
>  util-obj-$(CONFIG_POSIX) += oslib-posix.o qemu-thread-posix.o 
> event_notifier-posix.o qemu-openpty.o
>  util-obj-y += envlist.o path.o host-utils.o module.o
>  util-obj-y += bitmap.o bitops.o hbitmap.o
> -util-obj-y += fifo8.o
>  util-obj-y += acl.o
>  util-obj-y += error.o qemu-error.o
>  util-obj-$(CONFIG_POSIX) += compatfd.o
> diff --git a/util/fifo8.c b/util/fifo8.c
> deleted file mode 100644
> index 0ea5ad9..000
> --- 

Re: [Qemu-devel] [PATCH v5 00/15] TriCore architecture guest implementation

[Bastian Koppelmann]

Hi Andreas,

On 08/20/2014 02:17 PM, Andreas Färber wrote:

Hi,

Am 13.08.2014 14:07, schrieb Bastian Koppelmann:

  arch_init.c |2 +
  configure   |5 +
  cpu-exec.c  |   11 +-
  cpus.c  |6 +
  default-configs/tricore-softmmu.mak |3 +
  hw/tricore/Makefile.objs|1 +
  hw/tricore/tricore_testboard.c  |  129 
  include/elf.h   |2 +
  include/hw/tricore/tricore.h|   54 ++
  include/sysemu/arch_init.h  |1 +
  target-tricore/Makefile.objs|1 +
  target-tricore/cpu-qom.h|   71 ++
  target-tricore/cpu.c|  191 +
  target-tricore/cpu.h|  400 ++
  target-tricore/helper.c |  144 
  target-tricore/helper.h |   25 +
  target-tricore/op_helper.c  |  392 ++
  target-tricore/translate.c  | 1259 +++
  target-tricore/tricore-defs.h   |   28 +
  target-tricore/tricore-opcodes.h| 1406 +++
  20 files changed, 4130 insertions(+), 1 deletion(-)
  create mode 100644 default-configs/tricore-softmmu.mak
  create mode 100644 hw/tricore/Makefile.objs
  create mode 100644 hw/tricore/tricore_testboard.c
  create mode 100644 include/hw/tricore/tricore.h
  create mode 100644 target-tricore/Makefile.objs
  create mode 100644 target-tricore/cpu-qom.h
  create mode 100644 target-tricore/cpu.c
  create mode 100644 target-tricore/cpu.h
  create mode 100644 target-tricore/helper.c
  create mode 100644 target-tricore/helper.h
  create mode 100644 target-tricore/op_helper.c
  create mode 100644 target-tricore/translate.c
  create mode 100644 target-tricore/tricore-defs.h
  create mode 100644 target-tricore/tricore-opcodes.h

I still haven't really looked at the CPU QOM parts yet (and won't get to
that shortly, no blocker), in this diffstat I am missing a MAINTAINERS
entry that names and CCs someone (you?) to take care of all those new
target-tricore/, hw/tricore/ and hw/include/tricore/ files in the future.

I can definitly do that.


Have you tested your new machine using "make check V=1"? Just wondering
whether qom-test passes. (Sorry if I already asked earlier.)
I just tried that now and the test fails, because I have a whitespace in 
the machine name. I'll fix that as soon as I get review on the 
translation patches.


Regards,
Andreas


Thanks,

Bastian

[Qemu-devel] [PATCH] vmxnet3: Pad short frames to minimum size (60 bytes)

[Ben Draper]

When running VMware ESXi under qemu-kvm the guest discards frames
that are too short. Short ARP Requests will be dropped, this prevents
guests on the same bridge as VMware ESXi from communicating. This patch
simply adds the padding on the network device itself.

Signed-off-by: Ben Draper 
---
 hw/net/vmxnet3.c |   10 ++
 1 file changed, 10 insertions(+)

diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
index 791321f..f246fa1 100644
--- a/hw/net/vmxnet3.c
+++ b/hw/net/vmxnet3.c
@@ -34,6 +34,7 @@
 
 #define PCI_DEVICE_ID_VMWARE_VMXNET3_REVISION 0x1
 #define VMXNET3_MSIX_BAR_SIZE 0x2000
+#define MIN_BUF_SIZE 60
 
 #define VMXNET3_BAR0_IDX  (0)
 #define VMXNET3_BAR1_IDX  (1)
@@ -1871,12 +1872,21 @@ vmxnet3_receive(NetClientState *nc, const uint8_t *buf, 
size_t size)
 {
 VMXNET3State *s = qemu_get_nic_opaque(nc);
 size_t bytes_indicated;
+uint8_t min_buf[MIN_BUF_SIZE];
 
 if (!vmxnet3_can_receive(nc)) {
 VMW_PKPRN("Cannot receive now");
 return -1;
 }
 
+/* Pad to minimum Ethernet frame length */
+if (size < sizeof(min_buf)) {
+memcpy(min_buf, buf, size);
+memset(&min_buf[size], 0, sizeof(min_buf) - size);
+buf = min_buf;
+size = sizeof(min_buf);
+}
+
 if (s->peer_has_vhdr) {
 vmxnet_rx_pkt_set_vhdr(s->rx_pkt, (struct virtio_net_hdr *)buf);
 buf += sizeof(struct virtio_net_hdr);
-- 
1.7.10.4

[Qemu-devel] qemu-kvm process soft lockup cpu, results in server crush

[yue]

hi, i encounter several times   my server crushs  because qemu-kvm process 
locks cpu.
 
thanks
--log---
Aug 18 22:07:05 localhost kernel: [4625821.185649] nf_conntrack: table full, 
dropping packet.
Aug 18 22:07:05 localhost kernel: [4625821.192085] nf_conntrack: table full, 
dropping packet.
Aug 18 22:07:05 localhost kernel: [4625821.198608] nf_conntrack: table full, 
dropping packet.
Aug 18 22:07:05 localhost kernel: [4625821.205021] nf_conntrack: table full, 
dropping packet.
Aug 18 22:07:05 localhost kernel: [4625821.211432] nf_conntrack: table full, 
dropping packet.
Aug 18 22:07:05 localhost kernel: [4625821.217874] nf_conntrack: table full, 
dropping packet.
Aug 18 22:07:05 localhost kernel: [4625821.224301] nf_conntrack: table full, 
dropping packet.
Aug 18 22:07:05 localhost kernel: [4625821.230764] nf_conntrack: table full, 
dropping packet.
Aug 18 22:07:05 localhost kernel: [4625821.237219] nf_conntrack: table full, 
dropping packet.
Aug 18 22:07:05 localhost kernel: [4625821.243664] nf_conntrack: table full, 
dropping packet.
Aug 18 22:07:05 localhost ./gbalancer-0.5.1[19991]: dial tcp 10.200.86.30:3306: 
i/o timeout
Aug 18 22:07:06 localhost ./gbalancer-0.5.1[19991]: dial tcp 10.200.86.31:3306: 
i/o timeout
Aug 18 22:07:06 localhost ./gbalancer-0.5.1[19991]: wrangler: detected server 
10.200.86.32:3306 is down
Aug 18 22:07:06 localhost ./gbalancer-0.5.1[19991]: wrangler: detected server 
10.200.86.31:3306 is down
Aug 18 22:07:06 localhost ./gbalancer-0.5.1[19991]: wrangler: detected server 
10.200.86.30:3306 is down
Aug 18 22:07:07 localhost kernel: [4625822.875756] [ cut here 
]
Aug 18 22:07:07 localhost kernel: [4625822.881685] WARNING: at 
net/sched/sch_generic.c:261 dev_watchdog+0x26b/0x280() (Not tainted)
Aug 18 22:07:07 localhost kernel: [4625822.892399] Hardware name: IBM System 
x3550 M4: -[7914ON9]-
Aug 18 22:07:07 localhost kernel: [4625822.899346] NETDEV WATCHDOG: eth0 (igb): 
transmit queue 0 timed out
Aug 18 22:07:07 localhost kernel: [4625822.907052] Modules linked in: 
iptable_filter iptable_mangle iptable_nat nf_nat nf_conntrack_ipv4 nf_conntrack 
nf_defrag_ipv4 ip_tables tcp_diag inet_diag netconsole xt_CHECKSUM configfs 
ip6table_filter ip6_tables ebtable_nat ebtables cpufreq_ondemand acpi_cpufreq 
freq_table mperf ipmi_watchdog ipmi_poweroff ipmi_devintf bridge bonding 8021q 
garp stp llc ipv6 vhost_net macvtap macvlan tun kvm_intel kvm cdc_ether usbnet 
mii microcode iTCO_wdt iTCO_vendor_support igb dca i2c_algo_bit ptp pps_core sg 
ics932s401 i2c_i801 i2c_core lpc_ich mfd_core shpchp ext4 jbd2 mbcache sd_mod 
crc_t10dif megaraid_sas wmi dm_mirror dm_region_hash dm_log dm_mod [last 
unloaded: nf_conntrack]
Aug 18 22:07:07 localhost kernel: [4625822.979601] Pid: 0, comm: swapper Not 
tainted 2.6.32-431.11.2.el6.x86_64 #1
Aug 18 22:07:07 localhost kernel: [4625822.988093] Call Trace:
Aug 18 22:07:07 localhost kernel: [4625822.991527][] 
? warn_slowpath_common+0x87/0xc0
Aug 18 22:07:07 localhost kernel: [4625822.999861]  [] ? 
warn_slowpath_fmt+0x46/0x50
Aug 18 22:07:07 localhost kernel: [4625823.007193]  [] ? 
dev_watchdog+0x26b/0x280
Aug 18 22:07:07 localhost kernel: [4625823.014239]  [] ? 
scheduler_tick+0xcc/0x260
Aug 18 22:07:07 localhost kernel: [4625823.021369]  [] ? 
dev_watchdog+0x0/0x280
Aug 18 22:07:07 localhost kernel: [4625823.028204]  [] ? 
run_timer_softirq+0x197/0x340
Aug 18 22:07:07 localhost kernel: [4625823.035716]  [] ? 
tick_dev_program_event+0x65/0xc0
Aug 18 22:07:07 localhost kernel: [4625823.043526]  [] ? 
__do_softirq+0xc1/0x1e0
Aug 18 22:07:07 localhost kernel: [4625823.050447]  [] ? 
tick_program_event+0x2a/0x30
Aug 18 22:07:07 localhost kernel: [4625823.057877]  [] ? 
call_softirq+0x1c/0x30
Aug 18 22:07:07 localhost kernel: [4625823.064744]  [] ? 
do_softirq+0x65/0xa0
Aug 18 22:07:07 localhost kernel: [4625823.071385]  [] ? 
irq_exit+0x85/0x90
Aug 18 22:07:07 localhost kernel: [4625823.077831]  [] ? 
smp_apic_timer_interrupt+0x4a/0x60
Aug 18 22:07:07 localhost kernel: [4625823.085832]  [] ? 
apic_timer_interrupt+0x13/0x20
Aug 18 22:07:07 localhost kernel: [4625823.093612][] 
? intel_idle+0xde/0x170
Aug 18 22:07:07 localhost kernel: [4625823.101047]  [] ? 
intel_idle+0xc1/0x170
Aug 18 22:07:07 localhost kernel: [4625823.107876]  [] ? 
cpuidle_idle_call+0xa7/0x140
Aug 18 22:07:07 localhost kernel: [4625823.115287]  [] ? 
cpu_idle+0xb6/0x110
Aug 18 22:07:07 localhost kernel: [4625823.121822]  [] ? 
start_secondary+0x2ac/0x2ef
Aug 18 22:07:07 localhost kernel: [4625823.129159] ---[ end trace 
3e53505863a74c35 ]---
Aug 18 22:07:07 localhost kernel: [4625823.135043] igb :06:00.0: eth0: 
Reset adapter
Aug 18 22:07:10 localhost kernel: [4625826.197602] __ratelimit: 15954 callbacks 
suppressed
Aug 18 22:07:10 localhost kernel: [4625826.203755] nf_conntrack: table full, 
dropping packet.
Aug 18 22:07:10 localhost kernel: [4625826.209776] nf_conntrack: table full, 
dropping packet.
Aug 18 22:07:10 localhost 

Re: [Qemu-devel] QEMU, self-modifying code, and Windows 7 64-bit (no KVM)

[Hulin, Patrick - 0559 - MITLL]

On Aug 19, 2014, at 2:16 AM, Paolo Bonzini  wrote:

> Il 18/08/2014 22:50, Hulin, Patrick - 0559 - MITLL ha scritto:
 Correct. Doesn¹t work. Haven¹t fully diagnosed why, but it doesn¹t seem
 to ever hit the current_tb_modified passage if you invalidate beforehand.
>> Yeah - mem_io_pc doesn¹t get updated until we¹re inside io_write, so
>> tb_invalidate_phys_page_range thinks we¹re inside a different TB. As a
>> result, it¹s ³is this TB modified² check still returns false.
> 
> We can set that (and probably mem_io_vaddr) before the for loop, too.
> 
> Paolo

Strange - keeping the loop reversed and calling tb_invalidate_phys_range, with 
mem_io_pc set, works on the test case but fails on real Windows 7. It must be 
breaking something else.

I think making the loop forwards again makes the most sense. I’m not sure if 
the bug from 2007 will even still occur. That was caused by QEMU’s longjmp’ing 
only after the page boundary, similar to how it now only longjmp’s after the TB 
boundary. QEMU currently decides whether to longjmp by checking whether any of 
the TBs that intersect the page (i.e. p->first_tb etc.) contain the write 
address, and every code page should be TLB-protected. So every unaligned (and 
therefore out-of-line) write that touches already-translated code should get 
caught. As far as I can tell, the only problem is when a write splits into 
partially touching translated and untranslated code.

Can you guys explain why calling tlb_fill is necessary on the page boundary 
case?

[Qemu-devel] Commit d8d95814609e89e5438a3318a647ec322fc4ff16 seems to break ioh3420

[Knut Omang]

I am struggling with the latest master and 

-device ioh3420,slot=0,id=pcie_port.0

as argument. I bisected it down to:

d8d95814609e89e5438a3318a647ec322fc4ff16 is the first bad commit
commit d8d95814609e89e5438a3318a647ec322fc4ff16
Author: Paolo Bonzini 
Date:   Wed Jun 11 12:42:01 2014 +0200

memory: convert memory_region_destroy to object_unparent

Explicitly call object_unparent in the few places where we
will re-create the memory region.  If the memory region is
simply being destroyed as part of device teardown, let QOM
handle it.

Signed-off-by: Paolo Bonzini 

:04 04 b38d10817e8297b9c1ef8827b3429362e47578f1 
554905847501ec8e22669a6225cd979e42866905 M  docs
:04 04 a1a340ae1271b019975a5007673fef1490981a7b 
57c10cdc7bb0ee0535fdde4c80edaef174270c62 M  hw
:100644 100644 dce94a313044e35bdd841a9499c2f218f848ee72 
783a3ae6756ff4375267555607f46606a055b628 M  ioport.c
:100644 100644 64d7176193d829d9289c466bf4bd82b560b0c40a 
23c5269b8ef077e3c2114df2cbd79ff9e206bf98 M  memory.c


With this commit, qemu crashes at boot if I supply the ioh3420 argument,
without it, it seems to work - I am not immediately able to pinpoint
where the problem is,...

Knut

Re: [Qemu-devel] [PATCH v5 01/15] target-tricore: Add target stubs and qom-cpu

[Richard Henderson]

On 08/13/2014 05:07 AM, Bastian Koppelmann wrote:
> Add TriCore target stubs, and QOM cpu.
> 
> Signed-off-by: Bastian Koppelmann 
> ---
> v4 -> v5:
> - Change documentation of S, SV bits to use bit 31.
> - psw_read/_write now uses only bit 31 of S, SV bits.

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v5 03/15] target-tricore: Add softmmu support

[Richard Henderson]

On 08/13/2014 05:07 AM, Bastian Koppelmann wrote:
> +*prot = PAGE_READ | PAGE_WRITE;

No execute permission?


r~

Re: [Qemu-devel] [PATCH 1/6] build-sys: Move fifio8 to hw/

[Peter Crosthwaite]

On Wed, Aug 20, 2014 at 8:01 PM, Fam Zheng  wrote:
> Since it has a dependency on vmstate and is only used by device
> emulation, moving out from util will make the archive more independent.
>
> Signed-off-by: Fam Zheng 
> ---
>  hw/Makefile.objs   |   1 +
>  hw/fifo8.c | 125 
> +

Can we get it into a hw subdirectory? Either core or misc come to mind.

Regards,
Peter

Re: [Qemu-devel] Commit d8d95814609e89e5438a3318a647ec322fc4ff16 seems to break ioh3420

[Knut Omang]

Seems very similar to what Gonglei just reported...

Knut

On Wed, 2014-08-20 at 16:10 +0200, Knut Omang wrote:
> I am struggling with the latest master and 
> 
> -device ioh3420,slot=0,id=pcie_port.0
> 
> as argument. I bisected it down to:
> 
> d8d95814609e89e5438a3318a647ec322fc4ff16 is the first bad commit
> commit d8d95814609e89e5438a3318a647ec322fc4ff16
> Author: Paolo Bonzini 
> Date:   Wed Jun 11 12:42:01 2014 +0200
> 
> memory: convert memory_region_destroy to object_unparent
> 
> Explicitly call object_unparent in the few places where we
> will re-create the memory region.  If the memory region is
> simply being destroyed as part of device teardown, let QOM
> handle it.
> 
> Signed-off-by: Paolo Bonzini 
> 
> :04 04 b38d10817e8297b9c1ef8827b3429362e47578f1 
> 554905847501ec8e22669a6225cd979e42866905 M  docs
> :04 04 a1a340ae1271b019975a5007673fef1490981a7b 
> 57c10cdc7bb0ee0535fdde4c80edaef174270c62 M  hw
> :100644 100644 dce94a313044e35bdd841a9499c2f218f848ee72 
> 783a3ae6756ff4375267555607f46606a055b628 M  ioport.c
> :100644 100644 64d7176193d829d9289c466bf4bd82b560b0c40a 
> 23c5269b8ef077e3c2114df2cbd79ff9e206bf98 M  memory.c
> 
> 
> With this commit, qemu crashes at boot if I supply the ioh3420 argument,
> without it, it seems to work - I am not immediately able to pinpoint
> where the problem is,...
> 
> Knut