[Qemu-devel] memory stats collect on Windows virtio-driver

2016-02-11 Thread Jean-Pierre Ribeauville
Hi,

By validating memory stats collect via this libvirt call :

virsh dommemstat win2k8r2-2 --period 20

I'm not able to get others tags than theses ones :

#virsh dommemstat win2k8r2-2
actual 2097152
rss 4729140


For a Linux Guest , it works:

# virsh dommemstat rhel6.1
actual 8192000
swap_in 36269076
swap_out 12422088
major_fault 1781322
minor_fault 9997036
unused 1705388
available 7982708
rss 7906532


Did I miss something ?

Is  there any plan to add these tags collect on Windows side ?



Thx for help.

Regards,


J.P. Ribeauville


P: +33.(0).1.47.17.20.49
.
Puteaux 3 Etage 5  Bureau 4

jpribeauvi...@axway.com
http://www.axway.com



P Pensez à l'environnement avant d'imprimer.





Re: [Qemu-devel] lock-free monitor?

2016-02-11 Thread Markus Armbruster
"Dr. David Alan Gilbert"  writes:

> * Markus Armbruster (arm...@redhat.com) wrote:
>> "Dr. David Alan Gilbert"  writes:
>> 
>> > * Markus Armbruster (arm...@redhat.com) wrote:
>> >> "Dr. David Alan Gilbert"  writes:
>> >> 
>> >> > Hi,
>> >> >   I wondered what it would take to be able to do a lock-free monitor;
>> >> > i.e. one that could respond to (some) commands while the qemu big lock 
>> >> > is held.
>> >> 
>> >> Requires a careful audit of the monitor code.
>> >> 
>> >> For instance, cur_mon needs to be made thread local for running multiple
>> >> monitors concurrently.
>> >
>> > Hmm that's fun;  and cur_mon is used all over the place when 'fd' passing
>> > is used - although that probably should be cleaned up somehow.
>> 
>> When cur_mon got created, we had no threads.  Even now, monitors only
>> ever run under the BQL, so cur_mon's still fine.
>> 
>> Having a current monitor is simpler than passing one around, especially
>> when you go through layers that don't know about it.  Such cases exist,
>> and they made us abandon commit 376253e's hope to eliminate cur_mon.
>> Unfortunately, I've since forgotten the details, so I can't point you to
>> an example.
>
> Yes,  I think maybe if we can try and remove the use of cur_mon one
> use at a time outside of the monitor it might move it in the right direction.

For historical reasons, some interfaces assume the current monitor,
while others take Monitor * parameters.  The typical cur_mon use outside
the monitor is such an argument.

Whether such a Monitor * interface actually works when passed something
other than cur_mon is anybody's guess.  Moving monitors out of the BQL
is unlikely to increase my confidence.

If a function can safely work on non-current monitors, a Monitor *
parameter is fine.  If not, it shouldn't take one, and just use cur_mon.

>> >> > My reason for asking is that there are cases in migration and colo
>> >> > where a network block device has failed and is blocking, but it fails
>> >> > at just the wrong time while the lock is held, and now, you don't have
>> >> 
>> >> Is it okay to block while holding the BQL?
>> >
>> > I'd hope the simple answer was no; unfortunately the more complex answer
>> > is that we keep finding places that do.
>> 
>> Would you call these bugs?
>> 
>> Even if you do, we may want to recover from them.
>
> They're a bug in the end result that needs fixing; so for example a 
> crashing NBD server shouldn't lock you out of the monitor during a migrate,
> and I don't think we current;y have other solutions.

It shouldn't lock you out of anything.  The monitor is just one such
thing.  It's special only because it can let the user (human or
management application) recover from certain bugs with monitor commands.

I'd see enabling that as a partial work-around for a class of bugs that
is hard to fix.  Work-around is better than nothing.

However, it can reduce the incentive to actually fix the bug.  No reason
to block work-arounds, but I'd like to see an earnest commitment to
actual bug fixing.

>> >  The cases I'm aware of are
>> > mostly bdrv_drain_all calls in migration/colo, where one of the block
>> > devices (e.g. an NBD network device) fails.  Generally these are called
>> > at the end of a migration cycle when we're just ensuring that everything
>> > really is drained and are normally called with the lock held to ensure
>> > that nothing else is generating new traffic as we're clearing everything 
>> > else.
>> 
>> Using the BQL for that drives a cork into the I/O pipeline with a Really
>> Big Hammer.  Can we do better?
>> 
>> The answer may be "yes, but don't hold your breath."  Then we may need
>> means to better cope with the bugs.
>
> Yeh there are some places that the migraiton code holds the BQL where
> I don't really understand all the things it's guarding against.

I guess actual bug fixing would start with figuring that out.

>> >> > any way to unblock it since you can't do anything on the monitors.
>> >> > If I could issue a command then I could have it end up doing a 
>> >> > shutdown(2)
>> >> > on the network connection and free things up.
>> >> >
>> >> > Maybe this would also help real-time people?
>> >> >
>> >> > I was thinking then, we could:
>> >> >a) Have a monitor that wasn't tied to the main loop at all - each 
>> >> > instance
>> >> > would be it's own thread and have it's own poll() (probably using the 
>> >> > new
>> >> > IO channels?)
>> >> >b) for most commands it would take the big lock before execute the 
>> >> > command
>> >> > and release it afterwards - so there would be no risk in those commands.
>> >> 
>> >> Saves you the auditing their code, which would probably be impractical.
>> >> 
>> >> >c) Some commands you could mark as 'lock free' - they would be 
>> >> > required
>> >> > not to take any locks or wait for *anything* and for those the monitor 
>> >> > 

Re: [Qemu-devel] [PATCH 1/3] qdev-monitor: sort alias table by typename

2016-02-11 Thread Markus Armbruster
Sascha Silbe  writes:

> Sort the alias table by typename so it's easier to see which aliases
> exist.
>
> Signed-off-by: Sascha Silbe 
> ---
>  qdev-monitor.c | 14 +++---
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/qdev-monitor.c b/qdev-monitor.c
> index 81e3ff3..0145deb 100644
> --- a/qdev-monitor.c
> +++ b/qdev-monitor.c
> @@ -40,18 +40,18 @@ typedef struct QDevAlias
>  } QDevAlias;
>  
>  static const QDevAlias qdev_alias_table[] = {

Suggest to add

   /* Please keep this table sorted */

> -{ "virtio-blk-pci", "virtio-blk", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
> -{ "virtio-net-pci", "virtio-net", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
> -{ "virtio-serial-pci", "virtio-serial", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X 
> },
> +{ "e1000", "e1000-82540em" },
> +{ "ich9-ahci", "ahci" },
> +{ "kvm-pci-assign", "pci-assign" },
> +{ "lsi53c895a", "lsi" },
>  { "virtio-balloon-pci", "virtio-balloon",
>  QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
>  { "virtio-blk-ccw", "virtio-blk", QEMU_ARCH_S390X },
> +{ "virtio-blk-pci", "virtio-blk", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
>  { "virtio-net-ccw", "virtio-net", QEMU_ARCH_S390X },
> +{ "virtio-net-pci", "virtio-net", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
>  { "virtio-serial-ccw", "virtio-serial", QEMU_ARCH_S390X },
> -{ "lsi53c895a", "lsi" },
> -{ "ich9-ahci", "ahci" },
> -{ "kvm-pci-assign", "pci-assign" },
> -{ "e1000", "e1000-82540em" },
> +{ "virtio-serial-pci", "virtio-serial", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X 
> },
>  { }
>  };

Reviewed-by: Markus Armbruster 



Re: [Qemu-devel] [PATCH 2/3] qdev-monitor: add missing aliases for virtio-{9p, balloon, rng, scsi}

2016-02-11 Thread Markus Armbruster
Sascha Silbe  writes:

> virtio-{blk,balloon,net,serial} are aliases for their actual,
> architecture-dependent implementations (*-ccw on s390x, *-pci on other
> architectures supporting virtio). This makes it a lot easier to craft
> qemu invocations that work on all supported architectures. Complete
> the set to cover all virtio devices that are implemented on all
> architectures supporting virtio.
>
> For virtio-balloon, only the CCW implementation was missing.
>
> Signed-off-by: Sascha Silbe 

Reviewed-by: Markus Armbruster 

> ---
>
> This leaves out
> virtio-{gpu,input,input-hid,input-host,keyboard,mouse,tablet} because
> they're currently only implemented using PCI, so there's no immediate
> value in having them. It would nevertheless make sense to include them
> so they can get used already and will start to Just Work™ on s390x
> once a CCW implementation appears. I can post the corresponding patch
> if there's any interest.

I guess that's for the virtio people to decide.  I'm cc'ing some.

> ---
>  qdev-monitor.c | 7 +++
>  1 file changed, 7 insertions(+)
>
> diff --git a/qdev-monitor.c b/qdev-monitor.c
> index 0145deb..9c4217c 100644
> --- a/qdev-monitor.c
> +++ b/qdev-monitor.c
> @@ -44,12 +44,19 @@ static const QDevAlias qdev_alias_table[] = {
>  { "ich9-ahci", "ahci" },
>  { "kvm-pci-assign", "pci-assign" },
>  { "lsi53c895a", "lsi" },
> +{ "virtio-9p-ccw", "virtio-9p", QEMU_ARCH_S390X },
> +{ "virtio-9p-pci", "virtio-9p", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
> +{ "virtio-balloon-ccw", "virtio-balloon", QEMU_ARCH_S390X },
>  { "virtio-balloon-pci", "virtio-balloon",
>  QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
>  { "virtio-blk-ccw", "virtio-blk", QEMU_ARCH_S390X },
>  { "virtio-blk-pci", "virtio-blk", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
>  { "virtio-net-ccw", "virtio-net", QEMU_ARCH_S390X },
>  { "virtio-net-pci", "virtio-net", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
> +{ "virtio-rng-ccw", "virtio-rng", QEMU_ARCH_S390X },
> +{ "virtio-rng-pci", "virtio-rng", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
> +{ "virtio-scsi-ccw", "virtio-scsi", QEMU_ARCH_S390X },
> +{ "virtio-scsi-pci", "virtio-scsi", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
>  { "virtio-serial-ccw", "virtio-serial", QEMU_ARCH_S390X },
>  { "virtio-serial-pci", "virtio-serial", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X 
> },
>  { }



Re: [Qemu-devel] [PATCH 0/2] checkpatch: Fixing two cases of false positives in checkpatch.pl

2016-02-11 Thread Paolo Bonzini


On 11/02/2016 10:43, Leonid Bloch wrote:
> ping
> 
> http://patchwork.ozlabs.org/patch/537763
> http://patchwork.ozlabs.org/patch/537762
> 
> On Sun, Jan 31, 2016 at 4:13 PM, Leonid Bloch  wrote:
>> ping
>>
>> http://patchwork.ozlabs.org/patch/537763
>> http://patchwork.ozlabs.org/patch/537762
>>
>> On Mon, Jan 11, 2016 at 2:12 PM, Markus Armbruster 
>> wrote:
>>>
>>> Copying Paolo.
>>>
>>> Leonid Bloch  writes:
>>>
 This series addresses two cases where errors were printed if whitespaces
 appeared in front of a square bracket in places where there should be no
 problem with such placements (please see messages of individual
 commits).

 Leonid Bloch (2):
   checkpatch: Eliminate false positive in case of comma-space-square
 bracket
   checkpatch: Eliminate false positive in case of space before square
 bracket in a definition

  scripts/checkpatch.pl | 6 +-
  1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>

Thanks, applied to my queue.

Paolo



Re: [Qemu-devel] [PATCH] memory: fix usage of find_next_bit and find_next_zero_bit

2016-02-11 Thread Paolo Bonzini


On 10/02/2016 23:40, Peter Maydell wrote:
> On 10 February 2016 at 14:11, Paolo Bonzini  wrote:
>> The last two arguments to these functions are the last and first bit to
>> check relative to the base.  The code was using incorrectly the first
>> bit and the number of bits.  Fix this in cpu_physical_memory_get_dirty
>> and cpu_physical_memory_all_dirty.  This requires a few changes in the
>> iteration; change the code in cpu_physical_memory_set_dirty_range to
>> match.
>>
>> Fixes: 5b82b70
>> Cc: Stefan Hajnoczi 
>> Signed-off-by: Paolo Bonzini 
> 
> I've set the pre-merge tests running so I should be able
> to commit this to master first thing tomorrow. Was this
> the only patch that needs applying to fix your pullreq?

Nothing else.

Paolo




Re: [Qemu-devel] [PATCH v3 00/10] Allow hotplug of s390 CPUs

2016-02-11 Thread David Hildenbrand
> Am 10.02.2016 um 16:28 schrieb David Hildenbrand:
> > For x86, cpu models are realized by making x86_64-cpu an abstract class and
> > creating loads of new classes, e.g. host-x86_64-cpu or haswell-x86_64-cpu.
> > 
> > How does 'device_add ' play together with the x86 cpu model
> > approach? And with cpu models specified via "-cpu" in general?  
> 
> device_add needs to use an instantiatable type, like the ones you sketch
> above.
> 
> > Or does that in return mean, that "making models own classes" is outdated? 
> > Or
> > will some internal conversion happen that I am missing?
> > 
> > What is the plan for cpu models and cpu hotplug? How are cpu models to be
> > defined in the future?  
> 
> Someone at IBM was working on defining models for s390x - not sure what
> the status is?

That one is me right now :) Michael Mueller was working on a version without
explicit features last year. I'm now looking into models with features that can
be turned on/off - like x86 has.

As I'm trying to get a view of the bigger picture I also have to take care of
cpu hotplug, and I am not quite sure yet if we (s390) really want or need a
device_add.

a) Specification of cpu model and cpu hotplug on QEMU start

-smp 2,maxcpus=6 -cpu zBC12,+feata,+featb,prop=value

Here, it is quite clear that all cpus will get the same feature set. We don't
need any information about internals (e.g. which class is used internally for
the cpu)

b) Adding cpus via the monitor "cpu_add"

cpu-add id=3

Quite easy, we get what we ordered when starting QEMU, a cpu just like the
others.

c) Adding a cpu via device_add

device_add s390-cpu,id=3
-> Not uniform. We _want_ cpu models as cpu subclasses
(http://wiki.qemu.org/Features/CPUHotplug)

OR

device_add zBC12-s390-cpu,id=3
-> Not uniform. We don't specify the properties. But we have to specify some
magic class that we didn't have to specify on the command line. Implicitly
used information for a device.

OR

device_add zBC12-s390-cpu,id=3,feata=on,featb=off,prop=value
-> Fully specified. Again. And we need information about the internally used
class. Implicitly used information for a device.

Especially the last two examples are bad:
1) We could hotplug _different_ cpus, which is absolutely not what we want on
s390.
2) In every sane setup, we have to respecify what we already setup up at QEMU
start. (and I don't see any benefit)
3) Interface that is much more complex and more error prone to use.


d) Benefits of the new interface.

Unfortunately I can't seem to find any
(http://wiki.qemu.org/Features/CPUHotplug) but what I can think of

1) Specify something like topology more detailed (IMHO not applicable for s390)
2) Do a device_del (IMHO not applicable for s390)


Both of these points could easily be realized by extending the existing cpu-add
and by introducing a cpu_del (if really needed).


I am not against this, I just want to understand what the plan is. Because this
highly overcomplicates matter for us (s390) and requires yet another interface
to be maintained (I have some quote about new interfaces in the back of my hand
from some Linus guy ;) )

"Targets are encouraged to (re)design CPU creation so that it would be possible
to use device_add/device-del interface for it. However if due to target
design or a necessary long re-factoring time to use CPU with
device_add/device-del interface, it is possible speed-up CPU hot-add feature
development by using cpu-add interface."
(http://wiki.qemu.org/Features/CPUHotplug)

If nobody can convince me that this is the way to go and everything I said is
already clear or wrong, then I'd vote for keeping it simple and using cpu-add.

David




Re: [Qemu-devel] [PATCH 2/3] qdev-monitor: add missing aliases for virtio-{9p, balloon, rng, scsi}

2016-02-11 Thread Cornelia Huck
On Thu, 11 Feb 2016 10:01:35 +0100
Markus Armbruster  wrote:

> Sascha Silbe  writes:

> > This leaves out
> > virtio-{gpu,input,input-hid,input-host,keyboard,mouse,tablet} because
> > they're currently only implemented using PCI, so there's no immediate
> > value in having them. It would nevertheless make sense to include them
> > so they can get used already and will start to Just Work™ on s390x
> > once a CCW implementation appears. I can post the corresponding patch
> > if there's any interest.
> 
> I guess that's for the virtio people to decide.  I'm cc'ing some.

What would the error look like if one decided to use e.g. virtio-gpu on
s390x? If the error is more specific (i.e. virtio-gpu-ccw does not
exist vs. virtio-gpu does not exist), I think adding the aliases makes
sense: the user sees what is actually missing.




Re: [Qemu-devel] [PATCH 3/3] replay: introduce block devices record/replay

2016-02-11 Thread Kevin Wolf
Am 11.02.2016 um 07:05 hat Pavel Dovgalyuk geschrieben:
> > From: Kevin Wolf [mailto:kw...@redhat.com]
> > Am 10.02.2016 um 13:51 hat Pavel Dovgalyuk geschrieben:
> > > However, I don't understand yet which layer do you offer as the candidate
> > > for record/replay? What functions should be changed?
> > > I would like to investigate this way, but I don't got it yet.
> > 
> > At the core, I wouldn't change any existing function, but introduce a
> > new block driver. You could copy raw_bsd.c for a start and then tweak
> > it. Leave out functions that you don't want to support, and add the
> > necessary magic to .bdrv_co_readv/writev.
> > 
> > Something like this (can probably be generalised for more than just
> > reads as the part after the bdrv_co_reads() call should be the same for
> > reads, writes and any other request types):
> > 
> > int blkreplay_co_readv()
> > {
> > BlockReplayState *s = bs->opaque;
> > int reqid = s->reqid++;
> > 
> > bdrv_co_readv(bs->file, ...);
> > 
> > if (mode == record) {
> > log(reqid, time);
> > } else {
> > assert(mode == replay);
> > bool *done = req_replayed_list_get(reqid)
> > if (done) {
> > *done = true;
> > } else {
> > req_completed_list_insert(reqid, qemu_coroutine_self());
> > qemu_coroutine_yield();
> > }
> > }
> > }
> > 
> > /* called by replay.c */
> > int blkreplay_run_event()
> > {
> > if (mode == replay) {
> > co = req_completed_list_get(e.reqid);
> > if (co) {
> > qemu_coroutine_enter(co);
> > } else {
> > bool done = false;
> > req_replayed_list_insert(reqid, );
> > /* wait synchronously for completion */
> > while (!done) {
> > aio_poll();
> > }
> > }
> > }
> > }
> > 
> > Where we could consider changing existing code is that it might be
> > desirable to automatically put an instance of this block driver on top
> > of every block device when record/replay is used. If we don't do that,
> > you need to explicitly specify -drive driver=blkreplay,...
> 
> As far, as I understand, all synchronous read/write request are also passed
> through this coroutines layer.

Yes, all read/write requests go through the same function internally, no
matter which external interface was used.

> It means that every disk access in replay phase should match the recording 
> phase.

Right. If I'm not mistaken, this was the fundamental requirement you
have, so I wouldn't have suggested this otherwise.

> Record/replay is intended to be used for debugging and analysis.
> When execution is replayed, guest machine cannot notice analysis overhead.
> Some analysis methods may include disk image reading. E.g., qemu-based
> analysis framework DECAF uses sleuthkit for disk forensics ( 
> https://github.com/sycurelab/DECAF ).
> If similar framework will be used with replay, forensics disk access 
> operations
> won't work if we will record/replay the coroutines.

Sorry, I'm not sure if I can follow.

If such analysis software runs in the guest, it's not a replay any more
and I completely fail to see what you're doing.

If it's a qemu component independent from the guest, then my method
gives you a clean way to bypass the replay driver that wouldn't be
possible with yours.

If your plan was to record/replay only async requests and then use sync
requests to bypass the record/replay, let me clearly state that this is
the wrong approach: There are still guest devices which do synchronous
I/O and need to be considered in the replay log, and you shouldn't
prevent the analysis code from using AIO (in fact, using sync I/O in new
code is very much frowned upon).

I can explain in more detail what the block device structure looks like
and how to access an image with and without record/replay, but first let
me please know whether I guessed right what your problem is. Or if I
missed your point, can you please describe in detail a case that
wouldn't work?

> And if we'll record only high-level asynchronous disk operations, then
> there will be a way to performs disk accesses without matching these
> operations with replay log.
> 
> However, we already tried to record disk operations completion events
> (in previous more complicated version). Recording overhead was the same
> as with new blk_aio_ version of the patch. 

Well, if your code really intentionally ignores synchronous requests,
then the advantage you get from my approach is being correct.

Other than that, I'm personally mainly interested in keeping the core
block layer easily maintainable and the hot I/O paths clean for cases
that don't use all the features qemu has. This means that in the mid
term existing features need to move out of the core block layer rather
than new ones being added. So doing things in a separate block driver
might not be an advantage for your immediate needs, but it's important
for 

Re: [Qemu-devel] [PATCH 0/2] checkpatch: Fixing two cases of false positives in checkpatch.pl

2016-02-11 Thread Leonid Bloch
ping

http://patchwork.ozlabs.org/patch/537763
http://patchwork.ozlabs.org/patch/537762

On Sun, Jan 31, 2016 at 4:13 PM, Leonid Bloch  wrote:
> ping
>
> http://patchwork.ozlabs.org/patch/537763
> http://patchwork.ozlabs.org/patch/537762
>
> On Mon, Jan 11, 2016 at 2:12 PM, Markus Armbruster 
> wrote:
>>
>> Copying Paolo.
>>
>> Leonid Bloch  writes:
>>
>> > This series addresses two cases where errors were printed if whitespaces
>> > appeared in front of a square bracket in places where there should be no
>> > problem with such placements (please see messages of individual
>> > commits).
>> >
>> > Leonid Bloch (2):
>> >   checkpatch: Eliminate false positive in case of comma-space-square
>> > bracket
>> >   checkpatch: Eliminate false positive in case of space before square
>> > bracket in a definition
>> >
>> >  scripts/checkpatch.pl | 6 +-
>> >  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>



Re: [Qemu-devel] [PATCH v19 3/9] pc: add a Virtual Machine Generation ID device

2016-02-11 Thread Michael S. Tsirkin
On Thu, Feb 11, 2016 at 07:34:52PM +0200, Marcel Apfelbaum wrote:
> On 02/11/2016 06:30 PM, Michael S. Tsirkin wrote:
> >On Thu, Feb 11, 2016 at 04:16:05PM +0100, Igor Mammedov wrote:
> >>On Tue, 9 Feb 2016 14:17:44 +0200
> >>"Michael S. Tsirkin"  wrote:
> >>
> >>>On Tue, Feb 09, 2016 at 11:46:08AM +0100, Igor Mammedov wrote:
> >So the linker interface solves this rather neatly:
> >bios allocates memory, bios passes memory map to guest.
> >Served us well for several years without need for extensions,
> >and it does solve the VM GEN ID problem, even though
> >1. it was never designed for huge areas like nvdimm seems to want to use
> >2. we might want to add a new 64 bit flag to avoid touching low memory
> linker interface is fine for some readonly data, like ACPI tables
> especially fixed tables not so for AML ones is one wants to patch it.
> 
> However now when you want to use it for other purposes you start
> adding extensions and other guest->QEMU channels to communicate
> patching info back.
> It steals guest's memory which is also not nice and doesn't scale well.
> >>>
> >>>This is an argument I don't get. memory is memory. call it guest memory
> >>>or RAM backed PCI BAR - same thing. MMIO is cheaper of course
> >>>but much slower.
> >>>
> >>>...
> >>It however matters for user, he pays for guest with XXX RAM but gets less
> >>than that. And that will be getting worse as a number of such devices
> >>increases.
> >>
> >OK fine, but returning PCI BAR address to guest is wrong.
> >How about reading it from ACPI then? Is it really
> >broken unless there's *also* a driver?
> I don't get question, MS Spec requires address (ADDR method),
> and it's read by ACPI (AML).
> >>>
> >>>You were unhappy about DMA into guest memory.
> >>>As a replacement for DMA, we could have AML read from
> >>>e.g. PCI and write into RAM.
> >>>This way we don't need to pass address to QEMU.
> >>That sounds better as it saves us from allocation of IO port
> >>and QEMU don't need to write into guest memory, the only question is
> >>if PCI_Config opregion would work with driver-less PCI device.
> >
> >Or PCI BAR for that reason. I don't know for sure.
> >
> >>
> >>And it's still pretty much not test-able since it would require
> >>fully running OSPM to execute AML side.
> >
> >AML is not testable, but that's nothing new.
> >You can test reading from PCI.
> >
> >>>
> As for working PCI_Config OpRegion without driver, I haven't tried,
> but I wouldn't be surprised if it doesn't, taking in account that
> MS introduced _DSM doesn't.
> 
> >
> >
> Just compare with a graphics card design, where on device memory
> is mapped directly at some GPA not wasting RAM that guest could
> use for other tasks.
> >>>
> >>>This might have been true 20 years ago.  Most modern cards do DMA.
> >>
> >>Modern cards, with it's own RAM, map its VRAM in address space directly
> >>and allow users use it (GEM API). So they do not waste conventional RAM.
> >>For example NVIDIA VRAM is mapped as PCI BARs the same way like in this
> >>series (even PCI class id is the same)
> >
> >Don't know enough about graphics really, I'm not sure how these are
> >relevant.  NICs and disks certainly do DMA.  And virtio gl seems to
> >mostly use guest RAM, not on card RAM.
> >
> VMGENID and NVDIMM use-cases look to me exactly the same, i.e.
> instead of consuming guest's RAM they should be mapped at
> some GPA and their memory accessed directly.
> >>>
> >>>VMGENID is tied to a spec that rather arbitrarily asks for a fixed
> >>>address. This breaks the straight-forward approach of using a
> >>>rebalanceable PCI BAR.
> >>
> >>For PCI rebalance to work on Windows, one has to provide working PCI 
> >>driver
> >>otherwise OS will ignore it when rebalancing happens and
> >>might map something else over ignored BAR.
> >
> >Does it disable the BAR then? Or just move it elsewhere?
> it doesn't, it just blindly ignores BARs existence and maps BAR of
> another device with driver over it.
> >>>
> >>>Interesting. On classical PCI this is a forbidden configuration.
> >>>Maybe we do something that confuses windows?
> >>>Could you tell me how to reproduce this behaviour?
> >>#cat > t << EOF
> >>pci_update_mappings_del
> >>pci_update_mappings_add
> >>EOF
> >>
> >>#./x86_64-softmmu/qemu-system-x86_64 -snapshot -enable-kvm -snapshot \
> >>  -monitor unix:/tmp/m,server,nowait -device pci-bridge,chassis_nr=1 \
> >>  -boot menu=on -m 4G -trace events=t ws2012r2x64dc.img \
> >>  -device ivshmem,id=foo,size=2M,shm,bus=pci.1,addr=01
> >>
> >>wait till OS boots, note BARs programmed for ivshmem
> >>  in my case it was
> >>01:01.0 0,0xfe80+0x100
> >>then execute script and watch pci_update_mappings* trace events
> >>
> 

Re: [Qemu-devel] [Qemu-ppc] [PATCH] spapr: skip configuration section during migration of older machines

2016-02-11 Thread David Gibson
On Thu, Feb 11, 2016 at 04:53:40PM +, Dr. David Alan Gilbert wrote:
> * Greg Kurz (gk...@linux.vnet.ibm.com) wrote:
> > On Mon, 08 Feb 2016 16:59:47 +0100
> > Greg Kurz  wrote:
> > > Since QEMU 2.4, we have a configuration section in the migration stream.
> > > This must be skipped for older machines, like it is already done for x86.
> > > 
> > 
> > Ouch ! It is more complex than I thought... the migration of pseries-2.3
> > machine is already broken between QEMU-2.3 and QEMU-2.4. So this patch
> > fixes indeed migration of a pseries-2.3 machine from QEMU-2.3, but it
> > breaks migration of the same machine from QEMU-2.4 and up.
> > 
> > Not sure how to deal with that... is it reasonable to assume that
> > pseries-2.3 running with QEMU-2.3 is the common case ? If so, this
> > patch would bring more help than harm.
> 
> Unfortunately we can not fix history, so we have to pick something to fix.
> So unless there is another reason, then I normally say keep it working
> between the latest versions of qemu; i.e. if someone is running qemu 2.5 with
> -M 2.3  then dont break it when they try and migrate to 2.6, even though
> this would fix an older qemu migrating into 2.6.

Yeah, I tend to agree, but I'd change my mind if there's evidence that
the older qemu is much more widely deployed.

IIUC that would entail no actual change to the code yes?  But I think
we should put a comment there saying what the fix would be to talk to
the older qemu, and why we chose not to apply it.

> However, as discussed on irc you might be able to fudge it; for example
> using qemu_peek_byte to test whether or not you have a configuration
> section, and making it not error for some machine types.   This isn't
> pretty, but if it's important for you to get the coniguration working
> then it's the type of trick that might work.
> 
> Dave
> 
> > 
> > > Fixes: 61964c23e5ddd5a33f15699e45ce126f879e3e33
> > > Cc: qemu-sta...@nongnu.org
> > > Signed-off-by: Greg Kurz 
> > > ---
> > >  hw/ppc/spapr.c |1 +
> > >  1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > > index 5bd8fd3ef842..bca7cb8a5d27 100644
> > > --- a/hw/ppc/spapr.c
> > > +++ b/hw/ppc/spapr.c
> > > @@ -2446,6 +2446,7 @@ static void 
> > > spapr_machine_2_3_instance_options(MachineState *machine)
> > >  spapr_machine_2_4_instance_options(machine);
> > >  savevm_skip_section_footers();
> > >  global_state_set_optional();
> > > +savevm_skip_configuration();
> > >  }
> > > 
> > >  static void spapr_machine_2_3_class_options(MachineClass *mc)
> > > 
> > > 
> > 

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature


Re: [Qemu-devel] [PATCH 1/9] dma-helpers: Expose the sg mapping logic

2016-02-11 Thread Paolo Bonzini


On 16/12/2015 17:55, Alex Pyrgiotis wrote:
> +/*
> + * Create a QEMUIOVector from a scatter-gather list.
> + *
> + * This function does not copy the data of the scatter-gather list. Instead, 
> it
> + * uses the dma_memory_map() function to map physical memory regions of the
> + * virtual device (as interpreted by the guest kernel) into the address space
> + * of the QEMU process, in order to have access to the data.
> + */
> +static void dma_map_sg(DMAAIOCB *dbs)

In special cases where the QEMUSGList includes MMIO regions, dma_map_sg
might not be able to map the whole list.  In this case, for regular I/O
it is possible to break the operation in multiple steps---in fact, this
breaking of requests is the main purpose of most of the code in
dma-helpers.c.

However, it is not possible to do the same for ioctls.  This is actually
the reason why no one has ever tried to make scsi-generic do anything
but bounce-buffering.  I think that your code breaks horribly in this
case, and I don't see a way to fix it, except for reverting to bounce
buffering.

This would require major changes in your patches, and I'm not sure
whether they are worth it for the single use case of tape devices...

Paolo



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

2016-02-11 Thread Peter Maydell
On 9 February 2016 at 18:42, Peter Maydell  wrote:
> Various things in this pull, but the one I care most about is that
> it includes the "enable EL3 for 64-bit CPUs" patches.
>
> thanks
> -- PMM
>
>
> The following changes since commit 84c0781103dcbe9b5e5433ba16fbeb55d69d6cb7:
>
>   Merge remote-tracking branch 'remotes/armbru/tags/pull-error-2016-02-09' 
> into staging (2016-02-09 16:09:15 +)
>
> are available in the git repository at:
>
>
>   git://git.linaro.org/people/pmaydell/qemu-arm.git 
> tags/pull-target-arm-20160209
>
> for you to fetch changes up to dfe1da1c1271dff825676435ff90da92cf4f:
>
>   bcm2835_property: implement "get board revision" query (2016-02-09 18:27:27 
> +)
>
> 
> target-arm queue:
>  * fix some missing traps for EL3 support
>  * enable EL3 on Cortex-A53 and Cortex-A57
>  * fix syndrome IL bit for Thumb coprocessor, VFP and Neon traps
>  * fix mishandling of architectural watchpoints
>  * avoid buffer overflow in sd.c
>  * fix max-cpus check in virt board
>  * implement 'get board revision' query for BCM2835

Ran into the "one of our compilers doesn't like typedef redefinitions" issue:
/home/petmay01/linaro/qemu-for-merges/include/qom/cpu.h:221: error:
redefinition of typedef ‘CPUWatchpoint’
/home/petmay01/linaro/qemu-for-merges/include/qom/cpu.h:67: error:
previous declaration of ‘CPUWatchpoint’ was here

Will do the trivial fix and resend:

diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 984bc8d..ff54600 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -211,14 +211,14 @@ typedef struct CPUBreakpoint {
 QTAILQ_ENTRY(CPUBreakpoint) entry;
 } CPUBreakpoint;

-typedef struct CPUWatchpoint {
+struct CPUWatchpoint {
 vaddr vaddr;
 vaddr len;
 vaddr hitaddr;
 MemTxAttrs hitattrs;
 int flags; /* BP_* */
 QTAILQ_ENTRY(CPUWatchpoint) entry;
-} CPUWatchpoint;
+};

 struct KVMState;
 struct kvm_run;

thanks
-- PMM



Re: [Qemu-devel] [PATCH v6 5/9] qemu-log: support simple pid substitution in logfile

2016-02-11 Thread Alex Bennée

Alex Bennée  writes:

> When debugging stuff that occurs over several forks it would be useful
> not to keep overwriting the one logfile you've set-up. This allows a
> simple %d to be included once in the logfile parameter which is
> substituted with getpid().
>
> Signed-off-by: Alex Bennée 
> Reviewed-by: Leandro Dorileo 
> Reviewed-by: Aurelien Jarno 
>
> ---
> v5
>   - add another r-b
> ---
>  util/log.c | 17 +++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/util/log.c b/util/log.c
> index c89b226..3988b5d 100644
> --- a/util/log.c
> +++ b/util/log.c
> @@ -75,11 +75,24 @@ void do_qemu_set_log(int log_flags, bool use_own_buffers)
>  qemu_log_close();
>  }
>  }
> -
> +/*
> + * Allow the user to include %d in their logfile which will be
> + * substituted with the current PID. This is useful for debugging many
> + * nested linux-user tasks but will result in lots of logs.
> + */
>  void qemu_set_log_filename(const char *filename)
>  {
>  g_free(logfilename);
> -logfilename = g_strdup(filename);
> +if (g_strrstr(filename, "%d")) {

Just realised I missed the strrstr -> strstr fix. Not that it makes a
lot of difference in this case.

> +/* if we are going to format this we'd better validate first */
> +if (g_regex_match_simple("^[^%]+%d[^%]+$", filename, 0, 0)) {
> +logfilename = g_strdup_printf(filename, getpid());
> +} else {
> +g_error("Bad logfile format: %s", filename);
> +}
> +} else {
> +logfilename = g_strdup(filename);
> +}
>  qemu_log_close();
>  qemu_set_log(qemu_loglevel);
>  }


--
Alex Bennée



[Qemu-devel] [PULL 04/14] cpu: cpu_save/cpu_load is no more

2016-02-11 Thread Michael Tokarev
From: Paolo Bonzini 

Everything has been converted to vmstate.

Signed-off-by: Paolo Bonzini 
Signed-off-by: Michael Tokarev 
---
 exec.c| 6 --
 include/qemu-common.h | 6 --
 2 files changed, 12 deletions(-)

diff --git a/exec.c b/exec.c
index 7d67c11..ca7f8df 100644
--- a/exec.c
+++ b/exec.c
@@ -661,12 +661,6 @@ void cpu_exec_init(CPUState *cpu, Error **errp)
 if (qdev_get_vmsd(DEVICE(cpu)) == NULL) {
 vmstate_register(NULL, cpu_index, _cpu_common, cpu);
 }
-#if defined(CPU_SAVE_VERSION) && !defined(CONFIG_USER_ONLY)
-register_savevm(NULL, "cpu", cpu_index, CPU_SAVE_VERSION,
-cpu_save, cpu_load, cpu->env_ptr);
-assert(cc->vmsd == NULL);
-assert(qdev_get_vmsd(DEVICE(cpu)) == NULL);
-#endif
 if (cc->vmsd != NULL) {
 vmstate_register(NULL, cpu_index, cc->vmsd, cpu);
 }
diff --git a/include/qemu-common.h b/include/qemu-common.h
index 22b010c..f557be7 100644
--- a/include/qemu-common.h
+++ b/include/qemu-common.h
@@ -330,12 +330,6 @@ bool tcg_enabled(void);
 
 void cpu_exec_init_all(void);
 
-/* CPU save/load.  */
-#ifdef CPU_SAVE_VERSION
-void cpu_save(QEMUFile *f, void *opaque);
-int cpu_load(QEMUFile *f, void *opaque, int version_id);
-#endif
-
 /* Unblock cpu */
 void qemu_cpu_kick_self(void);
 
-- 
2.1.4




[Qemu-devel] [PULL 09/14] ES1370: QOMify

2016-02-11 Thread Michael Tokarev
From: Cao jin 

Signed-off-by: Cao jin 
Signed-off-by: Michael Tokarev 
---
 hw/audio/es1370.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/hw/audio/es1370.c b/hw/audio/es1370.c
index 96acbc6..8449b5f 100644
--- a/hw/audio/es1370.c
+++ b/hw/audio/es1370.c
@@ -289,6 +289,10 @@ struct chan_bits {
uint32_t *old_freq, uint32_t *new_freq);
 };
 
+#define TYPE_ES1370 "ES1370"
+#define ES1370(obj) \
+OBJECT_CHECK(ES1370State, (obj), TYPE_ES1370)
+
 static void es1370_dac1_calc_freq (ES1370State *s, uint32_t ctl,
uint32_t *old_freq, uint32_t *new_freq);
 static void es1370_dac2_and_adc_calc_freq (ES1370State *s, uint32_t ctl,
@@ -1014,7 +1018,7 @@ static void es1370_on_reset (void *opaque)
 
 static void es1370_realize(PCIDevice *dev, Error **errp)
 {
-ES1370State *s = DO_UPCAST (ES1370State, dev, dev);
+ES1370State *s = ES1370(dev);
 uint8_t *c = s->dev.config;
 
 c[PCI_STATUS + 1] = PCI_STATUS_DEVSEL_SLOW >> 8;
@@ -1039,7 +1043,7 @@ static void es1370_realize(PCIDevice *dev, Error **errp)
 
 static int es1370_init (PCIBus *bus)
 {
-pci_create_simple (bus, -1, "ES1370");
+pci_create_simple (bus, -1, TYPE_ES1370);
 return 0;
 }
 
@@ -1060,7 +1064,7 @@ static void es1370_class_init (ObjectClass *klass, void 
*data)
 }
 
 static const TypeInfo es1370_info = {
-.name  = "ES1370",
+.name  = TYPE_ES1370,
 .parent= TYPE_PCI_DEVICE,
 .instance_size = sizeof (ES1370State),
 .class_init= es1370_class_init,
-- 
2.1.4




[Qemu-devel] [PULL 01/14] remove libtool support

2016-02-11 Thread Michael Tokarev
Libtool support was needed to build shared library for libcacard.
Now there's no need to use libtool, and since the build system is
already complicated enough, we have a way to slightly de-complicate
it.

Signed-off-by: Michael Tokarev 
Reviewed-by: Greg Kurz 
---
 configure | 86 ++-
 qemu-doc.texi |  1 -
 rules.mak | 18 -
 3 files changed, 2 insertions(+), 103 deletions(-)

diff --git a/configure b/configure
index c9cf1c9..2ff8dd5 100755
--- a/configure
+++ b/configure
@@ -116,38 +116,6 @@ compile_prog() {
   do_cc $QEMU_CFLAGS $local_cflags -o $TMPE $TMPC $LDFLAGS $local_ldflags
 }
 
-do_libtool() {
-local mode=$1
-shift
-# Run the compiler, capturing its output to the log.
-echo $libtool $mode --tag=CC $cc "$@" >> config.log
-$libtool $mode --tag=CC $cc "$@" >> config.log 2>&1 || return $?
-# Test passed. If this is an --enable-werror build, rerun
-# the test with -Werror and bail out if it fails. This
-# makes warning-generating-errors in configure test code
-# obvious to developers.
-if test "$werror" != "yes"; then
-return 0
-fi
-# Don't bother rerunning the compile if we were already using -Werror
-case "$*" in
-*-Werror*)
-   return 0
-;;
-esac
-echo $libtool $mode --tag=CC $cc -Werror "$@" >> config.log
-$libtool $mode --tag=CC $cc -Werror "$@" >> config.log 2>&1 && return $?
-error_exit "configure test passed without -Werror but failed with 
-Werror." \
-"This is probably a bug in the configure script. The failing command" \
-"will be at the bottom of config.log." \
-"You can run configure with --disable-werror to bypass this check."
-}
-
-libtool_prog() {
-do_libtool --mode=compile $QEMU_CFLAGS -c -fPIE -DPIE -o $TMPO $TMPC || 
return $?
-do_libtool --mode=link $LDFLAGS -o $TMPA $TMPL -rpath /usr/local/lib
-}
-
 # symbolically link $1 to $2.  Portable version of "ln -sf".
 symlink() {
   rm -rf "$2"
@@ -398,7 +366,6 @@ as="${AS-${cross_prefix}as}"
 cpp="${CPP-$cc -E}"
 objcopy="${OBJCOPY-${cross_prefix}objcopy}"
 ld="${LD-${cross_prefix}ld}"
-libtool="${LIBTOOL-${cross_prefix}libtool}"
 nm="${NM-${cross_prefix}nm}"
 strip="${STRIP-${cross_prefix}strip}"
 windres="${WINDRES-${cross_prefix}windres}"
@@ -1514,7 +1481,6 @@ EOF
 if do_cc $QEMU_CFLAGS -Werror $flag -c -o $TMPO $TMPC &&
compile_prog "-Werror $flag" ""; then
   QEMU_CFLAGS="$QEMU_CFLAGS $flag"
-  LIBTOOLFLAGS="$LIBTOOLFLAGS -Wc,$flag"
   sp_on=1
   break
 fi
@@ -1609,32 +1575,6 @@ EOF
   fi
 fi
 
-# check for broken gcc and libtool in RHEL5
-if test -n "$libtool" -a "$pie" != "no" ; then
-  cat > $TMPC &1; then
-libtool=
-  fi
-fi
-
-##
 # Sparse probe
 if test "$sparse" != "no" ; then
   if has cgcc; then
@@ -5525,13 +5450,8 @@ echo "MAKE=$make" >> $config_host_mak
 echo "INSTALL=$install" >> $config_host_mak
 echo "INSTALL_DIR=$install -d -m 0755" >> $config_host_mak
 echo "INSTALL_DATA=$install -c -m 0644" >> $config_host_mak
-if test -n "$libtool"; then
-  echo "INSTALL_PROG=\$(LIBTOOL) --mode=install $install -c -m 0755" >> 
$config_host_mak
-  echo "INSTALL_LIB=\$(LIBTOOL) --mode=install $install -c -m 0644" >> 
$config_host_mak
-else
-  echo "INSTALL_PROG=$install -c -m 0755" >> $config_host_mak
-  echo "INSTALL_LIB=$install -c -m 0644" >> $config_host_mak
-fi
+echo "INSTALL_PROG=$install -c -m 0755" >> $config_host_mak
+echo "INSTALL_LIB=$install -c -m 0644" >> $config_host_mak
 echo "PYTHON=$python" >> $config_host_mak
 echo "CC=$cc" >> $config_host_mak
 if $iasl -h > /dev/null 2>&1; then
@@ -5549,7 +5469,6 @@ echo "OBJCOPY=$objcopy" >> $config_host_mak
 echo "LD=$ld" >> $config_host_mak
 echo "NM=$nm" >> $config_host_mak
 echo "WINDRES=$windres" >> $config_host_mak
-echo "LIBTOOL=$libtool" >> $config_host_mak
 echo "CFLAGS=$CFLAGS" >> $config_host_mak
 echo "CFLAGS_NOPIE=$CFLAGS_NOPIE" >> $config_host_mak
 echo "QEMU_CFLAGS=$QEMU_CFLAGS" >> $config_host_mak
@@ -5568,7 +5487,6 @@ else
 fi
 echo "LDFLAGS=$LDFLAGS" >> $config_host_mak
 echo "LDFLAGS_NOPIE=$LDFLAGS_NOPIE" >> $config_host_mak
-echo "LIBTOOLFLAGS=$LIBTOOLFLAGS" >> $config_host_mak
 echo "LIBS+=$LIBS" >> $config_host_mak
 echo "LIBS_TOOLS+=$libs_tools" >> $config_host_mak
 echo "EXESUF=$EXESUF" >> $config_host_mak
diff --git a/qemu-doc.texi b/qemu-doc.texi
index 212aba3..c324da8 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -3064,7 +3064,6 @@ Additional Requirements (install in order):
 @item pkg-config: @uref{http://www.freedesktop.org/wiki/Software/pkg-config/}
 @item autoconf: @uref{http://www.gnu.org/software/autoconf/autoconf.html}
 @item automake: @uref{http://www.gnu.org/software/automake/}
-@item libtool: @uref{http://www.gnu.org/software/libtool/}
 @item pixman: @uref{http://www.pixman.org/}
 @end enumerate
 
diff --git 

[Qemu-devel] [PATCH RESEND v5] i2c-tiny-usb is a small usb to i2c bridge

2016-02-11 Thread Tim Sander
Hi

Probably due to my less then stellar patch to mail handling i think this
patch got forgotten. I think its ok to resend.

The patch itself is unchanged. It should incorperate all sugestions but the 
one of Peter due not beeing mainlined:
https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg04237.html

Here are some pointers to the old discussion:
https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg00111.html
https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg02634.html

Best regards
Tim

i2c-tiny-usb is a small usb to i2c bridge:
 http://www.harbaum.org/till/i2c_tiny_usb/index.shtml

It is pretty simple and has no usb endpoints just a control.
Reasons for adding this device:
* Linux device driver available
* adding an additional i2c bus via command line e.g.
  -device usb-i2c-tiny,id=i2c-0 -device tmp105,bus=i2c,address=0x50
---
Signed-off-by: Tim Sander 
 default-configs/usb.mak |   1 +
 hw/usb/Makefile.objs|   1 +
 hw/usb/dev-i2c-tiny.c   | 314 

 trace-events|  11 ++
 4 files changed, 327 insertions(+)
 create mode 100644 hw/usb/dev-i2c-tiny.c

diff --git a/default-configs/usb.mak b/default-configs/usb.mak
index f4b8568..01d2c9f 100644
--- a/default-configs/usb.mak
+++ b/default-configs/usb.mak
@@ -8,3 +8,4 @@ CONFIG_USB_AUDIO=y
 CONFIG_USB_SERIAL=y
 CONFIG_USB_NETWORK=y
 CONFIG_USB_BLUETOOTH=y
+CONFIG_USB_I2C_TINY=y
diff --git a/hw/usb/Makefile.objs b/hw/usb/Makefile.objs
index 8f00fbd..3a4c337 100644
--- a/hw/usb/Makefile.objs
+++ b/hw/usb/Makefile.objs
@@ -20,6 +20,7 @@ common-obj-$(CONFIG_USB_AUDIO)+= dev-audio.o
 common-obj-$(CONFIG_USB_SERIAL)   += dev-serial.o
 common-obj-$(CONFIG_USB_NETWORK)  += dev-network.o
 common-obj-$(CONFIG_USB_BLUETOOTH)+= dev-bluetooth.o
+common-obj-$(CONFIG_USB_I2C_TINY) += dev-i2c-tiny.o
 
 ifeq ($(CONFIG_USB_SMARTCARD),y)
 common-obj-y  += dev-smartcard-reader.o
diff --git a/hw/usb/dev-i2c-tiny.c b/hw/usb/dev-i2c-tiny.c
new file mode 100644
index 000..7abb9df
--- /dev/null
+++ b/hw/usb/dev-i2c-tiny.c
@@ -0,0 +1,314 @@
+/*
+ * I2C tiny usb device emulation
+ *
+ * i2c-tiny-usb is a small usb to i2c bridge:
+ *
+ * http://www.harbaum.org/till/i2c_tiny_usb/index.shtml
+ *
+ * The simulated device is pretty simple and has no usb endpoints.
+ * There is a Linux device driver available named i2c-tiny-usb.
+ *
+ * Below is an example how to use this device from command line:
+ *  -device usb-i2c-tiny,id=i2c-0 -device tmp105,bus=i2c,address=0x50
+ *
+ * Copyright (c) 2015 Tim Sander 
+ *
+ * Loosly based on usb dev-serial.c:
+ * Copyright (c) 2006 CodeSourcery.
+ * Copyright (c) 2008 Samuel Thibault 
+ * Written by Paul Brook, reused for FTDI by Samuel Thibault
+ *
+ * This code is licensed under the LGPL.
+ *
+ */
+
+#include "trace.h"
+#include "qemu-common.h"
+#include "qemu/error-report.h"
+#include "hw/usb.h"
+#include "hw/usb/desc.h"
+#include "hw/i2c/i2c.h"
+#include "hw/i2c/smbus.h"
+#include "sysemu/char.h"
+#include "endian.h"
+
+/* commands from USB, must e.g. match command ids in kernel driver */
+#define CMD_ECHO   0
+#define CMD_GET_FUNC   1
+#define CMD_SET_DELAY  2
+#define CMD_GET_STATUS 3
+
+/* To determine what functionality is present */
+#define I2C_FUNC_I2C0x0001
+#define I2C_FUNC_10BIT_ADDR 0x0002
+#define I2C_FUNC_PROTOCOL_MANGLING  0x0004
+#define I2C_FUNC_SMBUS_HWPEC_CALC   0x0008 /* SMBus 2.0 */
+#define I2C_FUNC_SMBUS_READ_WORD_DATA_PEC   0x0800 /* SMBus 2.0 */
+#define I2C_FUNC_SMBUS_WRITE_WORD_DATA_PEC  0x1000 /* SMBus 2.0 */
+#define I2C_FUNC_SMBUS_PROC_CALL_PEC0x2000 /* SMBus 2.0 */
+#define I2C_FUNC_SMBUS_BLOCK_PROC_CALL_PEC  0x4000 /* SMBus 2.0 */
+#define I2C_FUNC_SMBUS_BLOCK_PROC_CALL  0x8000 /* SMBus 2.0 */
+#define I2C_FUNC_SMBUS_QUICK0x0001
+#define I2C_FUNC_SMBUS_READ_BYTE0x0002
+#define I2C_FUNC_SMBUS_WRITE_BYTE   0x0004
+#define I2C_FUNC_SMBUS_READ_BYTE_DATA   0x0008
+#define I2C_FUNC_SMBUS_WRITE_BYTE_DATA  0x0010
+#define I2C_FUNC_SMBUS_READ_WORD_DATA   0x0020
+#define I2C_FUNC_SMBUS_WRITE_WORD_DATA  0x0040
+#define I2C_FUNC_SMBUS_PROC_CALL0x0080
+#define I2C_FUNC_SMBUS_READ_BLOCK_DATA  0x0100
+#define I2C_FUNC_SMBUS_WRITE_BLOCK_DATA 0x0200
+#define I2C_FUNC_SMBUS_READ_I2C_BLOCK   0x0400 /*I2C-like blk-xfr 
*/
+#define I2C_FUNC_SMBUS_WRITE_I2C_BLOCK  0x0800 /*1-byte reg. 
addr.*/
+#define I2C_FUNC_SMBUS_READ_I2C_BLOCK_2 0x1000 /*I2C-like blk-
xfer*/
+#define I2C_FUNC_SMBUS_WRITE_I2C_BLOCK_20x2000 /* w/ 2-byte 
regadr*/
+#define I2C_FUNC_SMBUS_READ_BLOCK_DATA_PEC  0x4000 /* SMBus 2.0 

Re: [Qemu-devel] [PATCH v19 3/9] pc: add a Virtual Machine Generation ID device

2016-02-11 Thread Igor Mammedov
On Tue, 9 Feb 2016 14:17:44 +0200
"Michael S. Tsirkin"  wrote:

> On Tue, Feb 09, 2016 at 11:46:08AM +0100, Igor Mammedov wrote:
> > > So the linker interface solves this rather neatly:
> > > bios allocates memory, bios passes memory map to guest.
> > > Served us well for several years without need for extensions,
> > > and it does solve the VM GEN ID problem, even though
> > > 1. it was never designed for huge areas like nvdimm seems to want to use
> > > 2. we might want to add a new 64 bit flag to avoid touching low memory  
> > linker interface is fine for some readonly data, like ACPI tables
> > especially fixed tables not so for AML ones is one wants to patch it.
> > 
> > However now when you want to use it for other purposes you start
> > adding extensions and other guest->QEMU channels to communicate
> > patching info back.
> > It steals guest's memory which is also not nice and doesn't scale well.  
> 
> This is an argument I don't get. memory is memory. call it guest memory
> or RAM backed PCI BAR - same thing. MMIO is cheaper of course
> but much slower.
> 
> ...
It however matters for user, he pays for guest with XXX RAM but gets less
than that. And that will be getting worse as a number of such devices
increases.

> > > OK fine, but returning PCI BAR address to guest is wrong.
> > > How about reading it from ACPI then? Is it really
> > > broken unless there's *also* a driver?  
> > I don't get question, MS Spec requires address (ADDR method),
> > and it's read by ACPI (AML).  
> 
> You were unhappy about DMA into guest memory.
> As a replacement for DMA, we could have AML read from
> e.g. PCI and write into RAM.
> This way we don't need to pass address to QEMU.
That sounds better as it saves us from allocation of IO port
and QEMU don't need to write into guest memory, the only question is
if PCI_Config opregion would work with driver-less PCI device.

And it's still pretty much not test-able since it would require
fully running OSPM to execute AML side.

> 
> > As for working PCI_Config OpRegion without driver, I haven't tried,
> > but I wouldn't be surprised if it doesn't, taking in account that
> > MS introduced _DSM doesn't.
> >   
> > > 
> > >   
> > > > > >Just compare with a graphics card design, where on device memory
> > > > > >is mapped directly at some GPA not wasting RAM that guest could
> > > > > >use for other tasks.  
> > > > > 
> > > > > This might have been true 20 years ago.  Most modern cards do DMA.
> > > > 
> > > > Modern cards, with it's own RAM, map its VRAM in address space directly
> > > > and allow users use it (GEM API). So they do not waste conventional RAM.
> > > > For example NVIDIA VRAM is mapped as PCI BARs the same way like in this
> > > > series (even PCI class id is the same)
> > > 
> > > Don't know enough about graphics really, I'm not sure how these are
> > > relevant.  NICs and disks certainly do DMA.  And virtio gl seems to
> > > mostly use guest RAM, not on card RAM.
> > >   
> > > > > >VMGENID and NVDIMM use-cases look to me exactly the same, i.e.
> > > > > >instead of consuming guest's RAM they should be mapped at
> > > > > >some GPA and their memory accessed directly.  
> > > > > 
> > > > > VMGENID is tied to a spec that rather arbitrarily asks for a fixed
> > > > > address. This breaks the straight-forward approach of using a
> > > > > rebalanceable PCI BAR.
> > > > 
> > > > For PCI rebalance to work on Windows, one has to provide working PCI 
> > > > driver
> > > > otherwise OS will ignore it when rebalancing happens and
> > > > might map something else over ignored BAR.
> > > 
> > > Does it disable the BAR then? Or just move it elsewhere?  
> > it doesn't, it just blindly ignores BARs existence and maps BAR of
> > another device with driver over it.  
> 
> Interesting. On classical PCI this is a forbidden configuration.
> Maybe we do something that confuses windows?
> Could you tell me how to reproduce this behaviour?
#cat > t << EOF
pci_update_mappings_del
pci_update_mappings_add
EOF

#./x86_64-softmmu/qemu-system-x86_64 -snapshot -enable-kvm -snapshot \
 -monitor unix:/tmp/m,server,nowait -device pci-bridge,chassis_nr=1 \
 -boot menu=on -m 4G -trace events=t ws2012r2x64dc.img \
 -device ivshmem,id=foo,size=2M,shm,bus=pci.1,addr=01

wait till OS boots, note BARs programmed for ivshmem
 in my case it was
   01:01.0 0,0xfe80+0x100
then execute script and watch pci_update_mappings* trace events

# for i in $(seq 3 18); do printf -- "device_add e1000,bus=pci.1,addr=%x\n" $i 
| nc -U /tmp/m; sleep 5; done;

hotplugging e1000,bus=pci.1,addr=12 triggers rebalancing where
Windows unmaps all BARs of nics on bridge but doesn't touch ivshmem
and then programs new BARs, where:
  pci_update_mappings_add d=0x7fa02ff0cf90 01:11.0 0,0xfe80+0x2
creates overlapping BAR with ivshmem 


> 
> > >   
> > > > > 
> > > > > >In that case NVDIMM could even map whole label area and
> > > > > 

Re: [Qemu-devel] [PATCH v7 3/5] acpi: pc: add fw_cfg device node to ssdt

2016-02-11 Thread Igor Mammedov
On Wed, 10 Feb 2016 15:41:38 -0500
"Gabriel L. Somlo"  wrote:

> Add a fw_cfg device node to the ACPI SSDT. While the guest-side
> firmware can't utilize this information (since it has to access
> the hard-coded fw_cfg device to extract ACPI tables to begin with),
> having fw_cfg listed in ACPI will help the guest kernel keep a more
> accurate inventory of in-use IO port regions.
subj and commit msg:
s/SSDT/DSDT/

> 
> Signed-off-by: Gabriel Somlo 
> Reviewed-by: Laszlo Ersek 
> Reviewed-by: Marc Marí 
> ---
>  hw/i386/acpi-build.c | 29 +
>  1 file changed, 29 insertions(+)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 4554eb8..4762fd2 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -2190,6 +2190,35 @@ build_dsdt(GArray *table_data, GArray *linker,
>  aml_append(scope, aml_name_decl("_S5", pkg));
>  aml_append(dsdt, scope);
>  
> +/* create fw_cfg node, unconditionally */
Will that unconditionally make all Windows guests ask for driver for unknown 
device?


> +{
> +/* when using port i/o, the 8-bit data register *always* overlaps
> + * with half of the 16-bit control register. Hence, the total size
> + * of the i/o region used is FW_CFG_CTL_SIZE; when using DMA, the
> + * DMA control register is located at FW_CFG_DMA_IO_BASE + 4 */
> +uint8_t io_size = object_property_get_bool(OBJECT(pcms->fw_cfg),
> +   "dma_enabled", NULL) ?
> +  ROUND_UP(FW_CFG_CTL_SIZE, 4) + sizeof(dma_addr_t) :
> +  FW_CFG_CTL_SIZE;
> +
> +scope = aml_scope("\\_SB");
> +dev = aml_device("FWCF");
> +
> +aml_append(dev, aml_name_decl("_HID", aml_string("QEMU0002")));
> +
> +/* device present, functioning, decoding, not shown in UI */
> +aml_append(dev, aml_name_decl("_STA", aml_int(0xB)));
> +
> +crs = aml_resource_template();
> +aml_append(crs,
> +aml_io(AML_DECODE16, FW_CFG_IO_BASE, FW_CFG_IO_BASE, 0x01, 
> io_size)
> +);
> +aml_append(dev, aml_name_decl("_CRS", crs));
> +
> +aml_append(scope, dev);
> +aml_append(dsdt, scope);
> +}
> +
>  if (misc->applesmc_io_base) {
>  scope = aml_scope("\\_SB.PCI0.ISA");
>  dev = aml_device("SMC");




Re: [Qemu-devel] [PATCH v2 4/4] replay: introduce block devices record/replay

2016-02-11 Thread Stefan Hajnoczi
On Thu, Feb 11, 2016 at 04:52:42PM +0300, Pavel Dovgalyuk wrote:
> > From: Stefan Hajnoczi [mailto:stefa...@redhat.com]
> > On Wed, Feb 10, 2016 at 12:13:23PM +0300, Pavel Dovgalyuk wrote:
> > > @@ -784,7 +798,11 @@ BlockAIOCB *blk_aio_flush(BlockBackend *blk,
> > >  return blk_abort_aio_request(blk, cb, opaque, -ENOMEDIUM);
> > >  }
> > >
> > > -return bdrv_aio_flush(blk->bs, cb, opaque);
> > > +if (replay_mode == REPLAY_MODE_NONE) {
> > > +return bdrv_aio_flush(blk->bs, cb, opaque);
> > > +} else {
> > > +return replay_aio_flush(blk->bs, cb, opaque);
> > > +}
> > >  }
> > 
> > I am only looking at this patch in isolation and am not familiar with
> > the record/replay work, but these changes appear invasive.  In order for
> > record/replay to keep working in the future, everyone touching block
> > layer code must be familiar with the principles of how record/replay
> > works.  This patch does not include documentation.
> 
> We've already discussed it with Kevin.
> He proposes adding new block driver for record/replay.
> 
> > Can you point me to documentation that explains the how record replay
> > works?
> 
> There is some information about it in docs/replay.txt and in
> http://wiki.qemu.org/Features/record-replay

I was thinking about developer documentation.  A feature like this is
"cross-cutting" - it affects many components.  There should be
documentation that explains the semantics so everyone modifying code can
follow the rules to keep record/replay working.

Live migration is similar in that it touches many components.  The
docs/migration.txt file explains the APIs and general idea.  It
communicates the assumptions, limitations, and requirements that live
migration imposes.  Many QEMU developers understand migration basics and
keep them in mind during code review.  This way we can prevent most (but
not all) migration breakage.

Record/replay should aim for the same level of education/awareness,
otherwise the feature will break and bitrot.

The alternative is to implement it in a way that isn't invasive.  Then
other developers don't need to understand how it works.  Maybe
implementing it in a block driver can achieve this.

Stefan


signature.asc
Description: PGP signature


Re: [Qemu-devel] [PULL 00/11] Ide patches

2016-02-11 Thread Peter Maydell
On 10 February 2016 at 19:37, John Snow  wrote:
> The following changes since commit c9f19dff101e2c2cf3fa3967eceec2833e845e40:
>
>   Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into 
> staging (2016-02-09 19:34:46 +)
>
> are available in the git repository at:
>
>   https://github.com/jnsnow/qemu.git tags/ide-pull-request
>
> for you to fetch changes up to d590474922d37372c56075adb229c86d3aeae967:
>
>   ahci: prohibit "restarting" the FIS or CLB engines (2016-02-10 13:29:40 
> -0500)
>
> 
>
> 

Applied, thanks.

-- PMM



Re: [Qemu-devel] [BUG] trace: QEMU hangs on initialization with the "simple" backend

2016-02-11 Thread Stefan Hajnoczi
On Tue, Feb 09, 2016 at 09:24:04PM +0100, Lluís Vilanova wrote:
> While starting the softmmu version of QEMU, the simple backend waits for the
> writeout thread to signal a condition variable when initializing the output 
> file
> path. But since the writeout thread has not been created, it just waits 
> forever.

Denis Lunev posted a fix:
https://patchwork.ozlabs.org/patch/580968/

Stefan


signature.asc
Description: PGP signature


[Qemu-devel] [PATCH v2 0/2] move qcow2_invalidate_cache() out of coroutine context

2016-02-11 Thread Denis V. Lunev
There is a possibility to hit an assert in qcow2_get_specific_info that
s->qcow_version is undefined. This happens when VM in starting from
suspended state, i.e. it processes incoming migration, and in the same
time 'info block' is called.

The problem is that qcow2_invalidate_cache() closes the image and
memset()s BDRVQcowState in the middle.

This operation should not be performed in coroutine context.

Changes from v1:
- fixed spelling. Eric, thank you for spell checking

Signed-off-by: Denis V. Lunev 
CC: Paolo Bonzini 
CC: Juan Quintela 
CC: Amit Shah 




[Qemu-devel] [PATCH 2/2] migration: move bdrv_invalidate_cache_all of of coroutine context

2016-02-11 Thread Denis V. Lunev
There is a possibility to hit an assert in qcow2_get_specific_info that
s->qcow_version is undefined. This happens when VM in starting from
suspended state, i.e. it processes incoming migration, and in the same
time 'info block' is called.

The problem is that qcow2_invalidate_cache() closes the image and
memset()s BDRVQcowState in the middle.

The patch moves processing of bdrv_invalidate_cache_all out of
coroutine context for postcopy migration to avoid that. This function
is called with the following stack:
  process_incoming_migration_co
  qemu_loadvm_state
  qemu_loadvm_state_main
  loadvm_process_command
  loadvm_postcopy_handle_run

Signed-off-by: Denis V. Lunev 
CC: Paolo Bonzini 
CC: Juan Quintela 
CC: Amit Shah 
---
 migration/savevm.c | 27 +--
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/migration/savevm.c b/migration/savevm.c
index 94f2894..8415fd9 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1496,18 +1496,10 @@ static int 
loadvm_postcopy_handle_listen(MigrationIncomingState *mis)
 return 0;
 }
 
-/* After all discards we can start running and asking for pages */
-static int loadvm_postcopy_handle_run(MigrationIncomingState *mis)
+static void loadvm_postcopy_handle_run_bh(void *opaque)
 {
-PostcopyState ps = postcopy_state_set(POSTCOPY_INCOMING_RUNNING);
 Error *local_err = NULL;
 
-trace_loadvm_postcopy_handle_run();
-if (ps != POSTCOPY_INCOMING_LISTENING) {
-error_report("CMD_POSTCOPY_RUN in wrong postcopy state (%d)", ps);
-return -1;
-}
-
 /* TODO we should move all of this lot into postcopy_ram.c or a shared code
  * in migration.c
  */
@@ -1519,7 +1511,6 @@ static int 
loadvm_postcopy_handle_run(MigrationIncomingState *mis)
 bdrv_invalidate_cache_all(_err);
 if (local_err) {
 error_report_err(local_err);
-return -1;
 }
 
 trace_loadvm_postcopy_handle_run_cpu_sync();
@@ -1534,6 +1525,22 @@ static int 
loadvm_postcopy_handle_run(MigrationIncomingState *mis)
 /* leave it paused and let management decide when to start the CPU */
 runstate_set(RUN_STATE_PAUSED);
 }
+}
+
+/* After all discards we can start running and asking for pages */
+static int loadvm_postcopy_handle_run(MigrationIncomingState *mis)
+{
+PostcopyState ps = postcopy_state_set(POSTCOPY_INCOMING_RUNNING);
+QEMUBH *bh;
+
+trace_loadvm_postcopy_handle_run();
+if (ps != POSTCOPY_INCOMING_LISTENING) {
+error_report("CMD_POSTCOPY_RUN in wrong postcopy state (%d)", ps);
+return -1;
+}
+
+bh = qemu_bh_new(loadvm_postcopy_handle_run_bh, NULL);
+qemu_bh_schedule(bh);
 
 /* We need to finish reading the stream from the package
  * and also stop reading anything more from the stream that loaded the
-- 
2.1.4




[Qemu-devel] [PATCH 1/2] migration: move bdrv_invalidate_cache_all of of coroutine context

2016-02-11 Thread Denis V. Lunev
There is a possibility to hit an assert in qcow2_get_specific_info that
s->qcow_version is undefined. This happens when VM in starting from
suspended state, i.e. it processes incoming migration, and in the same
time 'info block' is called.

The problem is that qcow2_invalidate_cache() closes the image and
memset()s BDRVQcowState in the middle.

The patch moves processing of bdrv_invalidate_cache_all out of
coroutine context for standard migration to avoid that.

Signed-off-by: Denis V. Lunev 
CC: Paolo Bonzini 
CC: Juan Quintela 
CC: Amit Shah 
---
 migration/migration.c | 89 ---
 1 file changed, 49 insertions(+), 40 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index a64cfcd..1f8535e 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -323,13 +323,59 @@ void qemu_start_incoming_migration(const char *uri, Error 
**errp)
 }
 }
 
+static void process_incoming_migration_bh(void *opaque)
+{
+Error *local_err = NULL;
+MigrationIncomingState *mis = opaque;
+
+/* Make sure all file formats flush their mutable metadata */
+bdrv_invalidate_cache_all(_err);
+if (local_err) {
+migrate_set_state(>state, MIGRATION_STATUS_ACTIVE,
+  MIGRATION_STATUS_FAILED);
+error_report_err(local_err);
+migrate_decompress_threads_join();
+exit(EXIT_FAILURE);
+}
+
+/*
+ * This must happen after all error conditions are dealt with and
+ * we're sure the VM is going to be running on this host.
+ */
+qemu_announce_self();
+
+/* If global state section was not received or we are in running
+   state, we need to obey autostart. Any other state is set with
+   runstate_set. */
+
+if (!global_state_received() ||
+global_state_get_runstate() == RUN_STATE_RUNNING) {
+if (autostart) {
+vm_start();
+} else {
+runstate_set(RUN_STATE_PAUSED);
+}
+} else {
+runstate_set(global_state_get_runstate());
+}
+migrate_decompress_threads_join();
+/*
+ * This must happen after any state changes since as soon as an external
+ * observer sees this event they might start to prod at the VM assuming
+ * it's ready to use.
+ */
+migrate_set_state(>state, MIGRATION_STATUS_ACTIVE,
+  MIGRATION_STATUS_COMPLETED);
+migration_incoming_state_destroy();
+}
+
 static void process_incoming_migration_co(void *opaque)
 {
 QEMUFile *f = opaque;
-Error *local_err = NULL;
 MigrationIncomingState *mis;
 PostcopyState ps;
 int ret;
+QEMUBH *bh;
 
 mis = migration_incoming_state_new(f);
 postcopy_state_set(POSTCOPY_INCOMING_NONE);
@@ -369,45 +415,8 @@ static void process_incoming_migration_co(void *opaque)
 exit(EXIT_FAILURE);
 }
 
-/* Make sure all file formats flush their mutable metadata */
-bdrv_invalidate_cache_all(_err);
-if (local_err) {
-migrate_set_state(>state, MIGRATION_STATUS_ACTIVE,
-  MIGRATION_STATUS_FAILED);
-error_report_err(local_err);
-migrate_decompress_threads_join();
-exit(EXIT_FAILURE);
-}
-
-/*
- * This must happen after all error conditions are dealt with and
- * we're sure the VM is going to be running on this host.
- */
-qemu_announce_self();
-
-/* If global state section was not received or we are in running
-   state, we need to obey autostart. Any other state is set with
-   runstate_set. */
-
-if (!global_state_received() ||
-global_state_get_runstate() == RUN_STATE_RUNNING) {
-if (autostart) {
-vm_start();
-} else {
-runstate_set(RUN_STATE_PAUSED);
-}
-} else {
-runstate_set(global_state_get_runstate());
-}
-migrate_decompress_threads_join();
-/*
- * This must happen after any state changes since as soon as an external
- * observer sees this event they might start to prod at the VM assuming
- * it's ready to use.
- */
-migrate_set_state(>state, MIGRATION_STATUS_ACTIVE,
-  MIGRATION_STATUS_COMPLETED);
-migration_incoming_state_destroy();
+bh = qemu_bh_new(process_incoming_migration_bh, mis);
+qemu_bh_schedule(bh);
 }
 
 void process_incoming_migration(QEMUFile *f)
-- 
2.1.4




Re: [Qemu-devel] [PATCH v10] spec: add qcow2 bitmaps extension specification

2016-02-11 Thread Denis V. Lunev

On 02/05/2016 11:58 AM, Vladimir Sementsov-Ogievskiy wrote:

The new feature for qcow2: storing bitmaps.

This patch adds new header extension to qcow2 - Bitmaps Extension. It
provides an ability to store virtual disk related bitmaps in a qcow2
image. For now there is only one type of such bitmaps: Dirty Tracking
Bitmap, which just tracks virtual disk changes from some moment.

Note: Only bitmaps, relative to the virtual disk, stored in qcow2 file,
should be stored in this qcow2 file. The size of each bitmap
(considering its granularity) is equal to virtual disk size.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---

v10
 - rewordings, thanks to Fam
 - fix:
Given an offset byte_nr into the virtual disk and the bitmap's granularity, the
-bit offset into the bitmap can be calculated like this:
+bit offset into the image file to the corresponding bit of the bitmap can be
+calculated like this:
 - remove last sentence about 'auto' flasg consistency as superfluous.


v9
 - rewordings, thanks to Max

v8
 - rewordings
 - bitmap_directory_size: 4b -> 8b
 - add more descriptive description in == Bitmaps == section
 - add paragraph "Dirty tracking bitmaps"

   Bitmap directory entry:
 - extra data should not allocate additional clusters
 - padding must be all-bytes-zero
 - add extra_data_compatible flag (now behavior in case of unknown
   extra data is defined by this flag)

v7:

- Rewordings, grammar.
   Max, Eric, John, thank you very much.

- add last paragraph: remaining bits in bitmap data clusters must be
   zero.

- s/Bitmap Directory/bitmap directory/ and other names like this at
   the request of Max.

v6:

- reword bitmap_directory_size description
- bitmap type: make 0 reserved
- extra_data_size: resize to 4bytes
   Also, I've marked this field as "must be zero". We can always change
   it, if we decide allowing managing app to specify any extra data, by
   defining some magic value as a top of user extra data.. So, for now
   non zeor extra_data_size should be considered as an error.
- swap name and extra_data to give good alignment to extra_data.


v5:

- 'Dirty bitmaps' renamed to 'Bitmaps', as we may have several types of
   bitmaps.
- rewordings
- move upper bounds to "Notes about Qemu limits"
- s/should/must somewhere. (but not everywhere)
- move name_size field closer to name itself in bitmap header
- add extra data area to bitmap header
- move bitmap data description to separate section

  docs/specs/qcow2.txt | 221 ++-
  1 file changed, 220 insertions(+), 1 deletion(-)

diff --git a/docs/specs/qcow2.txt b/docs/specs/qcow2.txt
index f236d8c..80cdfd0 100644
--- a/docs/specs/qcow2.txt
+++ b/docs/specs/qcow2.txt
@@ -103,7 +103,18 @@ in the description of a field.
  write to an image with unknown auto-clear features if it
  clears the respective bits from this field first.
  
-Bits 0-63:  Reserved (set to 0)

+Bit 0:  Bitmaps extension bit
+This bit indicates consistency for the bitmaps
+extension data.
+
+It is an error if this bit is set without the
+bitmaps extension present.
+
+If the bitmaps extension is present but this
+bit is unset, the bitmaps extension data must 
be
+considered inconsistent.
+
+Bits 1-63:  Reserved (set to 0)
  
   96 -  99:  refcount_order

  Describes the width of a reference count block entry 
(width
@@ -123,6 +134,7 @@ be stored. Each extension has a structure like the 
following:
  0x - End of the header extension area
  0xE2792ACA - Backing file format name
  0x6803f857 - Feature name table
+0x23852875 - Bitmaps extension
  other  - Unknown header extension, can be safely
   ignored
  
@@ -166,6 +178,36 @@ the header extension data. Each entry look like this:

  terminated if it has full length)
  
  
+== Bitmaps extension ==

+
+The bitmaps extension is an optional header extension. It provides the ability
+to store bitmaps related to a virtual disk. For now, there is only one bitmap
+type: the dirty tracking bitmap, which tracks virtual disk changes from some
+point in time.
+
+The data of the extension should be considered consistent only if the
+corresponding auto-clear feature bit is set, see autoclear_features above.
+
+The fields of the bitmaps extension are:
+
+Byte  0 -  3:  nb_bitmaps
+   The number of bitmaps contained in the image. Must be
+   greater than or equal 

[Qemu-devel] [PATCH 2/4] target-arm: Move get/set_r13_banked() to op_helper.c

2016-02-11 Thread Peter Maydell
Move get/set_r13_banked() from helper.c to op_helper.c. This will
let us add exception-raising code to them, and also puts them
in the same file as get/set_user_reg(), which makes some conceptual
sense.

(The original reason for the helper.c/op_helper.c split was that
only op_helper.c had access to the CPU env pointer; this distinction
has not been true for a long time, though, and so the split is
now rather arbitrary.)

Signed-off-by: Peter Maydell 
---
 target-arm/helper.c| 33 -
 target-arm/op_helper.c | 37 +
 2 files changed, 37 insertions(+), 33 deletions(-)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index bb913c6..c46e3d0 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -5365,21 +5365,6 @@ void switch_mode(CPUARMState *env, int mode)
 }
 }
 
-void HELPER(set_r13_banked)(CPUARMState *env, uint32_t mode, uint32_t val)
-{
-ARMCPU *cpu = arm_env_get_cpu(env);
-
-cpu_abort(CPU(cpu), "banked r13 write\n");
-}
-
-uint32_t HELPER(get_r13_banked)(CPUARMState *env, uint32_t mode)
-{
-ARMCPU *cpu = arm_env_get_cpu(env);
-
-cpu_abort(CPU(cpu), "banked r13 read\n");
-return 0;
-}
-
 uint32_t arm_phys_excp_target_el(CPUState *cs, uint32_t excp_idx,
  uint32_t cur_el, bool secure)
 {
@@ -7762,24 +7747,6 @@ hwaddr arm_cpu_get_phys_page_attrs_debug(CPUState *cs, 
vaddr addr,
 return phys_addr;
 }
 
-void HELPER(set_r13_banked)(CPUARMState *env, uint32_t mode, uint32_t val)
-{
-if ((env->uncached_cpsr & CPSR_M) == mode) {
-env->regs[13] = val;
-} else {
-env->banked_r13[bank_number(mode)] = val;
-}
-}
-
-uint32_t HELPER(get_r13_banked)(CPUARMState *env, uint32_t mode)
-{
-if ((env->uncached_cpsr & CPSR_M) == mode) {
-return env->regs[13];
-} else {
-return env->banked_r13[bank_number(mode)];
-}
-}
-
 uint32_t HELPER(v7m_mrs)(CPUARMState *env, uint32_t reg)
 {
 ARMCPU *cpu = arm_env_get_cpu(env);
diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index 049b521..053e9b6 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -457,6 +457,43 @@ void HELPER(set_user_reg)(CPUARMState *env, uint32_t 
regno, uint32_t val)
 }
 }
 
+#if defined(CONFIG_USER_ONLY)
+void HELPER(set_r13_banked)(CPUARMState *env, uint32_t mode, uint32_t val)
+{
+ARMCPU *cpu = arm_env_get_cpu(env);
+
+cpu_abort(CPU(cpu), "banked r13 write\n");
+}
+
+uint32_t HELPER(get_r13_banked)(CPUARMState *env, uint32_t mode)
+{
+ARMCPU *cpu = arm_env_get_cpu(env);
+
+cpu_abort(CPU(cpu), "banked r13 read\n");
+return 0;
+}
+
+#else
+
+void HELPER(set_r13_banked)(CPUARMState *env, uint32_t mode, uint32_t val)
+{
+if ((env->uncached_cpsr & CPSR_M) == mode) {
+env->regs[13] = val;
+} else {
+env->banked_r13[bank_number(mode)] = val;
+}
+}
+
+uint32_t HELPER(get_r13_banked)(CPUARMState *env, uint32_t mode)
+{
+if ((env->uncached_cpsr & CPSR_M) == mode) {
+return env->regs[13];
+} else {
+return env->banked_r13[bank_number(mode)];
+}
+}
+#endif
+
 void HELPER(access_check_cp_reg)(CPUARMState *env, void *rip, uint32_t 
syndrome,
  uint32_t isread)
 {
-- 
1.9.1




Re: [Qemu-devel] [PATCHv7 9/9] qapi-schema, qemu-options & slirp: Adding Qemu options for IPv6 addresses

2016-02-11 Thread Thomas Huth
On 10.02.2016 14:08, Daniel P. Berrange wrote:
> On Wed, Feb 10, 2016 at 01:45:22PM +0100, Samuel Thibault wrote:
>> Thomas Huth, on Wed 10 Feb 2016 12:39:10 +0100, wrote:
 +if (!vprefix6) {
 +vprefix6 = "fec0::";
>>>
>>> Site-local prefixes have already been deprecated (see rfc3879) ... would
>>> it be feasible to use a ULA prefix instead (fd00::/8,
>>> see rfc4193) ?
>>
>> The question is which ULA. Ideally we'd take a random one at each qemu
>> startup, but then it's a pain for users to type IPs by hand, all the
>> more so when it changes at each qemu startup. Another way is to have the
>> same in all qemu instances, hardcoded in qemu, i.e. like
>>
>> https://xkcd.com/221/
>>
>> proposes. That's still a pain to type, even if it is always the same,
>> and can still (since it's the same for all qemu instances) pose some of
>> the problems raised by rfc3879. The rfc1918 addresses we use in qemu
>> for ipv4 have the same issues. That's why I considered that the issues
>> mentioned by rfc3879 would not be relevant to qemu, and be simpler to
>> just use fec0::, and let the user chose his public or ULA prefix if he
>> needs it.
> 
> I'm inclined to agree that fec0:: is a better bet for QEMU's default
> usage, despite rfc3879. As you say it is no worse than what we have
> with IPv4, and IMHO it is preferrable to using a fixed ULA since that
> would be non-compliant with the RFC which requires randomness.

Ok, then let's go with fec0:: first. We still can change it later if
necessary.

 Thomas




[Qemu-devel] [PATCH 1/4] target-arm: Clean up trap/undef handling of SRS

2016-02-11 Thread Peter Maydell
The SRS instruction is:
 * UNDEFINED in Hyp mode
 * UNPREDICTABLE in User or System mode
 * UNPREDICTABLE if the specified mode isn't accessible
 * trapped to EL3 if EL3 is AArch64 and we are at Secure EL1

Clean up the code to handle all these cases cleanly, including
picking UNDEF as our choice of UNPREDICTABLE behaviour rather
blindly trusting the mode field passed in the instruction.
As part of this, move the check for IS_USER into gen_srs()
itself rather than having it done by the caller.

The exception is that we don't UNDEF for calls from System
mode, which need a runtime check. This will be dealt with in
the following commits.

Signed-off-by: Peter Maydell 
---
 target-arm/translate.c | 66 ++
 1 file changed, 61 insertions(+), 5 deletions(-)

diff --git a/target-arm/translate.c b/target-arm/translate.c
index cf3dc33..7bceb05 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -7578,8 +7578,67 @@ static void gen_srs(DisasContext *s,
 uint32_t mode, uint32_t amode, bool writeback)
 {
 int32_t offset;
-TCGv_i32 addr = tcg_temp_new_i32();
-TCGv_i32 tmp = tcg_const_i32(mode);
+TCGv_i32 addr, tmp;
+bool undef = false;
+
+/* SRS is:
+ * - trapped to EL3 if EL3 is AArch64 and we are at Secure EL1
+ * - UNDEFINED in Hyp mode
+ * - UNPREDICTABLE in User or System mode
+ * - UNPREDICTABLE if the specified mode is:
+ * -- not implemented
+ * -- not a valid mode number
+ * -- a mode that's at a higher exception level
+ * -- Monitor, if we are Non-secure
+ * For the UNPREDICTABLE cases we choose to UNDEF, except that for
+ * "current mode is System" we will write a garbage SPSR.
+ * (This is because we don't have access to our current mode here
+ * and would have to do a runtime check to UNDEF for System.)
+ */
+if (s->current_el == 1 && !s->ns) {
+gen_exception_insn(s, 4, EXCP_UDEF, syn_uncategorized(), 3);
+return;
+}
+
+if (s->current_el == 0 || s->current_el == 2) {
+undef = true;
+}
+
+switch (mode) {
+case ARM_CPU_MODE_USR:
+case ARM_CPU_MODE_FIQ:
+case ARM_CPU_MODE_IRQ:
+case ARM_CPU_MODE_SVC:
+case ARM_CPU_MODE_ABT:
+case ARM_CPU_MODE_UND:
+case ARM_CPU_MODE_SYS:
+break;
+case ARM_CPU_MODE_HYP:
+if (s->current_el == 1 || !arm_dc_feature(s, ARM_FEATURE_EL2)) {
+undef = true;
+}
+break;
+case ARM_CPU_MODE_MON:
+/* No need to check specifically for "are we non-secure" because
+ * we've already made EL0 UNDEF and handled the trap for S-EL1;
+ * so if this isn't EL3 then we must be non-secure.
+ */
+if (s->current_el != 3) {
+undef = true;
+}
+break;
+default:
+undef = true;
+}
+
+if (undef) {
+gen_exception_insn(s, 4, EXCP_UDEF, syn_uncategorized(),
+   default_exception_el(s));
+return;
+}
+
+addr = tcg_temp_new_i32();
+tmp = tcg_const_i32(mode);
 gen_helper_get_r13_banked(addr, cpu_env, tmp);
 tcg_temp_free_i32(tmp);
 switch (amode) {
@@ -7739,9 +7798,6 @@ static void disas_arm_insn(DisasContext *s, unsigned int 
insn)
 }
 } else if ((insn & 0x0e5fffe0) == 0x084d0500) {
 /* srs */
-if (IS_USER(s)) {
-goto illegal_op;
-}
 ARCH(6);
 gen_srs(s, (insn & 0x1f), (insn >> 23) & 3, insn & (1 << 21));
 return;
-- 
1.9.1




[Qemu-devel] [PATCH 0/4] target-arm: Clean up trap/undef handling of SRS

2016-02-11 Thread Peter Maydell
The SRS instruction is a bit of an oddity because it isn't
used by Linux these days. Nonetheless it has a bunch of
UNPREDICTABLE, UNDEF and trapping behaviour that we weren't
correctly implementing:

 - trap to EL3 if EL3 is AArch64 and we are at Secure EL1
 - UNDEFINED in Hyp mode
 - UNPREDICTABLE in User or System mode
 - UNPREDICTABLE if the specified mode is:
 -- not implemented
 -- not a valid mode number
 -- a mode that's at a higher exception level
 -- Monitor, if we are Non-secure

This series implements the checks we were missing and makes
us UNDEF for all the UNPREDICTABLE cases.

Patch 1 does the easy checks that can be done at translate time;
patches 2 and 3 are code motion in preparation for patch 4, which
puts in a run-time check for the one awkward case we don't have
enough information to UNDEF at translate time.

thanks
-- PMM

Peter Maydell (4):
  target-arm: Clean up trap/undef handling of SRS
  target-arm: Move get/set_r13_banked() to op_helper.c
  target-arm: Combine user-only and softmmu get/set_r13_banked()
  target-arm: UNDEF in the UNPREDICTABLE SRS-from-System case

 target-arm/helper.c| 33 -
 target-arm/op_helper.c | 32 
 target-arm/translate.c | 67 ++
 3 files changed, 94 insertions(+), 38 deletions(-)

-- 
1.9.1




Re: [Qemu-devel] [PATCH V4] vl.c: fixed regression in machine error message

2016-02-11 Thread Eduardo Habkost
On Thu, Feb 11, 2016 at 08:49:25PM +0200, Marcel Apfelbaum wrote:
> On 02/11/2016 08:31 PM, Eduardo Habkost wrote:
> >On Mon, Feb 08, 2016 at 01:54:29PM +0200, Marcel Apfelbaum wrote:
> >>Commit e1ce0c3cb(vl.c: fix regression when reading machine type from config 
> >>file)
> >>fixed the error message when the machine type was supplied inside the
> >>config file. However now the option name is not displayed correctly if
> >>the error happens when the machine is specified at command line.
> >>
> >>Running
> >> ./x86_64-softmmu/qemu-system-x86_64 -M q35-1.5 -redir tcp:8022::22
> >>will result in the error message:
> >> qemu-system-x86_64: -redir tcp:8022::22: unsupported machine type
> >> Use -machine help to list supported machines
> >>
> >>Fixed it by restoring the error location and also extracted the code
> >>dealing with machine options into a separate function.
> >>
> >>Reported-by: Michael S. Tsirkin 
> >>Reviewed-by: Laszlo Ersek 
> >>Signed-off-by: Marcel Apfelbaum 
> >>---
> >>
> >>v3 -> v4:
> >>- removed double call of loc_push_none (thanks again Laszlo)
> >>
> >>v2 -> v3:
> >>  - fixed commit message and called qemu_get_machine_opts only once. 
> >> (thanks Laszlo)
> >>
> >>v1 -> v2:
> >>  - Addressed Laszlo Ersek's comments:
> >>- no need to save the machine options location, is saved in opts
> >>- rename the extracted method to set_machine_options
> >>- added the bug reporter to the CC
> >>
> >>  - tested with and without the config file and the error message is now OK:
> >>  config file:
> >> - qemu-system-x86_64:machine-bug.conf:3: unsupported machine type
> >>  cli:
> >>- qemu-system-x86_64: -M q35-1.5: unsupported machine type
> >>
> >>Thanks,
> >>Marcel
> >>
> >>  vl.c | 38 +++---
> >>  1 file changed, 27 insertions(+), 11 deletions(-)
> >>
> >>diff --git a/vl.c b/vl.c
> >>index 2c03f54..5e22a35 100644
> >>--- a/vl.c
> >>+++ b/vl.c
> >>@@ -2748,6 +2748,31 @@ static const QEMUOption *lookup_opt(int argc, char 
> >>**argv,
> >>  return popt;
> >>  }
> >>
> >>+static void set_machine_options(MachineClass **machine_class)
> >>+{
> >>+const char *optarg;
> >>+QemuOpts *opts;
> >>+Location loc;
> >>+
> >>+loc_push_none();
> >>+
> >>+opts = qemu_get_machine_opts();
> >>+qemu_opts_loc_restore(opts);
> >>+
> >>+optarg = qemu_opt_get(opts, "type");
> >>+if (optarg) {
> >>+*machine_class = machine_parse(optarg);
> >>+}
> >>+
> >>+if (*machine_class == NULL) {
> >>+error_report("No machine specified, and there is no default");
> >>+error_printf("Use -machine help to list supported machines\n");
> >>+exit(1);
> >>+}
> >>+
> >>+loc_pop();
> >>+}
> >>+
> >>  static int machine_set_property(void *opaque,
> >>  const char *name, const char *value,
> >>  Error **errp)
> >>@@ -4028,17 +4053,7 @@ int main(int argc, char **argv, char **envp)
> >>
> >>  replay_configure(icount_opts);
> >>
> >>-opts = qemu_get_machine_opts();
> >>-optarg = qemu_opt_get(opts, "type");
> >>-if (optarg) {
> >>-machine_class = machine_parse(optarg);
> >>-}
> >>-
> >>-if (machine_class == NULL) {
> >>-error_report("No machine specified, and there is no default");
> >>-error_printf("Use -machine help to list supported machines\n");
> >>-exit(1);
> >>-}
> >>+set_machine_options(_class);
> >>
> >>  set_memory_options(_slots, _size, machine_class);
> >[Extra context for reference:]
> >>
> >>loc_set_none();
> >
> >You are fixing the machine error message, but not the root cause:
> 
> Hi,
> 
> I knew that it would not solve all the cases but
> the machine error message is a regression while other
> were there way before (as far as I know)
> 
> I was specifically looking to fix the machine error regression.
> 
> >
> >   $ ./x86_64-softmmu/qemu-system-x86_64 -m size= -vnc :0
> >   qemu-system-x86_64: -vnc :0: missing 'size' option value
> >   $ ./x86_64-softmmu/qemu-system-x86_64 -icount rr=x -vnc :0
> >   qemu-system-x86_64: -vnc :0: Invalid icount rr option: x
> >
> >Moving the loc_set_none() call above replay_configure() should fix the bug in
> >the three cases.
> 
> I am really no expert in loc stuff, but it seamed to me it is placed
> especially after all the options are parsed. Maybe we need another
> call to loc_set_none before replay_configure instead of moving it?

It needs to be reset immediately after the option-parsing 'for'
loop, because the location info becomes obsolete as soon as we
exit the loop. All the remaining code is supposed to push/pop
location info if necessary, so we don't need another call.

> 
> If you think it should be moved, please go ahead :)
> is an one liner and the idea was yours.

I will do that.

> 
> >
> >Setting location in set_machine_options()
> 
> This is done in 

[Qemu-devel] [Bug 1098729] Re: qemu-user-static for armhf: segfault in threaded code

2016-02-11 Thread Andrea Mazzoleni
I can confirm that building QEMU 2.5.0 from source, all the multi-thread
issues seem to be fixed.

Specifically, the mentioned dotprod_mutex.c example, even when modified
to use 100 threads, is always running in the qemu-arm User mode
emulator.

Tested in Ubuntu 14.04 x86_64, with all the updates installed.

Note that instead the QEMU 2.0.0 from the Ubuntu 14.04 repository is
having issues even when using workarounds like running it with "taskset
0x1" to force the execution to a single CPU.

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

Title:
  qemu-user-static for armhf: segfault in threaded code

Status in QEMU:
  Confirmed

Bug description:
  
  Currently running QEMU from git (fedf2de31023) and running the armhf version 
of qemu-user-static which I have renamed qemu-armhf-static to follow the naming 
convention used in Debian.

  The host systems is a Debian testing x86_64-linux and I have an Debian
  testing armhf chroot which I invoke using schroot.

  Majority of program in the armhf chroot run fine, but I'm getting qemu
  segfaults in multi-threaded programs.

  As an example, I've grabbed the threads demo program here:

  https://computing.llnl.gov/tutorials/pthreads/samples/dotprod_mutex.c

  and changed NUMTHRDS from 4 to 10. I compile it as (same compile
  command on both x86_64 host and armhf guest):

  gcc -Wall -lpthread dotprod_mutex.c -o dotprod_mutex

  When compiled for x86_64 host it runs perfectly and even under
  Valgrind displays no errors whatsoever.

  However, when I compile the program in my armhs chroot and run it it
  usually (but not always) segaults or hangs or crashes. Example output:

  
  (armhf) $ ./dotprod_mutex
  Thread 1 did 10 to 20:  mysum=10.00 global 
sum=10.00
  Thread 0 did 0 to 10:  mysum=10.00 global sum=20.00
  TCG temporary leak before f6731ca0
  qemu-arm-static: 
/home/erikd/Git/qemu-posix-timer-hacking/Upstream/tcg/tcg-op.h:2371:
  tcg_gen_goto_tb: Assertion `(tcg_ctx.goto_tb_issue_mask & (1 << idx)) == 
0' failed.

  
  (armhf) $ ./dotprod_mutex
  qemu: uncaught target signal 11 (Segmentation fault) - core dumped
  Segmentation fault

  (armhf) $ ./dotprod_mutex
  qemu-arm-static: 
/home/erikd/Git/qemu-posix-timer-hacking/Upstream/tcg/tcg.c:519:
  tcg_temp_free_internal: Assertion `idx >= s->nb_globals && idx < 
s->nb_temps' failed.

  
  (armhf) $ ./dotprod_mutex
  Thread 1 did 10 to 20:  mysum=10.00 global 
sum=10.00
  qemu: uncaught target signal 11 (Segmentation fault) - core dumped
  Segmentation fault

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



[Qemu-devel] [PATCH 4/4] target-arm: UNDEF in the UNPREDICTABLE SRS-from-System case

2016-02-11 Thread Peter Maydell
Make get_r13_banked() raise an exception at runtime for the
corner case of SRS from System mode, so that we can UNDEF it;
this brings us in to line with the ARM ARM's set of permitted
CONSTRAINED UNPREDICTABLE choices.

Signed-off-by: Peter Maydell 
---
 target-arm/op_helper.c | 8 
 target-arm/translate.c | 9 +
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index 05f97a7..8183108 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -474,6 +474,14 @@ uint32_t HELPER(get_r13_banked)(CPUARMState *env, uint32_t 
mode)
 #if defined(CONFIG_USER_ONLY)
 g_assert_not_reached();
 #endif
+if ((env->uncached_cpsr & CPSR_M) == ARM_CPU_MODE_SYS) {
+/* SRS instruction is UNPREDICTABLE from System mode; we UNDEF.
+ * Other UNPREDICTABLE and UNDEF cases were caught at translate time.
+ */
+raise_exception(env, EXCP_UDEF, syn_uncategorized(),
+exception_target_el(env));
+}
+
 if ((env->uncached_cpsr & CPSR_M) == mode) {
 return env->regs[13];
 } else {
diff --git a/target-arm/translate.c b/target-arm/translate.c
index 7bceb05..e69145d 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -7590,10 +7590,7 @@ static void gen_srs(DisasContext *s,
  * -- not a valid mode number
  * -- a mode that's at a higher exception level
  * -- Monitor, if we are Non-secure
- * For the UNPREDICTABLE cases we choose to UNDEF, except that for
- * "current mode is System" we will write a garbage SPSR.
- * (This is because we don't have access to our current mode here
- * and would have to do a runtime check to UNDEF for System.)
+ * For the UNPREDICTABLE cases we choose to UNDEF.
  */
 if (s->current_el == 1 && !s->ns) {
 gen_exception_insn(s, 4, EXCP_UDEF, syn_uncategorized(), 3);
@@ -7639,6 +7636,9 @@ static void gen_srs(DisasContext *s,
 
 addr = tcg_temp_new_i32();
 tmp = tcg_const_i32(mode);
+/* get_r13_banked() will raise an exception if called from System mode */
+gen_set_condexec(s);
+gen_set_pc_im(s, s->pc - 4);
 gen_helper_get_r13_banked(addr, cpu_env, tmp);
 tcg_temp_free_i32(tmp);
 switch (amode) {
@@ -7688,6 +7688,7 @@ static void gen_srs(DisasContext *s,
 tcg_temp_free_i32(tmp);
 }
 tcg_temp_free_i32(addr);
+s->is_jmp = DISAS_UPDATE;
 }
 
 static void disas_arm_insn(DisasContext *s, unsigned int insn)
-- 
1.9.1




[Qemu-devel] [PATCH 3/4] target-arm: Combine user-only and softmmu get/set_r13_banked()

2016-02-11 Thread Peter Maydell
The user-mode versions of get/set_r13_banked() exist just to assert
if they're ever called -- the translate time code should never
emit calls to them because SRS from user mode always UNDEF.
There's no code in the softmmu versions that can't compile in
CONFIG_USER_ONLY, so combine the two functions rather than
having completely split versions under ifdefs.

Signed-off-by: Peter Maydell 
---
 target-arm/op_helper.c | 25 ++---
 1 file changed, 6 insertions(+), 19 deletions(-)

diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index 053e9b6..05f97a7 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -457,26 +457,11 @@ void HELPER(set_user_reg)(CPUARMState *env, uint32_t 
regno, uint32_t val)
 }
 }
 
-#if defined(CONFIG_USER_ONLY)
-void HELPER(set_r13_banked)(CPUARMState *env, uint32_t mode, uint32_t val)
-{
-ARMCPU *cpu = arm_env_get_cpu(env);
-
-cpu_abort(CPU(cpu), "banked r13 write\n");
-}
-
-uint32_t HELPER(get_r13_banked)(CPUARMState *env, uint32_t mode)
-{
-ARMCPU *cpu = arm_env_get_cpu(env);
-
-cpu_abort(CPU(cpu), "banked r13 read\n");
-return 0;
-}
-
-#else
-
 void HELPER(set_r13_banked)(CPUARMState *env, uint32_t mode, uint32_t val)
 {
+#if defined(CONFIG_USER_ONLY)
+g_assert_not_reached();
+#endif
 if ((env->uncached_cpsr & CPSR_M) == mode) {
 env->regs[13] = val;
 } else {
@@ -486,13 +471,15 @@ void HELPER(set_r13_banked)(CPUARMState *env, uint32_t 
mode, uint32_t val)
 
 uint32_t HELPER(get_r13_banked)(CPUARMState *env, uint32_t mode)
 {
+#if defined(CONFIG_USER_ONLY)
+g_assert_not_reached();
+#endif
 if ((env->uncached_cpsr & CPSR_M) == mode) {
 return env->regs[13];
 } else {
 return env->banked_r13[bank_number(mode)];
 }
 }
-#endif
 
 void HELPER(access_check_cp_reg)(CPUARMState *env, void *rip, uint32_t 
syndrome,
  uint32_t isread)
-- 
1.9.1




[Qemu-devel] [PATCH 1/1] hyperv: cpu hotplug fix with HyperV enabled

2016-02-11 Thread Denis V. Lunev
From: "Alexey V. Kostyushko" 

With Hyper-V enabled CPU hotplug stops working. The CPU appears in device
manager on Windows but does not appear in peformance monitor and control
panel.

The root of the problem is the following. Windows checks
HV_X64_CPU_DYNAMIC_PARTITIONING_AVAILABLE bit in CPUID. The presence of
this bit is enough to cure the situation.

Add option 'hv-cpuhotplug' to control this behavior.

Signed-off-by: Alexey V. Kostyushko 
Signed-off-by: Denis V. Lunev 
CC: Paolo Bonzini 
CC: Richard Henderson 
CC: Eduardo Habkost 
CC: "Andreas Färber" 
---
 target-i386/cpu-qom.h | 1 +
 target-i386/cpu.c | 1 +
 target-i386/kvm.c | 6 +-
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
index 5f9d960..4aec616 100644
--- a/target-i386/cpu-qom.h
+++ b/target-i386/cpu-qom.h
@@ -96,6 +96,7 @@ typedef struct X86CPU {
 bool hyperv_runtime;
 bool hyperv_synic;
 bool hyperv_stimer;
+bool hyperv_cpuhotplug;
 bool check_cpuid;
 bool enforce_cpuid;
 bool expose_kvm;
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index b255644..32c38ae 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -3172,6 +3172,7 @@ static Property x86_cpu_properties[] = {
 DEFINE_PROP_BOOL("hv-runtime", X86CPU, hyperv_runtime, false),
 DEFINE_PROP_BOOL("hv-synic", X86CPU, hyperv_synic, false),
 DEFINE_PROP_BOOL("hv-stimer", X86CPU, hyperv_stimer, false),
+DEFINE_PROP_BOOL("hv-cpuhotplug", X86CPU, hyperv_cpuhotplug, false),
 DEFINE_PROP_BOOL("check", X86CPU, check_cpuid, true),
 DEFINE_PROP_BOOL("enforce", X86CPU, enforce_cpuid, false),
 DEFINE_PROP_BOOL("kvm", X86CPU, expose_kvm, true),
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 94024bc..f4692b9 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -529,7 +529,8 @@ static bool hyperv_enabled(X86CPU *cpu)
 cpu->hyperv_vpindex ||
 cpu->hyperv_runtime ||
 cpu->hyperv_synic ||
-cpu->hyperv_stimer);
+cpu->hyperv_stimer ||
+cpu->hyperv_cpuhotplug);
 }
 
 static int kvm_arch_set_tsc_khz(CPUState *cs)
@@ -636,6 +637,9 @@ int kvm_arch_init_vcpu(CPUState *cs)
 c->eax |= 0x200;
 has_msr_hv_tsc = true;
 }
+if (cpu->hyperv_cpuhotplug) {
+c->edx |= HV_X64_CPU_DYNAMIC_PARTITIONING_AVAILABLE;
+}
 if (cpu->hyperv_crash && has_msr_hv_crash) {
 c->edx |= HV_X64_GUEST_CRASH_MSR_AVAILABLE;
 }
-- 
2.1.4




Re: [Qemu-devel] qdev & hw/core owner? (was Re: [PATCH v19 7/9] machine: add properties to compat_props incrementaly)

2016-02-11 Thread Eduardo Habkost
On Fri, Feb 05, 2016 at 09:51:07AM +0200, Marcel Apfelbaum wrote:
> On 02/05/2016 09:49 AM, Markus Armbruster wrote:
> >"Michael S. Tsirkin"  writes:
> >
> >>On Thu, Feb 04, 2016 at 12:55:22PM +0100, Paolo Bonzini wrote:
> >>>
> >>>
> >>>On 04/02/2016 12:41, Andreas Färber wrote:
> You're talking about machine, right? Some time ago I had proposed Marcel
> who initially worked on it, but I'm fine with anyone taking it.
> >>>
> >>>Yes.
> >>>
> For some (but not all) core qdev parts related to the (stalled) QOM
> migration I've been taking care of via qom-next. Last time this came up
> you didn't want anyone to be M: for qdev, so maybe we can use R: so that
> at least people automatically get CC'ed and we avoid this recurring
> discussion?
> >>>
> >>>I might have changed my mind on that.  You definitely should be M: for 
> >>>qdev.
> >>>
> >>>Paolo
> >>
> >>If Andreas wants to, that's also fine. Several maintainers are
> >>better than one.
> >
> >*If* the maintainers are all willing and able to work together.
> >
> 
> No problem here from my point of view :)

No problem to me, too. :)

I am going to be away from work for 15 days starting on Tuesday
Feb 16th. So if Marcel wants to start queueing patches already,
please be my guest. I will be able to help on that after I'm
back.

-- 
Eduardo



[Qemu-devel] [PATCH v6 2/9] qemu-log: correct help text for -d cpu

2016-02-11 Thread Alex Bennée
This doesn't just dump CPU state on translation but on every block
entrance.

Signed-off-by: Alex Bennée 
Reviewed-by: Andreas Färber 
Reviewed-by: Richard Henderson 

---
v4
  - add r-b tag
v5
  - slightly tweak the wording now nochain exists
v6
  - add rth r-b tag
---
 util/log.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/util/log.c b/util/log.c
index 2709e98..c39fa7f 100644
--- a/util/log.c
+++ b/util/log.c
@@ -110,7 +110,7 @@ const QEMULogItem qemu_log_items[] = {
 { CPU_LOG_EXEC, "exec",
   "show trace before each executed TB (lots of logs)" },
 { CPU_LOG_TB_CPU, "cpu",
-  "show CPU state before block translation" },
+  "show CPU registers before entering a TB (lots of logs)" },
 { CPU_LOG_MMU, "mmu",
   "log MMU-related activities" },
 { CPU_LOG_PCALL, "pcall",
-- 
2.7.0




[Qemu-devel] [PATCH v6 0/9] qemu-log, -dfilter and other logging tweaks

2016-02-11 Thread Alex Bennée
Mostly this is version has been collection r-b tags but the
re-factoring patch is now even simpler. The most attention has been on
the -dfilter option patch because handling user-input is hard. I've
removed the funky corner cases from trying to be clever and added some
unit tests to defend it.

Alex Bennée (7):
  tcg: pass down TranslationBlock to tcg_code_gen
  qemu-log: correct help text for -d cpu
  qemu-log: support simple pid substitution in logfile
  qemu-log: new option -dfilter to limit output
  qemu-log: dfilter-ise exec, out_asm, op and opt_op
  target-arm: dfilter support for in_asm
  cputlb: modernise the debug support

Peter Maydell (2):
  qemu-log: Avoid function call for disabled qemu_log_mask logging
  qemu-log: Improve the "exec" TB execution logging

 cpu-exec.c |  21 +
 cputlb.c   |  54 +
 include/exec/exec-all.h|   5 ++
 include/qemu/log.h |  28 ++-
 qemu-options.hx|  18 +++
 target-arm/translate-a64.c |   3 +-
 target-arm/translate.c |   3 +-
 tcg/tcg.c  |  12 +++--
 tcg/tcg.h  |   2 +-
 tests/Makefile |   4 ++
 tests/test-logging.c   |  75 +
 translate-all.c|   5 +-
 util/log.c | 115 +++--
 vl.c   |   3 ++
 14 files changed, 294 insertions(+), 54 deletions(-)
 create mode 100644 tests/test-logging.c

-- 
2.7.0




[Qemu-devel] [PATCH v6 3/9] qemu-log: Avoid function call for disabled qemu_log_mask logging

2016-02-11 Thread Alex Bennée
From: Peter Maydell 

Make qemu_log_mask() a macro which only calls the function to
do the actual work if the logging is enabled. This avoids making
a function call in possible fast paths where logging is disabled.

Signed-off-by: Peter Maydell 
Signed-off-by: Alex Bennée 
Reviewed-by: Andreas Färber 

---
v4
  - fix s-o-b tags, add r-b tag
---
 include/qemu/log.h | 13 ++---
 util/log.c | 11 ---
 2 files changed, 10 insertions(+), 14 deletions(-)

diff --git a/include/qemu/log.h b/include/qemu/log.h
index 30817f7..22c4dec 100644
--- a/include/qemu/log.h
+++ b/include/qemu/log.h
@@ -70,10 +70,17 @@ qemu_log_vprintf(const char *fmt, va_list va)
 }
 }
 
-/* log only if a bit is set on the current loglevel mask
+/* log only if a bit is set on the current loglevel mask:
+ * @mask: bit to check in the mask
+ * @fmt: printf-style format string
+ * @args: optional arguments for format string
  */
-void GCC_FMT_ATTR(2, 3) qemu_log_mask(int mask, const char *fmt, ...);
-
+#define qemu_log_mask(MASK, FMT, ...)   \
+do {\
+if (unlikely(qemu_loglevel_mask(MASK))) {   \
+qemu_log(FMT, ## __VA_ARGS__);  \
+}   \
+} while (0)
 
 /* Maintenance: */
 
diff --git a/util/log.c b/util/log.c
index c39fa7f..c89b226 100644
--- a/util/log.c
+++ b/util/log.c
@@ -38,17 +38,6 @@ void qemu_log(const char *fmt, ...)
 va_end(ap);
 }
 
-void qemu_log_mask(int mask, const char *fmt, ...)
-{
-va_list ap;
-
-va_start(ap, fmt);
-if ((qemu_loglevel & mask) && qemu_logfile) {
-vfprintf(qemu_logfile, fmt, ap);
-}
-va_end(ap);
-}
-
 /* enable or disable low levels log */
 void do_qemu_set_log(int log_flags, bool use_own_buffers)
 {
-- 
2.7.0




Re: [Qemu-devel] [PATCH 3/3] replay: introduce block devices record/replay

2016-02-11 Thread Pavel Dovgalyuk
> From: Kevin Wolf [mailto:kw...@redhat.com]
> Am 11.02.2016 um 12:00 hat Pavel Dovgalyuk geschrieben:
> > > From: Kevin Wolf [mailto:kw...@redhat.com]
> > > Am 11.02.2016 um 07:05 hat Pavel Dovgalyuk geschrieben:
> > > > > From: Kevin Wolf [mailto:kw...@redhat.com]
> > > > > Am 10.02.2016 um 13:51 hat Pavel Dovgalyuk geschrieben:
> > > > > > However, I don't understand yet which layer do you offer as the 
> > > > > > candidate
> > > > > > for record/replay? What functions should be changed?
> > > > > > I would like to investigate this way, but I don't got it yet.
> > > > >
> > > > > At the core, I wouldn't change any existing function, but introduce a
> > > > > new block driver. You could copy raw_bsd.c for a start and then tweak
> > > > > it. Leave out functions that you don't want to support, and add the
> > > > > necessary magic to .bdrv_co_readv/writev.
> > > > >
> > > > > Something like this (can probably be generalised for more than just
> > > > > reads as the part after the bdrv_co_reads() call should be the same 
> > > > > for
> > > > > reads, writes and any other request types):
> > > > >
> > > > > int blkreplay_co_readv()
> > > > > {
> > > > > BlockReplayState *s = bs->opaque;
> > > > > int reqid = s->reqid++;
> > > > >
> > > > > bdrv_co_readv(bs->file, ...);
> > > > >
> > > > > if (mode == record) {
> > > > > log(reqid, time);
> > > > > } else {
> > > > > assert(mode == replay);
> > > > > bool *done = req_replayed_list_get(reqid)
> > > > > if (done) {
> > > > > *done = true;
> > > > > } else {
> > > > > req_completed_list_insert(reqid, qemu_coroutine_self());
> > > > > qemu_coroutine_yield();
> > > > > }
> > > > > }
> > > > > }
> > > > >
> > > > > /* called by replay.c */
> > > > > int blkreplay_run_event()
> > > > > {
> > > > > if (mode == replay) {
> > > > > co = req_completed_list_get(e.reqid);
> > > > > if (co) {
> > > > > qemu_coroutine_enter(co);
> > > > > } else {
> > > > > bool done = false;
> > > > > req_replayed_list_insert(reqid, );
> > > > > /* wait synchronously for completion */
> > > > > while (!done) {
> > > > > aio_poll();
> > > > > }
> > > > > }
> > > > > }
> > > > > }
> > > > >
> > > > > Where we could consider changing existing code is that it might be
> > > > > desirable to automatically put an instance of this block driver on top
> > > > > of every block device when record/replay is used. If we don't do that,
> > > > > you need to explicitly specify -drive driver=blkreplay,...
> > > >
> > > > As far, as I understand, all synchronous read/write request are also 
> > > > passed
> > > > through this coroutines layer.
> > >
> > > Yes, all read/write requests go through the same function internally, no
> > > matter which external interface was used.
> > >
> > > > It means that every disk access in replay phase should match the 
> > > > recording phase.
> > >
> > > Right. If I'm not mistaken, this was the fundamental requirement you
> > > have, so I wouldn't have suggested this otherwise.
> > >
> > > > Record/replay is intended to be used for debugging and analysis.
> > > > When execution is replayed, guest machine cannot notice analysis 
> > > > overhead.
> > > > Some analysis methods may include disk image reading. E.g., qemu-based
> > > > analysis framework DECAF uses sleuthkit for disk forensics (
> > > https://github.com/sycurelab/DECAF ).
> > > > If similar framework will be used with replay, forensics disk access 
> > > > operations
> > > > won't work if we will record/replay the coroutines.
> > >
> > > Sorry, I'm not sure if I can follow.
> > >
> > > If such analysis software runs in the guest, it's not a replay any more
> > > and I completely fail to see what you're doing.
> > >
> > > If it's a qemu component independent from the guest, then my method
> > > gives you a clean way to bypass the replay driver that wouldn't be
> > > possible with yours.
> >
> > The second one. qemu may be extended with some components that
> > perform guest introspection.
> >
> > > If your plan was to record/replay only async requests and then use sync
> > > requests to bypass the record/replay, let me clearly state that this is
> > > the wrong approach: There are still guest devices which do synchronous
> > > I/O and need to be considered in the replay log, and you shouldn't
> > > prevent the analysis code from using AIO (in fact, using sync I/O in new
> > > code is very much frowned upon).
> >
> > Why do guest synchronous requests have to be recorded?
> > Aren't they completely deterministic?
> 
> Good point. I think you're right in practice. In theory, with dataplane
> (i.e. when running the request in a separate thread) it could happen,
> but I guess that isn't very compatible with replay anyway - and at the
> first sight I couldn't see it performing 

[Qemu-devel] [PULL 06/14] rdma: remove check on time_spent when calculating mbs

2016-02-11 Thread Michael Tokarev
From: Wei Yang 

Within the if statement, time_spent is assured to be non-zero.

This patch just removes the check on time_spent when calculating mbs.

Signed-off-by: Wei Yang 
Signed-off-by: Michael Tokarev 
---
 migration/migration.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 82604d2..a64cfcd 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1691,8 +1691,8 @@ static void *migration_thread(void *opaque)
 double bandwidth = (double)transferred_bytes / time_spent;
 max_size = bandwidth * migrate_max_downtime() / 100;
 
-s->mbps = time_spent ? (((double) transferred_bytes * 8.0) /
-((double) time_spent / 1000.0)) / 1000.0 / 1000.0 : -1;
+s->mbps = (((double) transferred_bytes * 8.0) /
+((double) time_spent / 1000.0)) / 1000.0 / 1000.0;
 
 trace_migrate_transferred(transferred_bytes, time_spent,
   bandwidth, max_size);
-- 
2.1.4




Re: [Qemu-devel] [PATCH] os-posix: Log to logfile in case of daemonize

2016-02-11 Thread Paolo Bonzini


On 11/02/2016 13:12, Dimitris Aragiorgis wrote:
> Besides that, when one executes a daemon, shell redirection is
> hardly, if ever, used. More so if the daemon already has a logfile
> option.
> 
> So, we decided to give it a go and find the least painful way to
> log the stderr of a QEMU process to a logfile.
> 
> To our understanding, the logfile (-D option) is used only for
> messages generated by qemu_log()/qemu_log_mask(). The current
> situation however is that fprintf(stderr, ...) is used in various
> places throughout the codebase for logging/debug purposes.

Right, mostly through error_report.

Actually I like your approach (log to -D if daemonize is used).  I
just was not sure of the best way to implement it.

Perhaps when the logfile is opened you can replace the straight fopen with

qemu_logfile = fopen(...);
if (daemonized) {
dup2(fileno(qemu_logfile), STDERR_FILENO);
fclose(qemu_logfile);
qemu_logfile = stderr;
}

Then the logfile will never be closed by qemu_log_close, and stderr
will always be sent to it.  Does this look sane?

Paolo



[Qemu-devel] [PATCH v2 1/4] hw/ppc/spapr: Add h_set_sprg0 hypercall

2016-02-11 Thread Thomas Huth
This is a very simple hypercall that only sets up the SPRG0
register for the guest (since writing to SPRG0 was only permitted
to the hypervisor in older versions of the PowerISA).

Signed-off-by: Thomas Huth 
---
 hw/ppc/spapr_hcall.c | 15 +--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 12f8c33..63f41ec 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -332,6 +332,15 @@ static target_ulong h_read(PowerPCCPU *cpu, 
sPAPRMachineState *spapr,
 return H_SUCCESS;
 }
 
+static target_ulong h_set_sprg0(PowerPCCPU *cpu, sPAPRMachineState *spapr,
+target_ulong opcode, target_ulong *args)
+{
+cpu_synchronize_state(CPU(cpu));
+cpu->env.spr[SPR_SPRG0] = args[0];
+
+return H_SUCCESS;
+}
+
 static target_ulong h_set_dabr(PowerPCCPU *cpu, sPAPRMachineState *spapr,
target_ulong opcode, target_ulong *args)
 {
@@ -997,6 +1006,10 @@ static void hypercall_register_types(void)
 spapr_register_hypercall(H_REGISTER_VPA, h_register_vpa);
 spapr_register_hypercall(H_CEDE, h_cede);
 
+/* processor register resource access h-calls */
+spapr_register_hypercall(H_SET_SPRG0, h_set_sprg0);
+spapr_register_hypercall(H_SET_MODE, h_set_mode);
+
 /* "debugger" hcalls (also used by SLOF). Note: We do -not- differenciate
  * here between the "CI" and the "CACHE" variants, they will use whatever
  * mapping attributes qemu is using. When using KVM, the kernel will
@@ -1013,8 +1026,6 @@ static void hypercall_register_types(void)
 /* qemu/KVM-PPC specific hcalls */
 spapr_register_hypercall(KVMPPC_H_RTAS, h_rtas);
 
-spapr_register_hypercall(H_SET_MODE, h_set_mode);
-
 /* ibm,client-architecture-support support */
 spapr_register_hypercall(KVMPPC_H_CAS, h_client_architecture_support);
 }
-- 
1.8.3.1




[Qemu-devel] [PATCH v2 2/4] hw/ppc/spapr: Implement h_set_dabr

2016-02-11 Thread Thomas Huth
According to LoPAPR, h_set_dabr should simply set DABRX to 3
(if the register is available), and load the parameter into DABR.
If DABRX is not available, the hypervisor has to check the
"Breakpoint Translation" bit of the DABR register first.

Signed-off-by: Thomas Huth 
---
 hw/ppc/spapr_hcall.c | 25 -
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 63f41ec..0004ca5 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -38,6 +38,12 @@ static void set_spr(CPUState *cs, int spr, target_ulong 
value,
 run_on_cpu(cs, do_spr_sync, );
 }
 
+static bool has_spr(PowerPCCPU *cpu, int spr)
+{
+/* We can test whether the SPR is defined by checking for a valid name */
+return cpu->env.spr_cb[spr].name != NULL;
+}
+
 static inline bool valid_pte_index(CPUPPCState *env, target_ulong pte_index)
 {
 /*
@@ -344,8 +350,19 @@ static target_ulong h_set_sprg0(PowerPCCPU *cpu, 
sPAPRMachineState *spapr,
 static target_ulong h_set_dabr(PowerPCCPU *cpu, sPAPRMachineState *spapr,
target_ulong opcode, target_ulong *args)
 {
-/* FIXME: actually implement this */
-return H_HARDWARE;
+if (!has_spr(cpu, SPR_DABR)) {
+return H_HARDWARE;  /* DABR register not available */
+}
+cpu_synchronize_state(CPU(cpu));
+
+if (has_spr(cpu, SPR_DABRX)) {
+cpu->env.spr[SPR_DABRX] = 0x3;  /* Use Problem and Privileged state */
+} else if (!(args[0] & 0x4)) {  /* Breakpoint Translation set? */
+return H_RESERVED_DABR;
+}
+
+cpu->env.spr[SPR_DABR] = args[0];
+return H_SUCCESS;
 }
 
 #define FLAGS_REGISTER_VPA 0x2000ULL
@@ -999,15 +1016,13 @@ static void hypercall_register_types(void)
 /* hcall-bulk */
 spapr_register_hypercall(H_BULK_REMOVE, h_bulk_remove);
 
-/* hcall-dabr */
-spapr_register_hypercall(H_SET_DABR, h_set_dabr);
-
 /* hcall-splpar */
 spapr_register_hypercall(H_REGISTER_VPA, h_register_vpa);
 spapr_register_hypercall(H_CEDE, h_cede);
 
 /* processor register resource access h-calls */
 spapr_register_hypercall(H_SET_SPRG0, h_set_sprg0);
+spapr_register_hypercall(H_SET_DABR, h_set_dabr);
 spapr_register_hypercall(H_SET_MODE, h_set_mode);
 
 /* "debugger" hcalls (also used by SLOF). Note: We do -not- differenciate
-- 
1.8.3.1




[Qemu-devel] [PATCH v2 0/4] hw/ppc/spapr: Add "Processor Register Hypervisor Resource Access" H-calls

2016-02-11 Thread Thomas Huth
While we were recently debugging a problem with the H_SET_DABR
call [1], I noticed that some hypercalls from the chapter 14.5.4.3
("Processor Register Hypervisor Resource Access") from the LoPAPR
spec [2] are still missing in QEMU.
So here's are some patches that implement these hypercalls. Linux
apparently does not depend on these hypercalls yet (otherwise somebody
would have noticed this earlier), but the hypercalls are rather simple,
so I think the implementations are quite straight-forward and easy to
read.

v2:
- Don't use set_spr() and set cpu->env.spr[] directly instead
- Completely reworked the H_PAGE_INIT patch to map the source
  and target pages for higher speed, and to be able to flush now
  the caches if requested.

[1] 
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=760a7364f27d974d

[2] https://members.openpowerfoundation.org/document/dl/469

Thomas Huth (4):
  hw/ppc/spapr: Add h_set_sprg0 hypercall
  hw/ppc/spapr: Implement h_set_dabr
  hw/ppc/spapr: Implement the h_set_xdabr hypercall
  hw/ppc/spapr: Implement the h_page_init hypercall

 hw/ppc/spapr_hcall.c | 132 ---
 target-ppc/kvm_ppc.h |  24 +-
 2 files changed, 147 insertions(+), 9 deletions(-)

-- 
1.8.3.1




Re: [Qemu-devel] [PATCH] memory: fix usage of find_next_bit and find_next_zero_bit

2016-02-11 Thread Peter Maydell
On 10 February 2016 at 22:40, Peter Maydell  wrote:
> On 10 February 2016 at 14:11, Paolo Bonzini  wrote:
>> The last two arguments to these functions are the last and first bit to
>> check relative to the base.  The code was using incorrectly the first
>> bit and the number of bits.  Fix this in cpu_physical_memory_get_dirty
>> and cpu_physical_memory_all_dirty.  This requires a few changes in the
>> iteration; change the code in cpu_physical_memory_set_dirty_range to
>> match.
>>
>> Fixes: 5b82b70
>> Cc: Stefan Hajnoczi 
>> Signed-off-by: Paolo Bonzini 
>
> I've set the pre-merge tests running so I should be able
> to commit this to master first thing tomorrow.

Applied to master, thanks.

-- PMM



[Qemu-devel] [PATCH v6 6/9] qemu-log: new option -dfilter to limit output

2016-02-11 Thread Alex Bennée
When debugging big programs or system emulation sometimes you want both
the verbosity of cpu,exec et all but don't want to generate lots of logs
for unneeded stuff. This patch adds a new option -dfilter which allows
you to specify interesting address ranges in the form:

  -dfilter 0x8000..0x9000,0xffc8+0x200,...

Then logging code can use the new qemu_log_in_addr_range() function to
decide if it will output logging information for the given range.

Signed-off-by: Alex Bennée 

---

v2
  - More clean-ups to the documentation
v3
  - re-base
  - use GArray instead of GList to avoid cache bouncing
  - checkpatch fixes
v5
  - minor re-base conflicts
  - *did not* convert to -d dfilter=x fmt as easier to document
  - strtoul -> qemu_strtoul
  - fix a few checkpatch spacing warnings
  - made range operator .., - now intuitively subtracts from start
  - no added r-b tag as changes a bit
v6
  - make parsing suck less (stroull, strstr, err)
  - add some basic unit tests
---
 include/qemu/log.h   |  2 ++
 qemu-options.hx  | 18 +++
 tests/Makefile   |  4 +++
 tests/test-logging.c | 75 ++
 util/log.c   | 85 
 vl.c |  3 ++
 6 files changed, 187 insertions(+)
 create mode 100644 tests/test-logging.c

diff --git a/include/qemu/log.h b/include/qemu/log.h
index 22c4dec..c0225e1 100644
--- a/include/qemu/log.h
+++ b/include/qemu/log.h
@@ -132,6 +132,8 @@ static inline void qemu_set_log(int log_flags)
 }
 
 void qemu_set_log_filename(const char *filename);
+void qemu_set_dfilter_ranges(const char *ranges);
+bool qemu_log_in_addr_range(uint64_t addr);
 int qemu_str_to_log_mask(const char *str);
 
 /* Print a usage message listing all the valid logging categories
diff --git a/qemu-options.hx b/qemu-options.hx
index 2f0465e..c7e0486 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3094,6 +3094,24 @@ STEXI
 Output log in @var{logfile} instead of to stderr
 ETEXI
 
+DEF("dfilter", HAS_ARG, QEMU_OPTION_DFILTER, \
+"-dfilter range,..  filter debug output to range of addresses (useful for 
-d cpu,exec,etc..)\n",
+QEMU_ARCH_ALL)
+STEXI
+@item -dfilter @var{range1}[,...]
+@findex -dfilter
+Filter debug output to that relevant to a range of target addresses. The filter
+spec can be either @var{start}+@var{size}, @var{start}-@var{size} or
+@var{start}..@var{end} where @var{start} @var{end} and @var{size} are the
+addresses and sizes required. For example:
+@example
+-dfilter 0x8000..0x9000,0xffc8+0x200,0xffc6-0x1000
+@end example
+Will dump output for any code in the 0x1000 sized block starting at 0x8000 and
+the 0x200 sized block starting at 0xffc8 and another 0x1000 sized
+block starting at 0xffc5f000.
+ETEXI
+
 DEF("L", HAS_ARG, QEMU_OPTION_L, \
 "-L path set the directory for the BIOS, VGA BIOS and keymaps\n",
 QEMU_ARCH_ALL)
diff --git a/tests/Makefile b/tests/Makefile
index 650e654..8abfa6e 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -92,6 +92,8 @@ check-unit-$(CONFIG_GNUTLS) += 
tests/test-io-channel-tls$(EXESUF)
 check-unit-y += tests/test-io-channel-command$(EXESUF)
 check-unit-y += tests/test-io-channel-buffer$(EXESUF)
 check-unit-y += tests/test-base64$(EXESUF)
+gcov-files-test-logging-y = tests/test-logging.c
+check-unit-y += tests/test-logging$(EXESUF)
 
 check-block-$(CONFIG_POSIX) += tests/qemu-iotests-quick.sh
 
@@ -430,6 +432,8 @@ tests/test-timed-average$(EXESUF): 
tests/test-timed-average.o qemu-timer.o \
 tests/test-base64$(EXESUF): tests/test-base64.o \
libqemuutil.a libqemustub.a
 
+tests/test-logging$(EXESUF): tests/test-logging.o $(test-util-obj-y)
+
 tests/test-qapi-types.c tests/test-qapi-types.h :\
 $(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json 
$(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py \
diff --git a/tests/test-logging.c b/tests/test-logging.c
new file mode 100644
index 000..c2dae49
--- /dev/null
+++ b/tests/test-logging.c
@@ -0,0 +1,75 @@
+/*
+ * logging unit-tests
+ *
+ * Copyright (C) 2016 Linaro Ltd.
+ *
+ *  Author: Alex Bennée 
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT 

Re: [Qemu-devel] [PATCH 3/9] dma-helpers: Do not truncate small qiovs

2016-02-11 Thread Paolo Bonzini


On 16/12/2015 17:55, Alex Pyrgiotis wrote:
> If the size of the qiov is smaller than the sector size, do not truncate
> the qiov, which would effectively make it empty. Instead, allow it to
> pass as is.
> 
> This is necessary for SCSI requests like READ CAPACITY which have small
> buffers, e.g. 32 bytes.
> 
> Signed-off-by: Alex Pyrgiotis 
> Signed-off-by: Dimitris Aragiorgis 
> 
> diff --git a/dma-helpers.c b/dma-helpers.c
> index e1ea7b3..b8f2ae0 100644
> --- a/dma-helpers.c
> +++ b/dma-helpers.c
> @@ -162,7 +162,16 @@ static void dma_map_sg(DMAAIOCB *dbs)
>  return;
>  }
>  
> -if (dbs->iov.size & ~BDRV_SECTOR_MASK) {
> +/*
> + * If the size of the qiov is not a multiple of the sector size, truncate
> + * the qiov.
> + *
> + * NOTE: If the qiov is less than a sector, we can assume that there is a
> + * reason for it, e.g., a SCSI request such as READ CAPACITY, and we 
> should
> + * not truncate it.
> + */

I'm afraid this is wrong.  It is legal to send SCSI requests that are
e.g. 513 bytes in size.  The sector limit is arbitrary.

I think the "if" statement is wrong too, however.  In particular it has
not broken IDE TRIM only because the ATA standard makes 512 bytes the
unit of DMA transfer.  Rather, it is the IDE device that should discard
extra data in the QEMUSGList.

Paolo

> +if (dbs->iov.size & ~BDRV_SECTOR_MASK &&
> +dbs->iov.size > BDRV_SECTOR_SIZE) {
>  qemu_iovec_discard_back(>iov, dbs->iov.size & 
> ~BDRV_SECTOR_MASK);
>  }
>  }
> 



[Qemu-devel] [PULL v2 00/15] target-arm queue

2016-02-11 Thread Peter Maydell
Resend, with typedef redefine fixed.

thanks
-- PMM


The following changes since commit 88c73d16ad1b6c22a2ab082064d0d521f756296a:

  memory: fix usage of find_next_bit and find_next_zero_bit (2016-02-10 
22:38:24 +)

are available in the git repository at:

  git://git.linaro.org/people/pmaydell/qemu-arm.git 
tags/pull-target-arm-20160211

for you to fetch changes up to f0afa73164778570083504a185d7498884c68d65:

  bcm2835_property: implement "get board revision" query (2016-02-11 11:17:32 
+)


target-arm queue:
 * fix some missing traps for EL3 support
 * enable EL3 on Cortex-A53 and Cortex-A57
 * fix syndrome IL bit for Thumb coprocessor, VFP and Neon traps
 * fix mishandling of architectural watchpoints
 * avoid buffer overflow in sd.c
 * fix max-cpus check in virt board
 * implement 'get board revision' query for BCM2835


Andrew Jones (1):
  hw/arm/virt: fix max-cpus check

Peter Maydell (10):
  target-arm: Fix typo in comment in arm_is_secure_below_el3()
  target-arm: Implement MDCR_EL3 and SDCR
  target-arm: Use access_trap_aa32s_el1() for SCR and MVBAR
  target-arm: Update arm_generate_debug_exceptions() to handle EL2/EL3
  target-arm: Add isread parameter to CPAccessFns
  target-arm: Implement NSACR trapping behaviour
  target-arm: Enable EL3 for Cortex-A53 and Cortex-A57
  target-arm: Correct misleading 'is_thumb' syn_* parameter names
  target-arm: Fix IL bit reported for Thumb coprocessor traps
  target-arm: Fix IL bit reported for Thumb VFP and Neon traps

Prasad J Pandit (1):
  sd: limit 'req.cmd' while using as an array index

Sergey Fedorov (2):
  cpu: Add callback to check architectural watchpoint match
  target-arm: Implement checking of fired watchpoint

Stephen Warren (1):
  bcm2835_property: implement "get board revision" query

 exec.c |   6 ++
 hw/arm/bcm2835_peripherals.c   |   2 +
 hw/arm/bcm2836.c   |   2 +
 hw/arm/raspi.c |   2 +
 hw/arm/virt.c  |  10 +--
 hw/misc/bcm2835_property.c |   4 +-
 hw/sd/sd.c |   7 +-
 include/hw/misc/bcm2835_property.h |   1 +
 include/qom/cpu.h  |   8 +-
 qom/cpu.c  |   9 ++
 target-arm/cpu.c   |   1 +
 target-arm/cpu.h   |  55 ++--
 target-arm/cpu64.c |   2 +
 target-arm/helper.c| 173 +
 target-arm/helper.h|   2 +-
 target-arm/internals.h |  31 ---
 target-arm/op_helper.c |  40 +
 target-arm/translate-a64.c |   6 +-
 target-arm/translate.c |  21 +++--
 19 files changed, 288 insertions(+), 94 deletions(-)



[Qemu-devel] [PULL 13/14] Adds keycode 86 to the hid_usage_keys translation table.

2016-02-11 Thread Michael Tokarev
From: Daniel Serpell 

This key is present in international keyboards, between left shift and
the 'Z' key, ant is described in the HID usage tables as "Keyboard
Non-US \ and |": http://www.usb.org/developers/hidpage/Hut1_12v2.pdf

This patch fixes the usb-kbd devices.

Signed-off-by: Daniel Serpell 
Reviewed-by: Gerd Hoffmann 
Signed-off-by: Michael Tokarev 
---
 hw/input/hid.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/input/hid.c b/hw/input/hid.c
index a11e2bc..b41efbb 100644
--- a/hw/input/hid.c
+++ b/hw/input/hid.c
@@ -45,7 +45,7 @@ static const uint8_t hid_usage_keys[0x100] = {
 0xe2, 0x2c, 0x39, 0x3a, 0x3b, 0x3c, 0x3d, 0x3e,
 0x3f, 0x40, 0x41, 0x42, 0x43, 0x53, 0x47, 0x5f,
 0x60, 0x61, 0x56, 0x5c, 0x5d, 0x5e, 0x57, 0x59,
-0x5a, 0x5b, 0x62, 0x63, 0x00, 0x00, 0x00, 0x44,
+0x5a, 0x5b, 0x62, 0x63, 0x00, 0x00, 0x64, 0x44,
 0x45, 0x68, 0x69, 0x6a, 0x6b, 0x6c, 0x6d, 0x6e,
 0xe8, 0xe9, 0x71, 0x72, 0x73, 0x00, 0x00, 0x00,
 0x00, 0x00, 0x00, 0x85, 0x00, 0x00, 0x00, 0x00,
-- 
2.1.4




[Qemu-devel] [PULL 08/14] char: fix parameter name / type in BSD codepath

2016-02-11 Thread Michael Tokarev
From: "Daniel P. Berrange" 

The BSD impl of qemu_chr_open_pp_fd had mis-declared
its parameter type as ChardevBackend instead of
ChardevCommon. It had also mistakenly used the variable
name 'common' instead of 'backend'.

Tested-by: Sean Bruno 
Signed-off-by: Daniel P. Berrange 
Signed-off-by: Michael Tokarev 
---
 qemu-char.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index 2b2c56b..1b7d5da 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -1796,12 +1796,12 @@ static int pp_ioctl(CharDriverState *chr, int cmd, void 
*arg)
 }
 
 static CharDriverState *qemu_chr_open_pp_fd(int fd,
-ChardevBackend *backend,
+ChardevCommon *backend,
 Error **errp)
 {
 CharDriverState *chr;
 
-chr = qemu_chr_alloc(common, errp);
+chr = qemu_chr_alloc(backend, errp);
 if (!chr) {
 return NULL;
 }
-- 
2.1.4




[Qemu-devel] [PATCH v6 7/9] qemu-log: dfilter-ise exec, out_asm, op and opt_op

2016-02-11 Thread Alex Bennée
This ensures the code generation debug code will honour -dfilter if set.
For the "exec" tracing I've added a new inline macro for efficiency's
sake.

Signed-off-by: Alex Bennée 
Reviewed-by: Aurelien Jarno 
Reviewed-by: Richard Henderson 


v2
   - checkpatch updates
   - add qemu_log_mask_and_addr macro for inline dump for traces
   - re-base on re-factored tcg layout
   - include new Trace & Link lines
v5
   - add r-b tag
   - slight reword to commit now LOG_OP is common
v6
   - minor merge conflict with earlier patch
   - add r-b tag
---
 cpu-exec.c  | 13 +++--
 include/exec/exec-all.h |  8 +---
 include/qemu/log.h  | 15 +++
 tcg/tcg.c   |  6 --
 translate-all.c |  3 ++-
 5 files changed, 33 insertions(+), 12 deletions(-)

diff --git a/cpu-exec.c b/cpu-exec.c
index 6206cdf..bbfcbfb 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -139,8 +139,9 @@ static inline tcg_target_ulong cpu_tb_exec(CPUState *cpu, 
TranslationBlock *itb)
 uintptr_t next_tb;
 uint8_t *tb_ptr = itb->tc_ptr;
 
-qemu_log_mask(CPU_LOG_EXEC, "Trace %p [" TARGET_FMT_lx "] %s\n",
-  itb->tc_ptr, itb->pc, lookup_symbol(itb->pc));
+qemu_log_mask_and_addr(CPU_LOG_EXEC, itb->pc,
+   "Trace %p [" TARGET_FMT_lx "] %s\n",
+   itb->tc_ptr, itb->pc, lookup_symbol(itb->pc));
 
 #if defined(DEBUG_DISAS)
 if (qemu_loglevel_mask(CPU_LOG_TB_CPU)) {
@@ -171,10 +172,10 @@ static inline tcg_target_ulong cpu_tb_exec(CPUState *cpu, 
TranslationBlock *itb)
  */
 CPUClass *cc = CPU_GET_CLASS(cpu);
 TranslationBlock *tb = (TranslationBlock *)(next_tb & ~TB_EXIT_MASK);
-qemu_log_mask(CPU_LOG_EXEC,
-  "Stopped execution of TB chain before %p ["
-  TARGET_FMT_lx "] %s\n",
-  itb->tc_ptr, itb->pc, lookup_symbol(itb->pc));
+qemu_log_mask_and_addr(CPU_LOG_EXEC, itb->pc,
+   "Stopped execution of TB chain before %p ["
+   TARGET_FMT_lx "] %s\n",
+   itb->tc_ptr, itb->pc, lookup_symbol(itb->pc));
 if (cc->synchronize_from_tb) {
 cc->synchronize_from_tb(cpu, tb);
 } else {
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index 1823ee3..7362095 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -379,9 +379,11 @@ static inline void tb_add_jump(TranslationBlock *tb, int n,
 {
 /* NOTE: this test is only needed for thread safety */
 if (!tb->jmp_next[n]) {
-qemu_log_mask(CPU_LOG_EXEC, "Linking TBs %p [" TARGET_FMT_lx
-  "] index %d -> %p [" TARGET_FMT_lx "]\n",
-  tb->tc_ptr, tb->pc, n, tb_next->tc_ptr, tb_next->pc);
+qemu_log_mask_and_addr(CPU_LOG_EXEC, tb->pc,
+   "Linking TBs %p [" TARGET_FMT_lx
+   "] index %d -> %p [" TARGET_FMT_lx "]\n",
+   tb->tc_ptr, tb->pc, n,
+   tb_next->tc_ptr, tb_next->pc);
 /* patch the native jump address */
 tb_set_jmp_target(tb, n, (uintptr_t)tb_next->tc_ptr);
 
diff --git a/include/qemu/log.h b/include/qemu/log.h
index c0225e1..78ac827 100644
--- a/include/qemu/log.h
+++ b/include/qemu/log.h
@@ -82,6 +82,21 @@ qemu_log_vprintf(const char *fmt, va_list va)
 }   \
 } while (0)
 
+/* log only if a bit is set on the current loglevel mask
+ * and we are in the address range we care about:
+ * @mask: bit to check in the mask
+ * @addr: address to check in dfilter
+ * @fmt: printf-style format string
+ * @args: optional arguments for format string
+ */
+#define qemu_log_mask_and_addr(MASK, ADDR, FMT, ...)\
+do {\
+if (unlikely(qemu_loglevel_mask(MASK)) &&   \
+ qemu_log_in_addr_range(ADDR)) {\
+qemu_log(FMT, ## __VA_ARGS__);  \
+}   \
+} while (0)
+
 /* Maintenance: */
 
 /* fflush() the log file */
diff --git a/tcg/tcg.c b/tcg/tcg.c
index b76d978..f059b56 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -2292,7 +2292,8 @@ int tcg_gen_code(TCGContext *s, TranslationBlock *tb)
 #endif
 
 #ifdef DEBUG_DISAS
-if (unlikely(qemu_loglevel_mask(CPU_LOG_TB_OP))) {
+if (unlikely(qemu_loglevel_mask(CPU_LOG_TB_OP)
+ && qemu_log_in_addr_range(tb->pc))) {
 qemu_log("OP:\n");
 tcg_dump_ops(s);
 qemu_log("\n");
@@ -2319,7 +2320,8 @@ int tcg_gen_code(TCGContext *s, TranslationBlock *tb)
 #endif
 
 #ifdef DEBUG_DISAS
-if (unlikely(qemu_loglevel_mask(CPU_LOG_TB_OP_OPT))) {
+if (unlikely(qemu_loglevel_mask(CPU_LOG_TB_OP_OPT)
+ && 

[Qemu-devel] [PATCH v6 8/9] target-arm: dfilter support for in_asm

2016-02-11 Thread Alex Bennée
Each individual architecture needs to use the qemu_log_in_addr_range()
feature for enabling in_asm output as it is part of the frontend.

Signed-off-by: Alex Bennée 
Reviewed-by: Aurelien Jarno 

---
v5
  - no longer wrapping tcg_gen_insn_start (was tcg_gen_debug)
  - reword to handle only in_asm
  - added r-b tag
---
 target-arm/translate-a64.c | 3 ++-
 target-arm/translate.c | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index d780e09..98ba35c 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -11211,7 +11211,8 @@ done_generating:
 gen_tb_end(tb, num_insns);
 
 #ifdef DEBUG_DISAS
-if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)) {
+if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM) &&
+qemu_log_in_addr_range(pc_start)) {
 qemu_log("\n");
 qemu_log("IN: %s\n", lookup_symbol(pc_start));
 log_target_disas(cs, pc_start, dc->pc - pc_start,
diff --git a/target-arm/translate.c b/target-arm/translate.c
index f6a38bc..267be26 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -11580,7 +11580,8 @@ done_generating:
 gen_tb_end(tb, num_insns);
 
 #ifdef DEBUG_DISAS
-if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)) {
+if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM) &&
+qemu_log_in_addr_range(pc_start)) {
 qemu_log("\n");
 qemu_log("IN: %s\n", lookup_symbol(pc_start));
 log_target_disas(cs, pc_start, dc->pc - pc_start,
-- 
2.7.0




Re: [Qemu-devel] [PATCH 3/3] replay: introduce block devices record/replay

2016-02-11 Thread Pavel Dovgalyuk
> From: Kevin Wolf [mailto:kw...@redhat.com]
> Am 11.02.2016 um 07:05 hat Pavel Dovgalyuk geschrieben:
> > > From: Kevin Wolf [mailto:kw...@redhat.com]
> > > Am 10.02.2016 um 13:51 hat Pavel Dovgalyuk geschrieben:
> > > > However, I don't understand yet which layer do you offer as the 
> > > > candidate
> > > > for record/replay? What functions should be changed?
> > > > I would like to investigate this way, but I don't got it yet.
> > >
> > > At the core, I wouldn't change any existing function, but introduce a
> > > new block driver. You could copy raw_bsd.c for a start and then tweak
> > > it. Leave out functions that you don't want to support, and add the
> > > necessary magic to .bdrv_co_readv/writev.
> > >
> > > Something like this (can probably be generalised for more than just
> > > reads as the part after the bdrv_co_reads() call should be the same for
> > > reads, writes and any other request types):
> > >
> > > int blkreplay_co_readv()
> > > {
> > > BlockReplayState *s = bs->opaque;
> > > int reqid = s->reqid++;
> > >
> > > bdrv_co_readv(bs->file, ...);
> > >
> > > if (mode == record) {
> > > log(reqid, time);
> > > } else {
> > > assert(mode == replay);
> > > bool *done = req_replayed_list_get(reqid)
> > > if (done) {
> > > *done = true;
> > > } else {
> > > req_completed_list_insert(reqid, qemu_coroutine_self());
> > > qemu_coroutine_yield();
> > > }
> > > }
> > > }
> > >
> > > /* called by replay.c */
> > > int blkreplay_run_event()
> > > {
> > > if (mode == replay) {
> > > co = req_completed_list_get(e.reqid);
> > > if (co) {
> > > qemu_coroutine_enter(co);
> > > } else {
> > > bool done = false;
> > > req_replayed_list_insert(reqid, );
> > > /* wait synchronously for completion */
> > > while (!done) {
> > > aio_poll();
> > > }
> > > }
> > > }
> > > }
> > >
> > > Where we could consider changing existing code is that it might be
> > > desirable to automatically put an instance of this block driver on top
> > > of every block device when record/replay is used. If we don't do that,
> > > you need to explicitly specify -drive driver=blkreplay,...
> >
> > As far, as I understand, all synchronous read/write request are also passed
> > through this coroutines layer.
> 
> Yes, all read/write requests go through the same function internally, no
> matter which external interface was used.
> 
> > It means that every disk access in replay phase should match the recording 
> > phase.
> 
> Right. If I'm not mistaken, this was the fundamental requirement you
> have, so I wouldn't have suggested this otherwise.
> 
> > Record/replay is intended to be used for debugging and analysis.
> > When execution is replayed, guest machine cannot notice analysis overhead.
> > Some analysis methods may include disk image reading. E.g., qemu-based
> > analysis framework DECAF uses sleuthkit for disk forensics (
> https://github.com/sycurelab/DECAF ).
> > If similar framework will be used with replay, forensics disk access 
> > operations
> > won't work if we will record/replay the coroutines.
> 
> Sorry, I'm not sure if I can follow.
> 
> If such analysis software runs in the guest, it's not a replay any more
> and I completely fail to see what you're doing.
> 
> If it's a qemu component independent from the guest, then my method
> gives you a clean way to bypass the replay driver that wouldn't be
> possible with yours.

The second one. qemu may be extended with some components that
perform guest introspection.

> If your plan was to record/replay only async requests and then use sync
> requests to bypass the record/replay, let me clearly state that this is
> the wrong approach: There are still guest devices which do synchronous
> I/O and need to be considered in the replay log, and you shouldn't
> prevent the analysis code from using AIO (in fact, using sync I/O in new
> code is very much frowned upon).

Why do guest synchronous requests have to be recorded?
Aren't they completely deterministic?

> I can explain in more detail what the block device structure looks like
> and how to access an image with and without record/replay, but first let
> me please know whether I guessed right what your problem is. Or if I
> missed your point, can you please describe in detail a case that
> wouldn't work?

You have understood it correctly.
And what is the solution for bypassing one of the layers from component that
should not affect the replay?

> > And if we'll record only high-level asynchronous disk operations, then
> > there will be a way to performs disk accesses without matching these
> > operations with replay log.
> >
> > However, we already tried to record disk operations completion events
> > (in previous more complicated version). Recording overhead was the same
> > as with new blk_aio_ 

[Qemu-devel] [PATCH] usb: check USB configuration descriptor object

2016-02-11 Thread P J P
From: Prasad J Pandit 

When processing remote NDIS control message packets, the USB Net
device emulator checks to see if the USB configuration descriptor
object is of RNDIS type(2). But it does not check if it is null,
which leads to a null dereference error. Add check to avoid it.

Reported-by: Qinghao Tang 
Signed-off-by: Prasad J Pandit 
---
 hw/usb/dev-network.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/usb/dev-network.c b/hw/usb/dev-network.c
index ba3c7a7..180adce 100644
--- a/hw/usb/dev-network.c
+++ b/hw/usb/dev-network.c
@@ -653,7 +653,8 @@ typedef struct USBNetState {
 
 static int is_rndis(USBNetState *s)
 {
-return s->dev.config->bConfigurationValue == DEV_RNDIS_CONFIG_VALUE;
+return s->dev.config ?
+s->dev.config->bConfigurationValue == DEV_RNDIS_CONFIG_VALUE : 0;
 }
 
 static int ndis_query(USBNetState *s, uint32_t oid,
-- 
2.5.0




[Qemu-devel] [PULL 05/14] qemu-sockets: simplify error handling

2016-02-11 Thread Michael Tokarev
From: Paolo Bonzini 

Just go always through the err label.  (Noticed because Coverity
complains that peer is always non-NULL in the error cleanup code,
but removing the "if" is arguably more prone to introducing the
opposite bug in the future).

Signed-off-by: Paolo Bonzini 
Reviewed-by: Daniel P. Berrange 
Signed-off-by: Michael Tokarev 
---
 util/qemu-sockets.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index b665cdb..557da20 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -459,7 +459,7 @@ static int inet_dgram_saddr(InetSocketAddress *sraddr,
 
 if (err) {
 error_propagate(errp, err);
-return -1;
+goto err;
 }
 
 addr = sraddr->host;
@@ -469,13 +469,13 @@ static int inet_dgram_saddr(InetSocketAddress *sraddr,
 }
 if (port == NULL || strlen(port) == 0) {
 error_setg(errp, "remote port not specified");
-return -1;
+goto err;
 }
 
 if (0 != (rc = getaddrinfo(addr, port, , ))) {
 error_setg(errp, "address resolution failed for %s:%s: %s", addr, port,
gai_strerror(rc));
-   return -1;
+   goto err;
 }
 
 /* lookup local addr */
-- 
2.1.4




[Qemu-devel] [PULL 02/14] man: virtfs-proxy-helper: Rework awkward sentence

2016-02-11 Thread Michael Tokarev
From: Christophe Fergeau 

There was a 'capbilities' typo in this man page. This commit
reformulates the sentence the typo was in to make it easier to grasp.
This is based on a suggestion from Eric Blake.

Signed-off-by: Christophe Fergeau 
Reviewed-by: Eric Blake 
Signed-off-by: Michael Tokarev 
---
 fsdev/virtfs-proxy-helper.texi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fsdev/virtfs-proxy-helper.texi b/fsdev/virtfs-proxy-helper.texi
index 9a25d7e..6eb2d50 100644
--- a/fsdev/virtfs-proxy-helper.texi
+++ b/fsdev/virtfs-proxy-helper.texi
@@ -28,8 +28,8 @@ QEMU and proxy helper communicate using this socket. QEMU 
proxy fs
 driver sends filesystem request to proxy helper and receives the
 response from it.
 
-Proxy helper is designed so that it can drop the root privilege with
-retaining capbilities needed for doing filesystem operations only.
+The proxy helper is designed so that it can drop root privileges except
+for the capabilities needed for doing filesystem operations.
 
 @end table
 @c man end
-- 
2.1.4




[Qemu-devel] [PULL 03/14] qom: Correct object_property_get_int() description

2016-02-11 Thread Michael Tokarev
From: Alistair Francis 

The description of object_property_get_int() stated that on an error
it returns NULL. This is not the case and the function will return -1
if an error occurs. Update the commented documentation accordingly.

Reported-By: Christian Liebhardt 
Signed-off-by: Christian Liebhardt 
Signed-off-by: Alistair Francis 
Signed-off-by: Michael Tokarev 
---
 include/qom/object.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/qom/object.h b/include/qom/object.h
index 698827d..33abce9 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -1115,7 +1115,7 @@ void object_property_set_int(Object *obj, int64_t value,
  * @name: the name of the property
  * @errp: returns an error if this function fails
  *
- * Returns: the value of the property, converted to an integer, or NULL if
+ * Returns: the value of the property, converted to an integer, or negative if
  * an error occurs (including when the property value is not an integer).
  */
 int64_t object_property_get_int(Object *obj, const char *name,
-- 
2.1.4




Re: [Qemu-devel] [RFC v7 01/16] exec.c: Add new exclusive bitmap to ram_list

2016-02-11 Thread Alex Bennée

Alvise Rigo  writes:

> The purpose of this new bitmap is to flag the memory pages that are in
> the middle of LL/SC operations (after a LL, before a SC). For all these
> pages, the corresponding TLB entries will be generated in such a way to
> force the slow-path for all the VCPUs (see the following patches).
>
> When the system starts, the whole memory is set to dirty.
>
> Suggested-by: Jani Kokkonen 
> Suggested-by: Claudio Fontana 
> Signed-off-by: Alvise Rigo 
> ---
>  exec.c  |  7 +--
>  include/exec/memory.h   |  3 ++-
>  include/exec/ram_addr.h | 31 +++
>  3 files changed, 38 insertions(+), 3 deletions(-)
>
> diff --git a/exec.c b/exec.c
> index 7115403..51f366d 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1575,11 +1575,14 @@ static ram_addr_t ram_block_add(RAMBlock *new_block, 
> Error **errp)
>  int i;
>
>  /* ram_list.dirty_memory[] is protected by the iothread lock.  */
> -for (i = 0; i < DIRTY_MEMORY_NUM; i++) {
> +for (i = 0; i < DIRTY_MEMORY_EXCLUSIVE; i++) {
>  ram_list.dirty_memory[i] =
>  bitmap_zero_extend(ram_list.dirty_memory[i],
> old_ram_size, new_ram_size);
> -   }
> +}
> +ram_list.dirty_memory[DIRTY_MEMORY_EXCLUSIVE] =
> +bitmap_zero_extend(ram_list.dirty_memory[DIRTY_MEMORY_EXCLUSIVE],
> +   old_ram_size, new_ram_size);

In the previous patch you moved this out of the loop as
ram_list.dirty_memory[DIRTY_MEMORY_EXCLUSIVE] was a different size to
the other dirty bitmaps. This no longer seems to be the case so this
seems pointless.

>  }
>  cpu_physical_memory_set_dirty_range(new_block->offset,
>  new_block->used_length,
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index c92734a..71e0480 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -19,7 +19,8 @@
>  #define DIRTY_MEMORY_VGA   0
>  #define DIRTY_MEMORY_CODE  1
>  #define DIRTY_MEMORY_MIGRATION 2
> -#define DIRTY_MEMORY_NUM   3/* num of dirty bits */
> +#define DIRTY_MEMORY_EXCLUSIVE 3
> +#define DIRTY_MEMORY_NUM   4/* num of dirty bits */
>
>  #include 
>  #include 
> diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
> index ef1489d..19789fc 100644
> --- a/include/exec/ram_addr.h
> +++ b/include/exec/ram_addr.h
> @@ -21,6 +21,7 @@
>
>  #ifndef CONFIG_USER_ONLY
>  #include "hw/xen/xen.h"
> +#include "sysemu/sysemu.h"
>
>  struct RAMBlock {
>  struct rcu_head rcu;
> @@ -172,6 +173,9 @@ static inline void 
> cpu_physical_memory_set_dirty_range(ram_addr_t start,
>  if (unlikely(mask & (1 << DIRTY_MEMORY_CODE))) {
>  bitmap_set_atomic(d[DIRTY_MEMORY_CODE], page, end - page);
>  }
> +if (unlikely(mask & (1 << DIRTY_MEMORY_EXCLUSIVE))) {
> +bitmap_set_atomic(d[DIRTY_MEMORY_EXCLUSIVE], page, end - page);
> +}
>  xen_modified_memory(start, length);
>  }
>
> @@ -287,5 +291,32 @@ uint64_t cpu_physical_memory_sync_dirty_bitmap(unsigned 
> long *dest,
>  }
>
>  void migration_bitmap_extend(ram_addr_t old, ram_addr_t new);
> +
> +/* Exclusive bitmap support. */
> +#define EXCL_BITMAP_GET_OFFSET(addr) (addr >> TARGET_PAGE_BITS)
> +
> +/* Make the page of @addr not exclusive. */
> +static inline void cpu_physical_memory_unset_excl(ram_addr_t addr)
> +{
> +set_bit_atomic(EXCL_BITMAP_GET_OFFSET(addr),
> +   ram_list.dirty_memory[DIRTY_MEMORY_EXCLUSIVE]);
> +}
> +
> +/* Return true if the page of @addr is exclusive, i.e. the EXCL bit is set. 
> */
> +static inline int cpu_physical_memory_is_excl(ram_addr_t addr)
> +{
> +return !test_bit(EXCL_BITMAP_GET_OFFSET(addr),
> + ram_list.dirty_memory[DIRTY_MEMORY_EXCLUSIVE]);
> +}
> +
> +/* Set the page of @addr as exclusive clearing its EXCL bit and return the
> + * previous bit's state. */
> +static inline int cpu_physical_memory_set_excl(ram_addr_t addr)
> +{
> +return bitmap_test_and_clear_atomic(
> +
> ram_list.dirty_memory[DIRTY_MEMORY_EXCLUSIVE],
> +EXCL_BITMAP_GET_OFFSET(addr), 1);
> +}
> +
>  #endif
>  #endif


--
Alex Bennée



Re: [Qemu-devel] Memory on stellaris board

2016-02-11 Thread Aurelio Remonda
On Fri, Feb 5, 2016 at 2:00 PM, Peter Maydell  wrote:
> On 5 February 2016 at 16:55, Aurelio Remonda
>  wrote:
>> Im making something like this:
>>
>> if (ram_size == 0x800)  /* default value, means whitout -m flag */
>>{
>> sram_size = ((board->dc0 >> 18) + * 1024;
>> }
>>   else if (ram_size >= UINT_MAX) /* more than 4GB */
>>   sram_size = UINT_MAX;
>>   else
>>   sram_size = ram_size
>>
>> So in case someone does not use the -m flag i want to be sure the ram is
>> calculated like it was before.
>
> The right way to do this is to set the MachineClass default_ram_size
> to what you want your default value to be. Then you should calculate
> the dc0 etc values to expose to the guest based on the ram size
> (which might be the default or might be user specified).

But stellaris board has a hardcoded dc0 value, when you place the -m flag
with for example 256M you shouldn't calculate any sram_size from dc0, you
just assign that as your ram size.

> If the user asks for a value that you can't handle (eg it is too big)
> then you should report the problem (via error_report()) and exit.

Fair enough.

Thank you!

-- 
Aurelio Remonda

Software Engineer

San Lorenzo 47, 3rd Floor, Office 5
Córdoba, Argentina
Phone: +54-351-4217888 / 4218211



Re: [Qemu-devel] [PULL 8/9] static checker: e1000-82540em got aliased to e1000

2016-02-11 Thread Paolo Bonzini


On 05/02/2016 14:56, Amit Shah wrote:
> Commit 8304402033e8dbe8e379017d51ed1dd8344f1dce changed the name of the
> e1000-82540em device to e1000.  This was flagged:
> 
>Section "e1000-82540em" does not exist in dest
> 
> Add the mapping to the changed section names dictionary so the checker
> can proceed.
> 
> Signed-off-by: Amit Shah 
> Acked-by: Jason Wang 
> Message-Id: 
> <7ccfe834c897142dceaa4da87c13b7059fa12aa8.1450416947.git.amit.s...@redhat.com>
> Signed-off-by: Amit Shah 
> ---
>  scripts/vmstate-static-checker.py | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/scripts/vmstate-static-checker.py 
> b/scripts/vmstate-static-checker.py
> index b6c0bbe..b5ecaf6 100755
> --- a/scripts/vmstate-static-checker.py
> +++ b/scripts/vmstate-static-checker.py
> @@ -99,6 +99,7 @@ def get_changed_sec_name(sec):
>  # Section names can change -- see commit 292b1634 for an example.
>  changes = {
>  "ICH9 LPC": "ICH9-LPC",
> +"e1000-82540em": "e1000",
>  }
>  
>  for item in changes:
> 

This means that 2.5 cannot migrate 2.4 virtual machines, right?  Is that
something we want to rectify in 2.6 by making e1000-82540em an alias of
e1000 (instead of the other way round)?

Paolo



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

2016-02-11 Thread Peter Maydell
On 11 February 2016 at 11:25, Peter Maydell <peter.mayd...@linaro.org> wrote:
> Resend, with typedef redefine fixed.
>
> thanks
> -- PMM
>
>
> The following changes since commit 88c73d16ad1b6c22a2ab082064d0d521f756296a:
>
>   memory: fix usage of find_next_bit and find_next_zero_bit (2016-02-10 
> 22:38:24 +)
>
> are available in the git repository at:
>
>   git://git.linaro.org/people/pmaydell/qemu-arm.git 
> tags/pull-target-arm-20160211
>
> for you to fetch changes up to f0afa73164778570083504a185d7498884c68d65:
>
>   bcm2835_property: implement "get board revision" query (2016-02-11 11:17:32 
> +)

Applied this version, thanks.

-- PMM



Re: [Qemu-devel] [RFC v7 02/16] softmmu: Simplify helper_*_st_name, wrap unaligned code

2016-02-11 Thread Alex Bennée

Alvise Rigo  writes:

> Attempting to simplify the helper_*_st_name, wrap the
> do_unaligned_access code into an inline function.
> Remove also the goto statement.

How are you generating your CC list? get_maintainer.pl shows Peter
Croshwaite (CC'ed) should also be CC'ed on these patches. If we want to
get any of this patch series merged before soft freeze we'll need some
signoffs from the maintainers ;-)

>
> Based on this work, Alex proposed the following patch series
> https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg01136.html
> that reduces code duplication of the softmmu_helpers.
>
> Suggested-by: Jani Kokkonen 
> Suggested-by: Claudio Fontana 
> Signed-off-by: Alvise Rigo 
> ---
>  softmmu_template.h | 96 
> ++
>  1 file changed, 60 insertions(+), 36 deletions(-)
>
> diff --git a/softmmu_template.h b/softmmu_template.h
> index 208f808..7029a03 100644
> --- a/softmmu_template.h
> +++ b/softmmu_template.h
> @@ -370,6 +370,32 @@ static inline void glue(io_write, SUFFIX)(CPUArchState 
> *env,
>   iotlbentry->attrs);
>  }
>
> +static inline void glue(helper_le_st_name, _do_unl_access)(CPUArchState *env,
> +   DATA_TYPE val,
> +   target_ulong addr,
> +   TCGMemOpIdx oi,
> +   unsigned mmu_idx,
> +   uintptr_t retaddr)
> +{
> +int i;
> +
> +if ((get_memop(oi) & MO_AMASK) == MO_ALIGN) {
> +cpu_unaligned_access(ENV_GET_CPU(env), addr, MMU_DATA_STORE,
> + mmu_idx, retaddr);
> +}
> +/* XXX: not efficient, but simple */
> +/* Note: relies on the fact that tlb_fill() does not remove the
> + * previous page from the TLB cache.  */
> +for (i = DATA_SIZE - 1; i >= 0; i--) {
> +/* Little-endian extract.  */
> +uint8_t val8 = val >> (i * 8);
> +/* Note the adjustment at the beginning of the function.
> +   Undo that for the recursion.  */
> +glue(helper_ret_stb, MMUSUFFIX)(env, addr + i, val8,
> +oi, retaddr + GETPC_ADJ);
> +}
> +}
> +

I still think there is some mileage in combining the unaligned stuff but
as no maintainer spoke out before or against last time I'll leave that
for future clean-ups.

Reviewed-by: Alex Bennée 

>  void helper_le_st_name(CPUArchState *env, target_ulong addr, DATA_TYPE val,
> TCGMemOpIdx oi, uintptr_t retaddr)
>  {
> @@ -399,7 +425,8 @@ void helper_le_st_name(CPUArchState *env, target_ulong 
> addr, DATA_TYPE val,
>  if (unlikely(tlb_addr & ~TARGET_PAGE_MASK)) {
>  CPUIOTLBEntry *iotlbentry;
>  if ((addr & (DATA_SIZE - 1)) != 0) {
> -goto do_unaligned_access;
> +glue(helper_le_st_name, _do_unl_access)(env, val, addr, mmu_idx,
> +oi, retaddr);
>  }
>  iotlbentry = >iotlb[mmu_idx][index];
>
> @@ -414,23 +441,8 @@ void helper_le_st_name(CPUArchState *env, target_ulong 
> addr, DATA_TYPE val,
>  if (DATA_SIZE > 1
>  && unlikely((addr & ~TARGET_PAGE_MASK) + DATA_SIZE - 1
>   >= TARGET_PAGE_SIZE)) {
> -int i;
> -do_unaligned_access:
> -if ((get_memop(oi) & MO_AMASK) == MO_ALIGN) {
> -cpu_unaligned_access(ENV_GET_CPU(env), addr, MMU_DATA_STORE,
> - mmu_idx, retaddr);
> -}
> -/* XXX: not efficient, but simple */
> -/* Note: relies on the fact that tlb_fill() does not remove the
> - * previous page from the TLB cache.  */
> -for (i = DATA_SIZE - 1; i >= 0; i--) {
> -/* Little-endian extract.  */
> -uint8_t val8 = val >> (i * 8);
> -/* Note the adjustment at the beginning of the function.
> -   Undo that for the recursion.  */
> -glue(helper_ret_stb, MMUSUFFIX)(env, addr + i, val8,
> -oi, retaddr + GETPC_ADJ);
> -}
> +glue(helper_le_st_name, _do_unl_access)(env, val, addr, mmu_idx,
> +oi, retaddr);
>  return;
>  }
>
> @@ -450,6 +462,32 @@ void helper_le_st_name(CPUArchState *env, target_ulong 
> addr, DATA_TYPE val,
>  }
>
>  #if DATA_SIZE > 1
> +static inline void glue(helper_be_st_name, _do_unl_access)(CPUArchState *env,
> +   DATA_TYPE val,
> +   target_ulong addr,
> +   

Re: [Qemu-devel] [PATCH] linux-user: Don't assert if guest tries shmdt(0)

2016-02-11 Thread Laurent Vivier


Le 09/02/2016 16:57, Peter Maydell a écrit :
> Our implementation of shmat() and shmdt() for linux-user was
> using "zero guest address" as its marker for "entry in the
> shm_regions[] array is not in use". This meant that if the
> guest did a shmdt(0) we would match on an unused array entry
> and call page_set_flags() with both start and end addresses zero,
> which causes an assertion failure.
> 
> Use an explicit in_use flag to manage the shm_regions[] array,
> so that we avoid this problem.
> 
> Signed-off-by: Peter Maydell 
> Reported-by: Pavel Shamis 
Reviewed-by: Laurent Vivier 

> ---
>  linux-user/syscall.c | 12 +++-
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 54ce14a..f46abf7 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -2598,8 +2598,9 @@ static abi_long do_socketcall(int num, abi_ulong vptr)
>  #define N_SHM_REGIONS32
>  
>  static struct shm_region {
> -abi_ulongstart;
> -abi_ulongsize;
> +abi_ulong start;
> +abi_ulong size;
> +bool in_use;
>  } shm_regions[N_SHM_REGIONS];
>  
>  struct target_semid_ds
> @@ -3291,7 +3292,8 @@ static inline abi_ulong do_shmat(int shmid, abi_ulong 
> shmaddr, int shmflg)
> ((shmflg & SHM_RDONLY)? 0 : PAGE_WRITE));
>  
>  for (i = 0; i < N_SHM_REGIONS; i++) {
> -if (shm_regions[i].start == 0) {
> +if (!shm_regions[i].in_use) {
> +shm_regions[i].in_use = true;
>  shm_regions[i].start = raddr;
>  shm_regions[i].size = shm_info.shm_segsz;
>  break;
> @@ -3308,8 +3310,8 @@ static inline abi_long do_shmdt(abi_ulong shmaddr)
>  int i;
>  
>  for (i = 0; i < N_SHM_REGIONS; ++i) {
> -if (shm_regions[i].start == shmaddr) {
> -shm_regions[i].start = 0;
> +if (shm_regions[i].in_use && shm_regions[i].start == shmaddr) {
> +shm_regions[i].in_use = false;
>  page_set_flags(shmaddr, shmaddr + shm_regions[i].size, 0);
>  break;
>  }
> 



[Qemu-devel] [Bug 1544524] [NEW] "info chardev" not showing the real port in use

2016-02-11 Thread Prem Anand
Public bug reported:

With Qemu 2.1.2
===
pharidos@uks2:~$ qemu-system-x86_64 -hda Disk.qcow2 -serial 
telnet::0,server,nowait -nographic
QEMU 2.1.2 monitor - type 'help' for more information
(qemu) info chardev
parallel0: filename=null
serial0: filename=telnet:0.0.0.0:44189,server
  ^^^ serial console is using port 44189
compat_monitor0: filename=stdio
(qemu)

With Qemu 2.5.0
===
pharidos@uks2:~$ qemu-system-x86_64 -hda Disk.qcow2 -serial  
telnet::0,server,nowait -nographic
QEMU 2.5.0 monitor - type 'help' for more information
(qemu) info chardev
parallel0: filename=null
serial0: filename=disconnected:telnet::0,server
  ^ serial console port not shown
compat_monitor0: filename=stdio
(qemu)

** Affects: qemu
 Importance: Undecided
 Status: New

** Description changed:

  With Qemu 2.1.2
- ==
+ ===
  pharidos@uks2:~/work/tplaf/☸ qemu-system-x86_64 -hda 
/space/pharidos/Disks/Blank_disk.qcow2 -serial telnet::0,server,nowait 
-nographic
  QEMU 2.1.2 monitor - type 'help' for more information
  (qemu) info chardev
  parallel0: filename=null
- serial0: filename=telnet:0.0.0.0:44189,server   <<<< serial console is 
using port 44189
+ serial0: filename=telnet:0.0.0.0:44189,server
+   ^^^ serial console is using port 
44189
  compat_monitor0: filename=stdio
- (qemu) 
- 
+ (qemu)
  
  With Qemu 2.5.0
- ==
+ ===
  pharidos@kvm:~/$ qemu-system-x86_64 -hda 
/space/pharidos/Disks/Blank_disk.qcow2 -serial telnet::0,server,nowait 
-nographic
  QEMU 2.5.0 monitor - type 'help' for more information
  (qemu) info chardev
  parallel0: filename=null
- serial0: filename=disconnected:telnet::0,server <<<< serial console port 
not shown
+ serial0: filename=disconnected:telnet::0,server 
+   ^ serial console port not shown
  compat_monitor0: filename=stdio
  (qemu)

** Description changed:

  With Qemu 2.1.2
  ===
- pharidos@uks2:~/work/tplaf/☸ qemu-system-x86_64 -hda 
/space/pharidos/Disks/Blank_disk.qcow2 -serial telnet::0,server,nowait 
-nographic
+ pharidos@uks2:~$ qemu-system-x86_64 -hda Disk.qcow2 -serial 
telnet::0,server,nowait -nographic
  QEMU 2.1.2 monitor - type 'help' for more information
  (qemu) info chardev
  parallel0: filename=null
  serial0: filename=telnet:0.0.0.0:44189,server
-   ^^^ serial console is using port 
44189
+   ^^^ serial console is using port 
44189
  compat_monitor0: filename=stdio
  (qemu)
  
  With Qemu 2.5.0
  ===
- pharidos@kvm:~/$ qemu-system-x86_64 -hda 
/space/pharidos/Disks/Blank_disk.qcow2 -serial telnet::0,server,nowait 
-nographic
+ pharidos@uks2:~$ qemu-system-x86_64 -hda Disk.qcow2 -serial  
telnet::0,server,nowait -nographic
  QEMU 2.5.0 monitor - type 'help' for more information
  (qemu) info chardev
  parallel0: filename=null
- serial0: filename=disconnected:telnet::0,server 
-   ^ serial console port not shown
+ serial0: filename=disconnected:telnet::0,server
+   ^ serial console port not shown
  compat_monitor0: filename=stdio
  (qemu)

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

Title:
  "info chardev" not showing the real port in use

Status in QEMU:
  New

Bug description:
  With Qemu 2.1.2
  ===
  pharidos@uks2:~$ qemu-system-x86_64 -hda Disk.qcow2 -serial 
telnet::0,server,nowait -nographic
  QEMU 2.1.2 monitor - type 'help' for more information
  (qemu) info chardev
  parallel0: filename=null
  serial0: filename=telnet:0.0.0.0:44189,server
    ^^^ serial console is using port 
44189
  compat_monitor0: filename=stdio
  (qemu)

  With Qemu 2.5.0
  ===
  pharidos@uks2:~$ qemu-system-x86_64 -hda Disk.qcow2 -serial  
telnet::0,server,nowait -nographic
  QEMU 2.5.0 monitor - type 'help' for more information
  (qemu) info chardev
  parallel0: filename=null
  serial0: filename=disconnected:telnet::0,server
    ^ serial console port not shown
  compat_monitor0: filename=stdio
  (qemu)

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



[Qemu-devel] [PULL 00/14] Trivial patches for 2016-02-11

2016-02-11 Thread Michael Tokarev
Here's yet another trivial-patches pull request.  There aren't many
patches in there this time, even if the previous pull request was
quite some time ago.

Speaking of the pull requests.  Maybe it is better to use the
same tag all the time, instead of separate tags for every pull?

Thanks,

/mjt

The following changes since commit 88c73d16ad1b6c22a2ab082064d0d521f756296a:

  memory: fix usage of find_next_bit and find_next_zero_bit (2016-02-10 
22:38:24 +)

are available in the git repository at:

  git://git.corpit.ru/qemu.git tags/pull-trivial-patches-2016-02-11

for you to fetch changes up to 1834ed3afc578b8dbf39838cfdf27d457771a334:

  w32: include winsock2.h before windows.h (2016-02-11 15:15:47 +0300)


trivial patches for 2016-02-11


Alistair Francis (1):
  qom: Correct object_property_get_int() description

Cao jin (3):
  ES1370: QOMify
  Emulated CCID card: QOMify
  Passthru CCID card: QOMify

Christophe Fergeau (1):
  man: virtfs-proxy-helper: Rework awkward sentence

Daniel P. Berrange (1):
  char: fix parameter name / type in BSD codepath

Daniel Serpell (1):
  Adds keycode 86 to the hid_usage_keys translation table.

Michael Tokarev (2):
  remove libtool support
  s390x: remove s390-zipl.rom

Paolo Bonzini (3):
  cpu: cpu_save/cpu_load is no more
  qemu-sockets: simplify error handling
  w32: include winsock2.h before windows.h

Wei Yang (2):
  rdma: remove check on time_spent when calculating mbs
  qmp-spec: fix index in doc

 Makefile   |   1 -
 configure  |  86 +
 docs/qmp-spec.txt  |   2 +-
 exec.c |   6 ---
 fsdev/virtfs-proxy-helper.texi |   4 +-
 hw/audio/es1370.c  |  10 +++--
 hw/input/hid.c |   2 +-
 hw/s390x/ipl.c |   2 +-
 hw/s390x/s390-virtio.c |   1 -
 hw/usb/ccid-card-emulated.c|  23 ++-
 hw/usb/ccid-card-passthru.c|  14 ---
 include/qemu-common.h  |   6 ---
 include/qom/object.h   |   2 +-
 include/sysemu/os-win32.h  |   2 +-
 migration/migration.c  |   4 +-
 pc-bios/README |   4 --
 pc-bios/s390-zipl.rom  | Bin 3304 -> 0 bytes
 qemu-char.c|   4 +-
 qemu-doc.texi  |   1 -
 rules.mak  |  18 -
 util/qemu-sockets.c|   6 +--
 21 files changed, 44 insertions(+), 154 deletions(-)
 delete mode 100644 pc-bios/s390-zipl.rom



[Qemu-devel] [PULL 12/14] s390x: remove s390-zipl.rom

2016-02-11 Thread Michael Tokarev
This is an s390 boot rom which was used in s390-virtio machine.
but since commit 3538fb6f89dd9bb2e7e59de2bfad52a45321c744
"s390x: remove s390-virtio machine", this file isn't used.
The only place it is referenced in the code is an unused
define ZIPL_FILENAME.  There's also comment in hw/s390/ipl.c
which I'm modifying too, to refer to s390-ccw.img instead.

Signed-off-by: Michael Tokarev 
Acked-by: Christian Borntraeger 
Acked-by: Cornelia Huck 
---
 Makefile   |   1 -
 hw/s390x/ipl.c |   2 +-
 hw/s390x/s390-virtio.c |   1 -
 pc-bios/README |   4 
 pc-bios/s390-zipl.rom  | Bin 3304 -> 0 bytes
 5 files changed, 1 insertion(+), 7 deletions(-)
 delete mode 100644 pc-bios/s390-zipl.rom

diff --git a/Makefile b/Makefile
index 30b1b2d..f9fae3a 100644
--- a/Makefile
+++ b/Makefile
@@ -400,7 +400,6 @@ efi-pcnet.rom efi-rtl8139.rom efi-virtio.rom \
 qemu-icon.bmp qemu_logo_no_text.svg \
 bamboo.dtb petalogix-s3adsp1800.dtb petalogix-ml605.dtb \
 multiboot.bin linuxboot.bin kvmvapic.bin \
-s390-zipl.rom \
 s390-ccw.img \
 spapr-rtas.bin slof.bin \
 palcode-clipper \
diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index 6992add..c9cf7cc 100644
--- a/hw/s390x/ipl.c
+++ b/hw/s390x/ipl.c
@@ -106,7 +106,7 @@ static void s390_ipl_realize(DeviceState *dev, Error **errp)
 /* Adjust ELF start address to final location */
 ipl->bios_start_addr += fwbase;
 } else {
-/* Try to load non-ELF file (e.g. s390-zipl.rom) */
+/* Try to load non-ELF file (e.g. s390-ccw.img) */
 bios_size = load_image_targphys(bios_filename, ZIPL_IMAGE_START,
 4096);
 ipl->bios_start_addr = ZIPL_IMAGE_START;
diff --git a/hw/s390x/s390-virtio.c b/hw/s390x/s390-virtio.c
index 9efb9c6..c320878 100644
--- a/hw/s390x/s390-virtio.c
+++ b/hw/s390x/s390-virtio.c
@@ -54,7 +54,6 @@
 #endif
 
 #define MAX_BLK_DEVS10
-#define ZIPL_FILENAME   "s390-zipl.rom"
 #define S390_MACHINE"s390-virtio"
 #define TYPE_S390_MACHINE   MACHINE_TYPE_NAME(S390_MACHINE)
 
diff --git a/pc-bios/README b/pc-bios/README
index d260c1b..9f65ffa 100644
--- a/pc-bios/README
+++ b/pc-bios/README
@@ -35,10 +35,6 @@
10ec:8139 -> pxe-rtl8139.rom
1af4:1000 -> pxe-virtio.rom
 
-- The S390 zipl loader is an addition to the official IBM s390-tools
-  package. That fork is maintained in its own git repository at:
-  git://repo.or.cz/s390-tools.git
-
 - The sources for the Alpha palcode image is available from:
   git://github.com/rth7680/qemu-palcode.git
 
diff --git a/pc-bios/s390-zipl.rom b/pc-bios/s390-zipl.rom
deleted file mode 100644
index 
3115128efe465a024b2deb780573358ae1b829a4..
GIT binary patch
literal 0
HcmV?d1

literal 3304
zcmZuzdvH`&8UOC?hP%xJ4gtbake-C44GpDB0K-eVH#`h2rHiy0klJiXfJjJU*wBL2
zu98~fSXolRYISA>mgBTqR@7m3wA0&`I)nZJR#L3~;o=?oM|YfFXzU;XuD|ar0V>{^
z`#9hEp1<#LuDZ07)R6y-Vy9C#g~;
zyiW#_?m-n26a}9nAQk{6b4=2>V>KlANSdN%kze)ESiF5IK@-#vl{v!wIqmBgG(I<;
zszC{;8J?$xMOxbrnAZjk_ed-V3;$L_-iM}H_`TaqVYS8}c=HzSU*iryU+sP{4^w
zqAE3m$cYFa?aMt+;anBfG)3%Dg!bB3ds3|_MNk4%q3$HZk#>-Rp?R5xYhK7w
zYsiR^3`T_wf*i
zg!=^K4YiU)zfE^I615YyB5pbcEVRB2;N6JTVYYGYUyxV`+pF~6YPHVrYx+6J9ETP|
z(;GqeY5JgEFJzyzkJUh<7MMu)IHd|l857{Za_s9d)8y{(|E(~MB$XHDP=v}=9aj>_
z$y_;2<2Y`{QW>fSH==U%xj18M5#*7d)|?GgHl#3`f(d>R}35
z%VMNPlh2wf$U3BJq!(*H`sZ~gk_}OKKedn(%?b)T`7AojNu$xiPAnTl-V^l7>(!?T
zwd+$S=rAx9w9kpfkOOpcy%S4j;vbbrdL?*HSJ
z_U5Gl16D=ePbpyYVXl06$7^$Pls%Y~TDs2Hwne_4~16qW7~UiqR{wWR%~
z;EO=v|+iL7lUO0ocxH9426A5%)W$j-C)#PM`}H
z>pfHRipm=Jjs8WI4(a}^)AVjTLS(#)`a-VV3_7CL0RAARCQSey!NzD>BM|9)yKO
zxbMCm%Mz_)_3ber_l8C?%yHt3d4tbFK4W}!+}ljeypfdZq%7u?XG0Y#k-I=#d;$62>qdEQUv0se1B;fe{~H!-Ev<~b@&2ISP3wse)E3V0-jy@^$>!5_
zla;k4|@C;r$9HmE?L8>^~PA7QC=i~ydaS<|*plRwPyqb~%4_5_t*K-WaiTq4a
zrwkY8-g##+W!G-mOBg=$E(r1^3I^%$I6>*!dbiyYtyuCXtEGd
zLlMJ4zqnOLmFBHt>w|w*%L`k#_{!wIV{-L!C0bG1Ln0q9uhW
zo;3PQ46rr!uPXiw@MHW0^G@mQ?t3n6e&_kaU!JwNwyyf_Z|~{<
z?$5qwzHsjG@Vzq@edWIU_PCZ!%-foc_d;2h0>pnr3&)!fDZaK38MLpWN8mwrTU0

[Qemu-devel] [PULL 14/14] w32: include winsock2.h before windows.h

2016-02-11 Thread Michael Tokarev
From: Paolo Bonzini 

Recent Fedora complains while compiling ui/sdl.c:

/usr/x86_64-w64-mingw32/sys-root/mingw/include/winsock2.h:15:2: warning: 
#warning Please include winsock2.h before windows.h [-Wcpp]

And with this patch we dutifully obey.

Stefan Weil:

Without that patch, windows.h will include winsock.h
(which conflicts with winsock2.h) when compiling sdl.c.

Normally we define WIN32_LEAN_AND_MEAN, and
windows.h won't include winsock.h.

include/ui/sdl2.h and ui/sdl.c undefine that macro,
so the order of the include files is important.

Signed-off-by: Paolo Bonzini 
Reviewed-by: Stefan Weil 
Signed-off-by: Michael Tokarev 
---
 include/sysemu/os-win32.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h
index 400e098..fbed346 100644
--- a/include/sysemu/os-win32.h
+++ b/include/sysemu/os-win32.h
@@ -26,8 +26,8 @@
 #ifndef QEMU_OS_WIN32_H
 #define QEMU_OS_WIN32_H
 
-#include 
 #include 
+#include 
 
 /* Workaround for older versions of MinGW. */
 #ifndef ECONNREFUSED
-- 
2.1.4




[Qemu-devel] [PATCH v2 4/4] hw/ppc/spapr: Implement the h_page_init hypercall

2016-02-11 Thread Thomas Huth
This hypercall either initializes a page with zeros, or copies
another page.
According to LoPAPR, the i-cache of the page should also be
flushed if using H_ICACHE_INVALIDATE or H_ICACHE_SYNCHRONIZE,
and the d-cache should be synchronized to the RAM if the
H_ICACHE_SYNCHRONIZE flag is used. For this, two new macros
are introduced, kvmppc_dcbst() and kvmppc_icbi(), which use
the corresponding assembler instructions to flush the caches
if running with KVM on Power. If the code runs with TCG instead,
the code only uses tb_flush(), assuming that this will be
enough for synchronization.

Signed-off-by: Thomas Huth 
---
 hw/ppc/spapr_hcall.c | 70 
 target-ppc/kvm_ppc.h | 24 --
 2 files changed, 92 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 6e9b6be..b6d9eee 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -386,6 +386,75 @@ static target_ulong h_set_xdabr(PowerPCCPU *cpu, 
sPAPRMachineState *spapr,
 return H_SUCCESS;
 }
 
+static target_ulong h_page_init(PowerPCCPU *cpu, sPAPRMachineState *spapr,
+target_ulong opcode, target_ulong *args)
+{
+CPUPPCState *env = >env;
+target_ulong flags = args[0];
+hwaddr dst = args[1];
+hwaddr src = args[2];
+hwaddr len = TARGET_PAGE_SIZE;
+uint8_t *pdst, *psrc, *ptr;
+
+if (flags & ~(H_ICACHE_SYNCHRONIZE | H_ICACHE_INVALIDATE
+  | H_COPY_PAGE | H_ZERO_PAGE)) {
+qemu_log_mask(LOG_UNIMP, "h_page_init: Bad flags (" TARGET_FMT_lx "\n",
+  flags);
+return H_PARAMETER;
+}
+
+if (!is_ram_address(spapr, dst) || (dst & ~TARGET_PAGE_MASK) != 0) {
+return H_PARAMETER;
+}
+
+/* Map-in source */
+if (flags & H_COPY_PAGE) {
+if (!is_ram_address(spapr, src) || (src & ~TARGET_PAGE_MASK) != 0) {
+return H_PARAMETER;
+}
+psrc = cpu_physical_memory_map(src, , 0);
+if (!psrc || len != TARGET_PAGE_SIZE) {
+return H_HARDWARE;
+}
+}
+
+/* Map-in destination */
+pdst = cpu_physical_memory_map(dst, , 1);
+if (!pdst || len != TARGET_PAGE_SIZE) {
+if (flags & H_COPY_PAGE) {
+cpu_physical_memory_unmap(psrc, len, 0, 0);
+}
+return H_HARDWARE;
+}
+
+if (flags & H_ZERO_PAGE) {
+memset(pdst, 0, len);
+}
+if (flags & H_COPY_PAGE) {
+memcpy(pdst, psrc, len);
+cpu_physical_memory_unmap(psrc, len, 0, len);
+}
+
+if (kvm_enabled() && (flags & H_ICACHE_SYNCHRONIZE) != 0) {
+for (ptr = pdst; ptr < pdst + len; ptr += env->dcache_line_size) {
+kvmppc_dcbst(ptr);
+}
+}
+
+if (flags & (H_ICACHE_SYNCHRONIZE | H_ICACHE_INVALIDATE)) {
+if (kvm_enabled()) {
+for (ptr = pdst; ptr < pdst + len; ptr += env->icache_line_size) {
+kvmppc_icbi(ptr);
+}
+} else {
+tb_flush(CPU(cpu));
+}
+}
+
+cpu_physical_memory_unmap(pdst, len, 1, len);
+return H_SUCCESS;
+}
+
 #define FLAGS_REGISTER_VPA 0x2000ULL
 #define FLAGS_REGISTER_DTL 0x4000ULL
 #define FLAGS_REGISTER_SLBSHADOW   0x6000ULL
@@ -1045,6 +1114,7 @@ static void hypercall_register_types(void)
 spapr_register_hypercall(H_SET_SPRG0, h_set_sprg0);
 spapr_register_hypercall(H_SET_DABR, h_set_dabr);
 spapr_register_hypercall(H_SET_XDABR, h_set_xdabr);
+spapr_register_hypercall(H_PAGE_INIT, h_page_init);
 spapr_register_hypercall(H_SET_MODE, h_set_mode);
 
 /* "debugger" hcalls (also used by SLOF). Note: We do -not- differenciate
diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h
index 62406ce..2354aaf 100644
--- a/target-ppc/kvm_ppc.h
+++ b/target-ppc/kvm_ppc.h
@@ -254,15 +254,35 @@ static inline int kvmppc_enable_hwrng(void)
 #endif
 
 #ifndef CONFIG_KVM
+
 #define kvmppc_eieio() do { } while (0)
-#else
+#define kvmppc_dcbst(addr) do { } while (0)
+#define kvmppc_icbi(addr) do { } while (0)
+
+#else   /* CONFIG_KVM */
+
 #define kvmppc_eieio() \
 do {  \
 if (kvm_enabled()) {  \
 asm volatile("eieio" : : : "memory"); \
 } \
 } while (0)
-#endif
+
+#define kvmppc_dcbst(addr) \
+do { \
+if (kvm_enabled()) { \
+asm volatile("dcbst 0,%0" : : "r"(addr)); \
+} \
+} while (0)
+
+#define kvmppc_icbi(addr) \
+do { \
+if (kvm_enabled()) { \
+asm volatile("icbi 0,%0" : : "r"(addr)); \
+} \
+} while (0)
+
+#endif  /* CONFIG_KVM */
 
 #ifndef KVM_INTERRUPT_SET
 #define KVM_INTERRUPT_SET -1
-- 
1.8.3.1




Re: [Qemu-devel] Memory on stellaris board

2016-02-11 Thread Peter Maydell
On 11 February 2016 at 12:46, Aurelio Remonda
 wrote:
> On Fri, Feb 5, 2016 at 2:00 PM, Peter Maydell  
> wrote:
>> The right way to do this is to set the MachineClass default_ram_size
>> to what you want your default value to be. Then you should calculate
>> the dc0 etc values to expose to the guest based on the ram size
>> (which might be the default or might be user specified).
>
> But stellaris board has a hardcoded dc0 value, when you place the -m flag
> with for example 256M you shouldn't calculate any sram_size from dc0, you
> just assign that as your ram size.

What I am suggesting is that dc0 should not be hardcoded, but
that you should work out what dc0 to use based on the user
provided RAM size.

thanks
-- PMM



Re: [Qemu-devel] [PATCH v2 4/4] replay: introduce block devices record/replay

2016-02-11 Thread Stefan Hajnoczi
On Wed, Feb 10, 2016 at 12:13:23PM +0300, Pavel Dovgalyuk wrote:
> @@ -784,7 +798,11 @@ BlockAIOCB *blk_aio_flush(BlockBackend *blk,
>  return blk_abort_aio_request(blk, cb, opaque, -ENOMEDIUM);
>  }
>  
> -return bdrv_aio_flush(blk->bs, cb, opaque);
> +if (replay_mode == REPLAY_MODE_NONE) {
> +return bdrv_aio_flush(blk->bs, cb, opaque);
> +} else {
> +return replay_aio_flush(blk->bs, cb, opaque);
> +}
>  }

I am only looking at this patch in isolation and am not familiar with
the record/replay work, but these changes appear invasive.  In order for
record/replay to keep working in the future, everyone touching block
layer code must be familiar with the principles of how record/replay
works.  This patch does not include documentation.

Can you point me to documentation that explains the how record replay
works?

Thanks,
Stefan


signature.asc
Description: PGP signature


Re: [Qemu-devel] [PATCH v2 4/4] replay: introduce block devices record/replay

2016-02-11 Thread Pavel Dovgalyuk
> From: Stefan Hajnoczi [mailto:stefa...@redhat.com]
> On Wed, Feb 10, 2016 at 12:13:23PM +0300, Pavel Dovgalyuk wrote:
> > @@ -784,7 +798,11 @@ BlockAIOCB *blk_aio_flush(BlockBackend *blk,
> >  return blk_abort_aio_request(blk, cb, opaque, -ENOMEDIUM);
> >  }
> >
> > -return bdrv_aio_flush(blk->bs, cb, opaque);
> > +if (replay_mode == REPLAY_MODE_NONE) {
> > +return bdrv_aio_flush(blk->bs, cb, opaque);
> > +} else {
> > +return replay_aio_flush(blk->bs, cb, opaque);
> > +}
> >  }
> 
> I am only looking at this patch in isolation and am not familiar with
> the record/replay work, but these changes appear invasive.  In order for
> record/replay to keep working in the future, everyone touching block
> layer code must be familiar with the principles of how record/replay
> works.  This patch does not include documentation.

We've already discussed it with Kevin.
He proposes adding new block driver for record/replay.

> Can you point me to documentation that explains the how record replay
> works?

There is some information about it in docs/replay.txt and in
http://wiki.qemu.org/Features/record-replay

Pavel Dovgalyuk




Re: [Qemu-devel] [RFC v7 04/16] softmmu: Simplify helper_*_st_name, wrap RAM code

2016-02-11 Thread Alex Bennée

Alvise Rigo  writes:

> Attempting to simplify the helper_*_st_name, wrap the code relative to a
> RAM access into an inline function.
>
> Based on this work, Alex proposed the following patch series
> https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg01136.html
> that reduces code duplication of the softmmu_helpers.
>
> Suggested-by: Jani Kokkonen 
> Suggested-by: Claudio Fontana 
> Signed-off-by: Alvise Rigo 
> ---
>  softmmu_template.h | 110 
> +
>  1 file changed, 68 insertions(+), 42 deletions(-)
>
> diff --git a/softmmu_template.h b/softmmu_template.h
> index 3d388ec..6279437 100644
> --- a/softmmu_template.h
> +++ b/softmmu_template.h
> @@ -416,13 +416,46 @@ static inline void glue(helper_le_st_name, 
> _do_mmio_access)(CPUArchState *env,
>  glue(io_write, SUFFIX)(env, iotlbentry, val, addr, retaddr);
>  }
>
> +static inline void glue(helper_le_st_name, _do_ram_access)(CPUArchState *env,
> +   DATA_TYPE val,
> +   target_ulong addr,
> +   TCGMemOpIdx oi,
> +   unsigned mmu_idx,
> +   int index,
> +   uintptr_t retaddr)
> +{
> +uintptr_t haddr;
> +
> +/* Handle slow unaligned access (it spans two pages or IO).  */
> +if (DATA_SIZE > 1
> +&& unlikely((addr & ~TARGET_PAGE_MASK) + DATA_SIZE - 1
> + >= TARGET_PAGE_SIZE)) {
> +glue(helper_le_st_name, _do_unl_access)(env, val, addr, oi, mmu_idx,
> +retaddr);
> +return;
> +}
> +
> +/* Handle aligned access or unaligned access in the same page.  */
> +if ((addr & (DATA_SIZE - 1)) != 0
> +&& (get_memop(oi) & MO_AMASK) == MO_ALIGN) {
> +cpu_unaligned_access(ENV_GET_CPU(env), addr, MMU_DATA_STORE,
> + mmu_idx, retaddr);
> +}
> +
> +haddr = addr + env->tlb_table[mmu_idx][index].addend;
> +#if DATA_SIZE == 1
> +glue(glue(st, SUFFIX), _p)((uint8_t *)haddr, val);
> +#else
> +glue(glue(st, SUFFIX), _le_p)((uint8_t *)haddr, val);
> +#endif
> +}
> +
>  void helper_le_st_name(CPUArchState *env, target_ulong addr, DATA_TYPE val,
> TCGMemOpIdx oi, uintptr_t retaddr)
>  {
>  unsigned mmu_idx = get_mmuidx(oi);
>  int index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1);
>  target_ulong tlb_addr = env->tlb_table[mmu_idx][index].addr_write;
> -uintptr_t haddr;
>
>  /* Adjust the given return address.  */
>  retaddr -= GETPC_ADJ;
> @@ -448,28 +481,8 @@ void helper_le_st_name(CPUArchState *env, target_ulong 
> addr, DATA_TYPE val,
>  return;
>  }
>
> -/* Handle slow unaligned access (it spans two pages or IO).  */
> -if (DATA_SIZE > 1
> -&& unlikely((addr & ~TARGET_PAGE_MASK) + DATA_SIZE - 1
> - >= TARGET_PAGE_SIZE)) {
> -glue(helper_le_st_name, _do_unl_access)(env, val, addr, mmu_idx,
> -oi, retaddr);
> -return;
> -}
> -
> -/* Handle aligned access or unaligned access in the same page.  */
> -if ((addr & (DATA_SIZE - 1)) != 0
> -&& (get_memop(oi) & MO_AMASK) == MO_ALIGN) {
> -cpu_unaligned_access(ENV_GET_CPU(env), addr, MMU_DATA_STORE,
> - mmu_idx, retaddr);
> -}
> -
> -haddr = addr + env->tlb_table[mmu_idx][index].addend;
> -#if DATA_SIZE == 1
> -glue(glue(st, SUFFIX), _p)((uint8_t *)haddr, val);
> -#else
> -glue(glue(st, SUFFIX), _le_p)((uint8_t *)haddr, val);
> -#endif
> +glue(helper_le_st_name, _do_ram_access)(env, val, addr, oi, mmu_idx, 
> index,
> +retaddr);
>  }
>
>  #if DATA_SIZE > 1
> @@ -519,13 +532,42 @@ static inline void glue(helper_be_st_name, 
> _do_mmio_access)(CPUArchState *env,
>  glue(io_write, SUFFIX)(env, iotlbentry, val, addr, retaddr);
>  }
>
> +static inline void glue(helper_be_st_name, _do_ram_access)(CPUArchState *env,
> +   DATA_TYPE val,
> +   target_ulong addr,
> +   TCGMemOpIdx oi,
> +   unsigned mmu_idx,
> +   int index,
> +   uintptr_t retaddr)
> +{
> +uintptr_t haddr;
> +
> +/* Handle slow unaligned access (it spans two pages or IO).  */
> +

Re: [Qemu-devel] [RFC v7 01/16] exec.c: Add new exclusive bitmap to ram_list

2016-02-11 Thread alvise rigo
You are right, the for loop with i < DIRTY_MEMORY_NUM works just fine.

Thank you,
alvise

On Thu, Feb 11, 2016 at 2:00 PM, Alex Bennée  wrote:
>
> Alvise Rigo  writes:
>
>> The purpose of this new bitmap is to flag the memory pages that are in
>> the middle of LL/SC operations (after a LL, before a SC). For all these
>> pages, the corresponding TLB entries will be generated in such a way to
>> force the slow-path for all the VCPUs (see the following patches).
>>
>> When the system starts, the whole memory is set to dirty.
>>
>> Suggested-by: Jani Kokkonen 
>> Suggested-by: Claudio Fontana 
>> Signed-off-by: Alvise Rigo 
>> ---
>>  exec.c  |  7 +--
>>  include/exec/memory.h   |  3 ++-
>>  include/exec/ram_addr.h | 31 +++
>>  3 files changed, 38 insertions(+), 3 deletions(-)
>>
>> diff --git a/exec.c b/exec.c
>> index 7115403..51f366d 100644
>> --- a/exec.c
>> +++ b/exec.c
>> @@ -1575,11 +1575,14 @@ static ram_addr_t ram_block_add(RAMBlock *new_block, 
>> Error **errp)
>>  int i;
>>
>>  /* ram_list.dirty_memory[] is protected by the iothread lock.  */
>> -for (i = 0; i < DIRTY_MEMORY_NUM; i++) {
>> +for (i = 0; i < DIRTY_MEMORY_EXCLUSIVE; i++) {
>>  ram_list.dirty_memory[i] =
>>  bitmap_zero_extend(ram_list.dirty_memory[i],
>> old_ram_size, new_ram_size);
>> -   }
>> +}
>> +ram_list.dirty_memory[DIRTY_MEMORY_EXCLUSIVE] =
>> +
>> bitmap_zero_extend(ram_list.dirty_memory[DIRTY_MEMORY_EXCLUSIVE],
>> +   old_ram_size, new_ram_size);
>
> In the previous patch you moved this out of the loop as
> ram_list.dirty_memory[DIRTY_MEMORY_EXCLUSIVE] was a different size to
> the other dirty bitmaps. This no longer seems to be the case so this
> seems pointless.
>
>>  }
>>  cpu_physical_memory_set_dirty_range(new_block->offset,
>>  new_block->used_length,
>> diff --git a/include/exec/memory.h b/include/exec/memory.h
>> index c92734a..71e0480 100644
>> --- a/include/exec/memory.h
>> +++ b/include/exec/memory.h
>> @@ -19,7 +19,8 @@
>>  #define DIRTY_MEMORY_VGA   0
>>  #define DIRTY_MEMORY_CODE  1
>>  #define DIRTY_MEMORY_MIGRATION 2
>> -#define DIRTY_MEMORY_NUM   3/* num of dirty bits */
>> +#define DIRTY_MEMORY_EXCLUSIVE 3
>> +#define DIRTY_MEMORY_NUM   4/* num of dirty bits */
>>
>>  #include 
>>  #include 
>> diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
>> index ef1489d..19789fc 100644
>> --- a/include/exec/ram_addr.h
>> +++ b/include/exec/ram_addr.h
>> @@ -21,6 +21,7 @@
>>
>>  #ifndef CONFIG_USER_ONLY
>>  #include "hw/xen/xen.h"
>> +#include "sysemu/sysemu.h"
>>
>>  struct RAMBlock {
>>  struct rcu_head rcu;
>> @@ -172,6 +173,9 @@ static inline void 
>> cpu_physical_memory_set_dirty_range(ram_addr_t start,
>>  if (unlikely(mask & (1 << DIRTY_MEMORY_CODE))) {
>>  bitmap_set_atomic(d[DIRTY_MEMORY_CODE], page, end - page);
>>  }
>> +if (unlikely(mask & (1 << DIRTY_MEMORY_EXCLUSIVE))) {
>> +bitmap_set_atomic(d[DIRTY_MEMORY_EXCLUSIVE], page, end - page);
>> +}
>>  xen_modified_memory(start, length);
>>  }
>>
>> @@ -287,5 +291,32 @@ uint64_t cpu_physical_memory_sync_dirty_bitmap(unsigned 
>> long *dest,
>>  }
>>
>>  void migration_bitmap_extend(ram_addr_t old, ram_addr_t new);
>> +
>> +/* Exclusive bitmap support. */
>> +#define EXCL_BITMAP_GET_OFFSET(addr) (addr >> TARGET_PAGE_BITS)
>> +
>> +/* Make the page of @addr not exclusive. */
>> +static inline void cpu_physical_memory_unset_excl(ram_addr_t addr)
>> +{
>> +set_bit_atomic(EXCL_BITMAP_GET_OFFSET(addr),
>> +   ram_list.dirty_memory[DIRTY_MEMORY_EXCLUSIVE]);
>> +}
>> +
>> +/* Return true if the page of @addr is exclusive, i.e. the EXCL bit is set. 
>> */
>> +static inline int cpu_physical_memory_is_excl(ram_addr_t addr)
>> +{
>> +return !test_bit(EXCL_BITMAP_GET_OFFSET(addr),
>> + ram_list.dirty_memory[DIRTY_MEMORY_EXCLUSIVE]);
>> +}
>> +
>> +/* Set the page of @addr as exclusive clearing its EXCL bit and return the
>> + * previous bit's state. */
>> +static inline int cpu_physical_memory_set_excl(ram_addr_t addr)
>> +{
>> +return bitmap_test_and_clear_atomic(
>> +
>> ram_list.dirty_memory[DIRTY_MEMORY_EXCLUSIVE],
>> +EXCL_BITMAP_GET_OFFSET(addr), 1);
>> +}
>> +
>>  #endif
>>  #endif
>
>
> --
> Alex Bennée



[Qemu-devel] [PATCH v3 2/5] drivers/hv: Move VMBus hypercall codes into Hyper-V UAPI header

2016-02-11 Thread Andrey Smetanin
VMBus hypercall codes inside Hyper-V UAPI header will
be used by QEMU to implement VMBus host devices support.

Signed-off-by: Andrey Smetanin 
Acked-by: K. Y. Srinivasan 
Reviewed-by: Roman Kagan 
CC: Gleb Natapov 
CC: Paolo Bonzini 
CC: Joerg Roedel 
CC: "K. Y. Srinivasan" 
CC: Haiyang Zhang 
CC: Roman Kagan 
CC: Denis V. Lunev 
CC: qemu-devel@nongnu.org
---
 arch/x86/include/uapi/asm/hyperv.h | 2 ++
 drivers/hv/connection.c| 2 +-
 drivers/hv/hv.c| 2 +-
 drivers/hv/hyperv_vmbus.h  | 6 --
 4 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/arch/x86/include/uapi/asm/hyperv.h 
b/arch/x86/include/uapi/asm/hyperv.h
index 0c50fab..bc1c8a9 100644
--- a/arch/x86/include/uapi/asm/hyperv.h
+++ b/arch/x86/include/uapi/asm/hyperv.h
@@ -227,6 +227,8 @@
 
 /* Declare the various hypercall operations. */
 #define HV_X64_HCALL_NOTIFY_LONG_SPIN_WAIT 0x0008
+#define HV_X64_HCALL_POST_MESSAGE  0x005c
+#define HV_X64_HCALL_SIGNAL_EVENT  0x005d
 
 #define HV_X64_MSR_APIC_ASSIST_PAGE_ENABLE 0x0001
 #define HV_X64_MSR_APIC_ASSIST_PAGE_ADDRESS_SHIFT  12
diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
index fa86b2c..84700c6 100644
--- a/drivers/hv/connection.c
+++ b/drivers/hv/connection.c
@@ -485,5 +485,5 @@ void vmbus_set_event(struct vmbus_channel *channel)
(child_relid >> 5));
}
 
-   hv_do_hypercall(HVCALL_SIGNAL_EVENT, channel->sig_event, NULL);
+   hv_do_hypercall(HV_X64_HCALL_SIGNAL_EVENT, channel->sig_event, NULL);
 }
diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
index ccb335f..48388dc 100644
--- a/drivers/hv/hv.c
+++ b/drivers/hv/hv.c
@@ -337,7 +337,7 @@ int hv_post_message(union hv_connection_id connection_id,
aligned_msg->payload_size = payload_size;
memcpy((void *)aligned_msg->payload, payload, payload_size);
 
-   status = hv_do_hypercall(HVCALL_POST_MESSAGE, aligned_msg, NULL);
+   status = hv_do_hypercall(HV_X64_HCALL_POST_MESSAGE, aligned_msg, NULL);
 
put_cpu();
return status & 0x;
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index b9ea7f5..1dabaa4 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -256,12 +256,6 @@ struct hv_monitor_page {
u8 rsvdz4[1984];
 };
 
-/* Declare the various hypercall operations. */
-enum hv_call_code {
-   HVCALL_POST_MESSAGE = 0x005c,
-   HVCALL_SIGNAL_EVENT = 0x005d,
-};
-
 /* Definition of the hv_post_message hypercall input structure. */
 struct hv_input_post_message {
union hv_connection_id connectionid;
-- 
2.4.3




Re: [Qemu-devel] [PATCH v6 2/4] trace: Remove unnecessary intermediate event copies

2016-02-11 Thread Lluís Vilanova
Lluís Vilanova writes:

> The current code forces the use of a chain of ".original" dereferences,
> which looks odd.

> Signed-off-by: Lluís Vilanova 
> ---
>  scripts/tracetool/__init__.py|4 +---
>  scripts/tracetool/format/events_h.py |4 ++--
>  scripts/tracetool/format/tcg_h.py|4 ++--
>  3 files changed, 5 insertions(+), 7 deletions(-)

> diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
> index 0663e7f..1bf9246 100644
> --- a/scripts/tracetool/__init__.py
> +++ b/scripts/tracetool/__init__.py
> @@ -6,7 +6,7 @@ Machinery for generating tracing-related intermediate files.
>  """
 
>  __author__ = "Lluís Vilanova "
> -__copyright__  = "Copyright 2012-2014, Lluís Vilanova "
> +__copyright__  = "Copyright 2012-2016, Lluís Vilanova "
>  __license__= "GPL version 2 or (at your option) any later version"
 
>  __maintainer__ = "Stefan Hajnoczi"
> @@ -287,8 +287,6 @@ def _read_events(fobj):
>  event.args):
>  if atrans == aorig:
>  args_trans.append(atrans)
> -event_trans.args = Arguments(args_trans)
> -event_trans = event_trans.copy()
 
>  event_exec = event.copy()
>  event_exec.name += "_exec"

I've detected an error in this piece. Please ignore this patch until v7.


Thanks,
  Lluis



Re: [Qemu-devel] [RFC v7 03/16] softmmu: Simplify helper_*_st_name, wrap MMIO code

2016-02-11 Thread Alex Bennée

Alvise Rigo  writes:

> Attempting to simplify the helper_*_st_name, wrap the MMIO code into an
> inline function.
>
> Based on this work, Alex proposed the following patch series
> https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg01136.html
> that reduces code duplication of the softmmu_helpers.
>
> Suggested-by: Jani Kokkonen 
> Suggested-by: Claudio Fontana 
> Signed-off-by: Alvise Rigo 
> ---
>  softmmu_template.h | 66 
> --
>  1 file changed, 44 insertions(+), 22 deletions(-)
>
> diff --git a/softmmu_template.h b/softmmu_template.h
> index 7029a03..3d388ec 100644
> --- a/softmmu_template.h
> +++ b/softmmu_template.h
> @@ -396,6 +396,26 @@ static inline void glue(helper_le_st_name, 
> _do_unl_access)(CPUArchState *env,
>  }
>  }
>
> +static inline void glue(helper_le_st_name, _do_mmio_access)(CPUArchState 
> *env,
> +DATA_TYPE val,
> +target_ulong 
> addr,
> +TCGMemOpIdx oi,
> +unsigned mmu_idx,
> +int index,
> +uintptr_t 
> retaddr)
> +{
> +CPUIOTLBEntry *iotlbentry = >iotlb[mmu_idx][index];
> +
> +if ((addr & (DATA_SIZE - 1)) != 0) {
> +glue(helper_le_st_name, _do_unl_access)(env, val, addr, mmu_idx,
> +oi, retaddr);
> +}
> +/* ??? Note that the io helpers always read data in the target
> +   byte ordering.  We should push the LE/BE request down into io.  */
> +val = TGT_LE(val);
> +glue(io_write, SUFFIX)(env, iotlbentry, val, addr, retaddr);
> +}
> +
>  void helper_le_st_name(CPUArchState *env, target_ulong addr, DATA_TYPE val,
> TCGMemOpIdx oi, uintptr_t retaddr)
>  {
> @@ -423,17 +443,8 @@ void helper_le_st_name(CPUArchState *env, target_ulong 
> addr, DATA_TYPE val,
>
>  /* Handle an IO access.  */
>  if (unlikely(tlb_addr & ~TARGET_PAGE_MASK)) {
> -CPUIOTLBEntry *iotlbentry;
> -if ((addr & (DATA_SIZE - 1)) != 0) {
> -glue(helper_le_st_name, _do_unl_access)(env, val, addr, mmu_idx,
> -oi, retaddr);
> -}
> -iotlbentry = >iotlb[mmu_idx][index];
> -
> -/* ??? Note that the io helpers always read data in the target
> -   byte ordering.  We should push the LE/BE request down into io.  */
> -val = TGT_LE(val);
> -glue(io_write, SUFFIX)(env, iotlbentry, val, addr, retaddr);
> +glue(helper_le_st_name, _do_mmio_access)(env, val, addr, oi,
> + mmu_idx, index, retaddr);
>  return;
>  }
>
> @@ -488,6 +499,26 @@ static inline void glue(helper_be_st_name, 
> _do_unl_access)(CPUArchState *env,
>  }
>  }
>
> +static inline void glue(helper_be_st_name, _do_mmio_access)(CPUArchState 
> *env,
> +DATA_TYPE val,
> +target_ulong 
> addr,
> +TCGMemOpIdx oi,
> +unsigned mmu_idx,
> +int index,
> +uintptr_t 
> retaddr)
> +{
> +CPUIOTLBEntry *iotlbentry = >iotlb[mmu_idx][index];
> +
> +if ((addr & (DATA_SIZE - 1)) != 0) {
> +glue(helper_be_st_name, _do_unl_access)(env, val, addr, mmu_idx,
> +oi, retaddr);
> +}
> +/* ??? Note that the io helpers always read data in the target
> +   byte ordering.  We should push the LE/BE request down into io.  */
> +val = TGT_BE(val);
> +glue(io_write, SUFFIX)(env, iotlbentry, val, addr, retaddr);
> +}
> +

As before I still thing there is millage in having a common helper
between LE/BE which the compiler can sort out. Having said that there is
less argument for this function as it is a bit smalled and you would
need a bit more faffing about.

Reviewed-by: Alex Bennée 

>  void helper_be_st_name(CPUArchState *env, target_ulong addr, DATA_TYPE val,
> TCGMemOpIdx oi, uintptr_t retaddr)
>  {
> @@ -515,17 +546,8 @@ void helper_be_st_name(CPUArchState *env, target_ulong 
> addr, DATA_TYPE val,
>
>  /* Handle an IO access.  */
>  if (unlikely(tlb_addr & ~TARGET_PAGE_MASK)) {
> -CPUIOTLBEntry *iotlbentry;
> -if ((addr & (DATA_SIZE - 1)) != 0) {
> -  

Re: [Qemu-devel] [RFC v7 06/16] qom: cpu: Add CPUClass hooks for exclusive range

2016-02-11 Thread Alex Bennée

Alvise Rigo  writes:

> The excl_protected_range is a hwaddr range set by the VCPU at the
> execution of a LoadLink instruction. If a normal access writes to this
> range, the corresponding StoreCond will fail.
>
> Each architecture can set the exclusive range when issuing the LoadLink
> operation through a CPUClass hook. This comes in handy to emulate, for
> instance, the exclusive monitor implemented in some ARM architectures
> (more precisely, the Exclusive Reservation Granule).
>
> In addition, add another CPUClass hook called to decide whether a
> StoreCond has to fail or not.
>
> Suggested-by: Jani Kokkonen 
> Suggested-by: Claudio Fontana 
> Signed-off-by: Alvise Rigo 
> ---
>  include/qom/cpu.h | 15 +++
>  qom/cpu.c | 20 
>  2 files changed, 35 insertions(+)
>
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index 2e5229d..682c81d 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -29,6 +29,7 @@
>  #include "qemu/queue.h"
>  #include "qemu/thread.h"
>  #include "qemu/typedefs.h"
> +#include "qemu/range.h"
>
>  typedef int (*WriteCoreDumpFunction)(const void *buf, size_t size,
>   void *opaque);
> @@ -183,6 +184,12 @@ typedef struct CPUClass {
>  void (*cpu_exec_exit)(CPUState *cpu);
>  bool (*cpu_exec_interrupt)(CPUState *cpu, int interrupt_request);
>
> +/* Atomic instruction handling */
> +void (*cpu_set_excl_protected_range)(CPUState *cpu, hwaddr addr,
> + hwaddr size);
> +int (*cpu_valid_excl_access)(CPUState *cpu, hwaddr addr,
> + hwaddr size);
> +
>  void (*disas_set_info)(CPUState *cpu, disassemble_info *info);
>  } CPUClass;
>
> @@ -219,6 +226,9 @@ struct kvm_run;
>  #define TB_JMP_CACHE_BITS 12
>  #define TB_JMP_CACHE_SIZE (1 << TB_JMP_CACHE_BITS)
>
> +/* Atomic insn translation TLB support. */
> +#define EXCLUSIVE_RESET_ADDR ULLONG_MAX
> +
>  /**
>   * CPUState:
>   * @cpu_index: CPU index (informative).
> @@ -341,6 +351,11 @@ struct CPUState {
>   */
>  bool throttle_thread_scheduled;
>
> +/* vCPU's exclusive addresses range.
> + * The address is set to EXCLUSIVE_RESET_ADDR if the vCPU is not
> + * in the middle of a LL/SC. */
> +struct Range excl_protected_range;
> +

In which case we should probably initialise that on CPU creation as we
don't start in the middle of a LL/SC.

>  /* Note that this is accessed at the start of every TB via a negative
> offset from AREG0.  Leave this field at the end so as to make the
> (absolute value) offset as small as possible.  This reduces code
> diff --git a/qom/cpu.c b/qom/cpu.c
> index 8f537a4..a5d360c 100644
> --- a/qom/cpu.c
> +++ b/qom/cpu.c
> @@ -203,6 +203,24 @@ static bool cpu_common_exec_interrupt(CPUState *cpu, int 
> int_req)
>  return false;
>  }
>
> +static void cpu_common_set_excl_range(CPUState *cpu, hwaddr addr, hwaddr 
> size)
> +{
> +cpu->excl_protected_range.begin = addr;
> +cpu->excl_protected_range.end = addr + size;
> +}
> +
> +static int cpu_common_valid_excl_access(CPUState *cpu, hwaddr addr, hwaddr 
> size)
> +{
> +/* Check if the excl range completely covers the access */
> +if (cpu->excl_protected_range.begin <= addr &&
> +cpu->excl_protected_range.end >= addr + size) {
> +
> +return 1;
> +}
> +
> +return 0;
> +}

This can be a bool function.

> +
>  void cpu_dump_state(CPUState *cpu, FILE *f, fprintf_function cpu_fprintf,
>  int flags)
>  {
> @@ -355,6 +373,8 @@ static void cpu_class_init(ObjectClass *klass, void *data)
>  k->cpu_exec_enter = cpu_common_noop;
>  k->cpu_exec_exit = cpu_common_noop;
>  k->cpu_exec_interrupt = cpu_common_exec_interrupt;
> +k->cpu_set_excl_protected_range = cpu_common_set_excl_range;
> +k->cpu_valid_excl_access = cpu_common_valid_excl_access;
>  dc->realize = cpu_common_realizefn;
>  /*
>   * Reason: CPUs still need special care by board code: wiring up


--
Alex Bennée



[Qemu-devel] [PATCH v3 4/5] kvm/x86: Reject Hyper-V hypercall continuation

2016-02-11 Thread Andrey Smetanin
Currently we do not support Hyper-V hypercall continuation
so reject it.

Signed-off-by: Andrey Smetanin 
Reviewed-by: Roman Kagan 
CC: Gleb Natapov 
CC: Paolo Bonzini 
CC: Joerg Roedel 
CC: "K. Y. Srinivasan" 
CC: Haiyang Zhang 
CC: Roman Kagan 
CC: Denis V. Lunev 
CC: qemu-devel@nongnu.org
---
 arch/x86/kvm/hyperv.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index 0e7c90f..e1daa8b 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -1083,6 +1083,12 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
 
trace_kvm_hv_hypercall(code, fast, rep_cnt, rep_idx, ingpa, outgpa);
 
+   /* Hypercall continuation is not supported yet */
+   if (rep_cnt || rep_idx) {
+   res = HV_STATUS_INVALID_HYPERCALL_CODE;
+   goto set_result;
+   }
+
switch (code) {
case HV_X64_HCALL_NOTIFY_LONG_SPIN_WAIT:
kvm_vcpu_on_spin(vcpu);
@@ -1092,6 +1098,7 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
break;
}
 
+set_result:
ret = res | (((u64)rep_done & 0xfff) << 32);
if (longmode) {
kvm_register_write(vcpu, VCPU_REGS_RAX, ret);
-- 
2.4.3




[Qemu-devel] [PATCH v3 0/5] KVM: Hyper-V VMBus hypercalls

2016-02-11 Thread Andrey Smetanin
The patch implements userspace exit 'KVM_EXIT_HYPERV'
for Hyper-V VMBus hypercalls(postmsg, signalevent)
to handle these hypercalls by QEMU.

Changes v3:
* use vcpu->arch.complete_userspace_io to setup hypercall
result
* rebase for 'next-20160211'

Changes v2:
* use KVM_EXIT_HYPERV for hypercalls

Signed-off-by: Andrey Smetanin <asmeta...@virtuozzo.com>
Reviewed-by: Roman Kagan <rka...@virtuozzo.com>
CC: Gleb Natapov <g...@kernel.org>
CC: Paolo Bonzini <pbonz...@redhat.com>
CC: Joerg Roedel <j...@8bytes.org>
CC: "K. Y. Srinivasan" <k...@microsoft.com>
CC: Haiyang Zhang <haiya...@microsoft.com>
CC: Roman Kagan <rka...@virtuozzo.com>
CC: Denis V. Lunev <d...@openvz.org>
CC: qemu-devel@nongnu.org

Andrey Smetanin (5):
  kvm/x86: Rename Hyper-V long spin wait hypercall
  drivers/hv: Move VMBus hypercall codes into Hyper-V UAPI header
  kvm/x86: Pass return code of kvm_emulate_hypercall
  kvm/x86: Reject Hyper-V hypercall continuation
  kvm/x86: Hyper-V VMBus hypercall userspace exit

 Documentation/virtual/kvm/api.txt  |  6 +
 arch/x86/include/uapi/asm/hyperv.h |  4 ++-
 arch/x86/kvm/hyperv.c  | 50 +++---
 arch/x86/kvm/svm.c |  3 +--
 arch/x86/kvm/vmx.c |  3 +--
 drivers/hv/connection.c|  2 +-
 drivers/hv/hv.c|  2 +-
 drivers/hv/hyperv_vmbus.h  |  6 -
 include/uapi/linux/kvm.h   |  6 +
 9 files changed, 60 insertions(+), 22 deletions(-)

-- 
2.4.3




Re: [Qemu-devel] [RFC v7 05/16] softmmu: Add new TLB_EXCL flag

2016-02-11 Thread Alex Bennée

Alvise Rigo  writes:

> Add a new TLB flag to force all the accesses made to a page to follow
> the slow-path.
>
> The TLB entries referring guest pages with the DIRTY_MEMORY_EXCLUSIVE
> bit clean will have this flag set.
>
> Suggested-by: Jani Kokkonen 
> Suggested-by: Claudio Fontana 
> Signed-off-by: Alvise Rigo 
> ---
>  include/exec/cpu-all.h | 8 
>  1 file changed, 8 insertions(+)
>
> diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
> index 83b1781..f8d8feb 100644
> --- a/include/exec/cpu-all.h
> +++ b/include/exec/cpu-all.h
> @@ -277,6 +277,14 @@ CPUArchState *cpu_copy(CPUArchState *env);
>  #define TLB_NOTDIRTY(1 << 4)
>  /* Set if TLB entry is an IO callback.  */
>  #define TLB_MMIO(1 << 5)
> +/* Set if TLB entry references a page that requires exclusive access.  */
> +#define TLB_EXCL(1 << 6)
> +
> +/* Do not allow a TARGET_PAGE_MASK which covers one or more bits defined
> + * above. */
> +#if TLB_EXCL >= TARGET_PAGE_SIZE
> +#error TARGET_PAGE_MASK covering the low bits of the TLB virtual address
> +#endif
>
>  void dump_exec_info(FILE *f, fprintf_function cpu_fprintf);
>  void dump_opcount_info(FILE *f, fprintf_function cpu_fprintf);

Reviewed-by: Alex Bennée 

--
Alex Bennée



[Qemu-devel] [PATCH v3 1/5] kvm/x86: Rename Hyper-V long spin wait hypercall

2016-02-11 Thread Andrey Smetanin
Rename HV_X64_HV_NOTIFY_LONG_SPIN_WAIT
by HV_X64_HCALL_NOTIFY_LONG_SPIN_WAIT. So
the name better reflects hypercall codes accessory.

Signed-off-by: Andrey Smetanin 
Reviewed-by: Roman Kagan 
CC: Gleb Natapov 
CC: Paolo Bonzini 
CC: Joerg Roedel 
CC: "K. Y. Srinivasan" 
CC: Haiyang Zhang 
CC: Roman Kagan 
CC: Denis V. Lunev 
CC: qemu-devel@nongnu.org
---
 arch/x86/include/uapi/asm/hyperv.h | 2 +-
 arch/x86/kvm/hyperv.c  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/uapi/asm/hyperv.h 
b/arch/x86/include/uapi/asm/hyperv.h
index 7956412..0c50fab 100644
--- a/arch/x86/include/uapi/asm/hyperv.h
+++ b/arch/x86/include/uapi/asm/hyperv.h
@@ -226,7 +226,7 @@
(~((1ull << HV_X64_MSR_HYPERCALL_PAGE_ADDRESS_SHIFT) - 1))
 
 /* Declare the various hypercall operations. */
-#define HV_X64_HV_NOTIFY_LONG_SPIN_WAIT0x0008
+#define HV_X64_HCALL_NOTIFY_LONG_SPIN_WAIT 0x0008
 
 #define HV_X64_MSR_APIC_ASSIST_PAGE_ENABLE 0x0001
 #define HV_X64_MSR_APIC_ASSIST_PAGE_ADDRESS_SHIFT  12
diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index c58ba67..f1a42e1 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -1084,7 +1084,7 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
trace_kvm_hv_hypercall(code, fast, rep_cnt, rep_idx, ingpa, outgpa);
 
switch (code) {
-   case HV_X64_HV_NOTIFY_LONG_SPIN_WAIT:
+   case HV_X64_HCALL_NOTIFY_LONG_SPIN_WAIT:
kvm_vcpu_on_spin(vcpu);
break;
default:
-- 
2.4.3




[Qemu-devel] [PATCH v3 5/5] kvm/x86: Hyper-V VMBus hypercall userspace exit

2016-02-11 Thread Andrey Smetanin
The patch implements KVM_EXIT_HYPERV userspace exit
functionality for Hyper-V VMBus hypercalls:
HV_X64_HCALL_POST_MESSAGE, HV_X64_HCALL_SIGNAL_EVENT.

Changes v3:
* use vcpu->arch.complete_userspace_io to setup hypercall
result

Changes v2:
* use KVM_EXIT_HYPERV for hypercalls

Signed-off-by: Andrey Smetanin 
Reviewed-by: Roman Kagan 
CC: Gleb Natapov 
CC: Paolo Bonzini 
CC: Joerg Roedel 
CC: "K. Y. Srinivasan" 
CC: Haiyang Zhang 
CC: Roman Kagan 
CC: Denis V. Lunev 
CC: qemu-devel@nongnu.org
---
 Documentation/virtual/kvm/api.txt |  6 ++
 arch/x86/kvm/hyperv.c | 39 ---
 include/uapi/linux/kvm.h  |  6 ++
 3 files changed, 44 insertions(+), 7 deletions(-)

diff --git a/Documentation/virtual/kvm/api.txt 
b/Documentation/virtual/kvm/api.txt
index 07e4cdf..4a661e5 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -3339,6 +3339,7 @@ EOI was received.
 
struct kvm_hyperv_exit {
 #define KVM_EXIT_HYPERV_SYNIC  1
+#define KVM_EXIT_HYPERV_HCALL  2
__u32 type;
union {
struct {
@@ -3347,6 +3348,11 @@ EOI was received.
__u64 evt_page;
__u64 msg_page;
} synic;
+   struct {
+   __u64 input;
+   __u64 result;
+   __u64 params[2];
+   } hcall;
} u;
};
/* KVM_EXIT_HYPERV */
diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index e1daa8b..f8d97ee 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -1043,6 +1043,27 @@ bool kvm_hv_hypercall_enabled(struct kvm *kvm)
return kvm->arch.hyperv.hv_hypercall & HV_X64_MSR_HYPERCALL_ENABLE;
 }
 
+static void kvm_hv_hypercall_set_result(struct kvm_vcpu *vcpu, u64 result)
+{
+   bool longmode;
+
+   longmode = is_64_bit_mode(vcpu);
+   if (longmode)
+   kvm_register_write(vcpu, VCPU_REGS_RAX, result);
+   else {
+   kvm_register_write(vcpu, VCPU_REGS_RDX, result >> 32);
+   kvm_register_write(vcpu, VCPU_REGS_RAX, result & 0x);
+   }
+}
+
+static int kvm_hv_hypercall_complete_userspace(struct kvm_vcpu *vcpu)
+{
+   struct kvm_run *run = vcpu->run;
+
+   kvm_hv_hypercall_set_result(vcpu, run->hyperv.u.hcall.result);
+   return 1;
+}
+
 int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
 {
u64 param, ingpa, outgpa, ret;
@@ -1093,6 +1114,16 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
case HV_X64_HCALL_NOTIFY_LONG_SPIN_WAIT:
kvm_vcpu_on_spin(vcpu);
break;
+   case HV_X64_HCALL_POST_MESSAGE:
+   case HV_X64_HCALL_SIGNAL_EVENT:
+   vcpu->run->exit_reason = KVM_EXIT_HYPERV;
+   vcpu->run->hyperv.type = KVM_EXIT_HYPERV_HCALL;
+   vcpu->run->hyperv.u.hcall.input = param;
+   vcpu->run->hyperv.u.hcall.params[0] = ingpa;
+   vcpu->run->hyperv.u.hcall.params[1] = outgpa;
+   vcpu->arch.complete_userspace_io =
+   kvm_hv_hypercall_complete_userspace;
+   return 0;
default:
res = HV_STATUS_INVALID_HYPERCALL_CODE;
break;
@@ -1100,12 +1131,6 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
 
 set_result:
ret = res | (((u64)rep_done & 0xfff) << 32);
-   if (longmode) {
-   kvm_register_write(vcpu, VCPU_REGS_RAX, ret);
-   } else {
-   kvm_register_write(vcpu, VCPU_REGS_RDX, ret >> 32);
-   kvm_register_write(vcpu, VCPU_REGS_RAX, ret & 0x);
-   }
-
+   kvm_hv_hypercall_set_result(vcpu, ret);
return 1;
 }
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index a2fe0ac..82581b6 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -157,6 +157,7 @@ struct kvm_s390_skeys {
 
 struct kvm_hyperv_exit {
 #define KVM_EXIT_HYPERV_SYNIC  1
+#define KVM_EXIT_HYPERV_HCALL  2
__u32 type;
union {
struct {
@@ -165,6 +166,11 @@ struct kvm_hyperv_exit {
__u64 evt_page;
__u64 msg_page;
} synic;
+   struct {
+   __u64 input;
+   __u64 result;
+   __u64 params[2];
+   } hcall;
} u;
 };
 
-- 
2.4.3




[Qemu-devel] [PATCH v3 3/5] kvm/x86: Pass return code of kvm_emulate_hypercall

2016-02-11 Thread Andrey Smetanin
Pass the return code from kvm_emulate_hypercall on to the caller,
in order to allow it to indicate to the userspace that
the hypercall has to be handled there.

Also adjust all the existing code paths to return 1 to make sure the
hypercall isn't passed to the userspace without setting kvm_run
appropriately.

Signed-off-by: Andrey Smetanin 
Reviewed-by: Roman Kagan 
CC: Gleb Natapov 
CC: Paolo Bonzini 
CC: Joerg Roedel 
CC: "K. Y. Srinivasan" 
CC: Haiyang Zhang 
CC: Roman Kagan 
CC: Denis V. Lunev 
CC: qemu-devel@nongnu.org
---
 arch/x86/kvm/hyperv.c | 2 +-
 arch/x86/kvm/svm.c| 3 +--
 arch/x86/kvm/vmx.c| 3 +--
 3 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index f1a42e1..0e7c90f 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -1055,7 +1055,7 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
 */
if (kvm_x86_ops->get_cpl(vcpu) != 0 || !is_protmode(vcpu)) {
kvm_queue_exception(vcpu, UD_VECTOR);
-   return 0;
+   return 1;
}
 
longmode = is_64_bit_mode(vcpu);
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index c13a64b..9507038 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -1858,8 +1858,7 @@ static int halt_interception(struct vcpu_svm *svm)
 static int vmmcall_interception(struct vcpu_svm *svm)
 {
svm->next_rip = kvm_rip_read(>vcpu) + 3;
-   kvm_emulate_hypercall(>vcpu);
-   return 1;
+   return kvm_emulate_hypercall(>vcpu);
 }
 
 static unsigned long nested_svm_get_tdp_cr3(struct kvm_vcpu *vcpu)
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 164eb9e..2edca5d 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -5747,8 +5747,7 @@ static int handle_halt(struct kvm_vcpu *vcpu)
 
 static int handle_vmcall(struct kvm_vcpu *vcpu)
 {
-   kvm_emulate_hypercall(vcpu);
-   return 1;
+   return kvm_emulate_hypercall(vcpu);
 }
 
 static int handle_invd(struct kvm_vcpu *vcpu)
-- 
2.4.3




Re: [Qemu-devel] [PATCH v2 0/5] q35: Remove old machines and unused compat code

2016-02-11 Thread Eduardo Habkost
On Sat, Feb 06, 2016 at 08:34:07PM +0200, Michael S. Tsirkin wrote:
> On Fri, Feb 05, 2016 at 12:46:11PM -0200, Eduardo Habkost wrote:
> > On Fri, Feb 05, 2016 at 12:14:16AM +0200, Michael S. Tsirkin wrote:
> > > On Thu, Feb 04, 2016 at 05:09:44PM -0200, Eduardo Habkost wrote:
> > > > On Thu, Feb 04, 2016 at 08:02:30PM +0200, Michael S. Tsirkin wrote:
> > > > > On Thu, Feb 04, 2016 at 03:16:17PM -0200, Eduardo Habkost wrote:
> > > > > > On Thu, Feb 04, 2016 at 06:01:50PM +0200, Michael S. Tsirkin wrote:
> > > > > > > On Sat, Jan 23, 2016 at 02:02:08PM -0200, Eduardo Habkost wrote:
> > > > > > > > This is another attempt to remove old q35 machine code. Now I am
> > > > > > > > also removing unused compat code to demonstrate the benefit of
> > > > > > > > throwing away the old code that nobody uses.
> > > > > > > 
> > > > > > > The same thing I said applies - we don't know that nobody uses 
> > > > > > > old q35
> > > > > > > machine types.
> > > > > > > We do know we don't need to migrate to/from them,
> > > > > > > so we can drop compat code.
> > > > > > > But please add aliases so people can still start these machines.
> > > > > > 
> > > > > > If people use them, they can easily update their configurations.
> > > > > > I will copy and paste the reply Markus sent 4 months ago below.
> > > > > > 
> > > > > > On Mon, Sep 14, 2015 at 09:18:47AM +0200, Markus Armbruster wrote:
> > > > > > > We've been through this before, but we can go through it once 
> > > > > > > more.
> > > > > > > Choices:
> > > > > > > 
> > > > > > > A. Remove old machine type
> > > > > > > 
> > > > > > >A guest using it can't be started.  Easy to understand on the 
> > > > > > > host.
> > > > > > >An error message advising to switch to a newer machine type 
> > > > > > > would be
> > > > > > >a nice touch.
> > > > > > > 
> > > > > > >This is a clean break in backward compatibility.  To be 
> > > > > > > mentioned in
> > > > > > >release notes, obviously.
> > > > > > > 
> > > > > > > B. Change old machine type in a guest-visible way
> > > > > > > 
> > > > > > >Depending on the nature of the change and the guest, a guest 
> > > > > > > using it
> > > > > > >either doesn't notice, copes with it successfully, or fails in
> > > > > > >guest-specific ways.  If the latter, the failure can be "guest
> > > > > > >hangs", which is much harder to figure out than A.
> > > > > > > 
> > > > > > >Unless we can *demonstrate* that nothing bad happens for all 
> > > > > > > the
> > > > > > >guests people actually use with the old machine types, this is 
> > > > > > > a
> > > > > > >different kind of backward compatibility break.
> > > > > > > 
> > > > > > >Demonstrating this is feels infeasible to me, but you're 
> > > > > > > welcome to
> > > > > > >try.
> > > > > > > 
> > > > > > > I could call the difference between the two a tradeoff, but since 
> > > > > > > we've
> > > > > > > been through this before, I'll be more blunt: choosing B robs 
> > > > > > > Peter (the
> > > > > > > guy with guests where badness happens) to pay Paul (the guy with 
> > > > > > > guests
> > > > > > > that cope).  Paul is saved the inconvenience of having to read 
> > > > > > > release
> > > > > > > notes or his logs, and change machine types.  Peter pays for that 
> > > > > > > with
> > > > > > > figuring out WTF his guests are doing now.
> > > > > > > 
> > > > > > > As a user, I'd pick a clean break in backward compatibility over 
> > > > > > > a hack
> > > > > > > that preserves effective compatibility when it works, but breaks 
> > > > > > > it
> > > > > > > uncleanly when it doesn't.
> > > > > > > 
> > > > > > > As a developer, I'm insisting on it.
> > > > > > > 
> > > > > > > So, if you want B, the onus is on *you* to show us why nothing 
> > > > > > > bad will
> > > > > > > happen.
> > > > > > > 
> > > > > 
> > > > > I agree with the conclusion for option B.  But I think the correct
> > > > > solution is not A, it is to analyse changes, maybe even test, and show
> > > > > that nothing bad can happen.
> > > > 
> > > > Do you volunteer for that work?
> > > 
> > > Nope, sorry. It's your idea, your patchset.
> > 
> > It's your idea. You are the one proposing to waste resources
> > keeping an old machine-type name "working" just because you don't
> > want users (who we don't even know if they actually exist) to
> > update their configurations on a QEMU upgrade.
> > 
> > I am proposing the opposite: dropping support to a feature that
> > people are unlikely to be using, in a very clear way.
> 
> What will happen with installed VMs? Will they break?

They won't start unless the QEMU command-line is changed, because
they are using a feature QEMU won't support anymore. Why is that
a problem?

Why do you want to waste so much time keeping a feature that
people are not even supposed to be using?

> 
> > 
> > > I am saying, look
> > > for some low-hanging fruit.  Find some compat options we can
> > > drop without breaking 

[Qemu-devel] [PATCH 5/5] target-tricore: add opd trap generation

2016-02-11 Thread Bastian Koppelmann
If an instruction uses a 64 bit register which consists of an even-odd pair
of 32 bit registers and if the register specifier in the instruction is
odd an opd trap is raised.

Signed-off-by: Bastian Koppelmann 
---
 target-tricore/translate.c | 285 +++--
 1 file changed, 277 insertions(+), 8 deletions(-)

diff --git a/target-tricore/translate.c b/target-tricore/translate.c
index 83e0927..08fad05 100644
--- a/target-tricore/translate.c
+++ b/target-tricore/translate.c
@@ -216,6 +216,15 @@ void tricore_cpu_dump_state(CPUState *cs, FILE *f,
 #define EA_B_ABSOLUT(con) (((offset & 0xf0) << 8) | \
((offset & 0x0f) << 1))
 
+/* For two 32-bit registers used a 64-bit register, the first
+   registernumber needs to be even. Otherwise we trap. */
+static inline void generate_trap(DisasContext *ctx, int class, int tin);
+#define CHECK_REG_PAIR(reg) do {  \
+if (reg & 0x1) {  \
+generate_trap(ctx, TRAPC_INSN_ERR, TIN2_OPD); \
+} \
+} while (0)
+
 /* Functions for load/save to/from memory */
 
 static inline void gen_offset_ld(DisasContext *ctx, TCGv r1, TCGv r2,
@@ -301,6 +310,7 @@ static void gen_ldmst(DisasContext *ctx, int ereg, TCGv ea)
 TCGv temp = tcg_temp_new();
 TCGv temp2 = tcg_temp_new();
 
+CHECK_REG_PAIR(ereg);
 /* temp = (M(EA, word) */
 tcg_gen_qemu_ld_tl(temp, ea, ctx->mem_idx, MO_LEUL);
 /* temp = temp & ~E[a][63:32]) */
@@ -4213,9 +4223,11 @@ static void decode_abs_ldw(CPUTriCoreState *env, 
DisasContext *ctx)
 tcg_gen_qemu_ld_tl(cpu_gpr_a[r1], temp, ctx->mem_idx, MO_LESL);
 break;
 case OPC2_32_ABS_LD_D:
+CHECK_REG_PAIR(r1);
 gen_ld_2regs_64(cpu_gpr_d[r1+1], cpu_gpr_d[r1], temp, ctx);
 break;
 case OPC2_32_ABS_LD_DA:
+CHECK_REG_PAIR(r1);
 gen_ld_2regs_64(cpu_gpr_a[r1+1], cpu_gpr_a[r1], temp, ctx);
 break;
 case OPC2_32_ABS_LD_W:
@@ -4332,9 +4344,11 @@ static void decode_abs_store(CPUTriCoreState *env, 
DisasContext *ctx)
 tcg_gen_qemu_st_tl(cpu_gpr_a[r1], temp, ctx->mem_idx, MO_LESL);
 break;
 case OPC2_32_ABS_ST_D:
+CHECK_REG_PAIR(r1);
 gen_st_2regs_64(cpu_gpr_d[r1+1], cpu_gpr_d[r1], temp, ctx);
 break;
 case OPC2_32_ABS_ST_DA:
+CHECK_REG_PAIR(r1);
 gen_st_2regs_64(cpu_gpr_a[r1+1], cpu_gpr_a[r1], temp, ctx);
 break;
 case OPC2_32_ABS_ST_W:
@@ -4712,14 +4726,17 @@ static void 
decode_bo_addrmode_post_pre_base(CPUTriCoreState *env,
 gen_st_preincr(ctx, cpu_gpr_d[r1], cpu_gpr_a[r2], off10, MO_UB);
 break;
 case OPC2_32_BO_ST_D_SHORTOFF:
+CHECK_REG_PAIR(r1);
 gen_offset_st_2regs(cpu_gpr_d[r1+1], cpu_gpr_d[r1], cpu_gpr_a[r2],
 off10, ctx);
 break;
 case OPC2_32_BO_ST_D_POSTINC:
+CHECK_REG_PAIR(r1);
 gen_st_2regs_64(cpu_gpr_d[r1+1], cpu_gpr_d[r1], cpu_gpr_a[r2], ctx);
 tcg_gen_addi_tl(cpu_gpr_a[r2], cpu_gpr_a[r2], off10);
 break;
 case OPC2_32_BO_ST_D_PREINC:
+CHECK_REG_PAIR(r1);
 temp = tcg_temp_new();
 tcg_gen_addi_tl(temp, cpu_gpr_a[r2], off10);
 gen_st_2regs_64(cpu_gpr_d[r1+1], cpu_gpr_d[r1], temp, ctx);
@@ -4727,14 +4744,17 @@ static void 
decode_bo_addrmode_post_pre_base(CPUTriCoreState *env,
 tcg_temp_free(temp);
 break;
 case OPC2_32_BO_ST_DA_SHORTOFF:
+CHECK_REG_PAIR(r1);
 gen_offset_st_2regs(cpu_gpr_a[r1+1], cpu_gpr_a[r1], cpu_gpr_a[r2],
 off10, ctx);
 break;
 case OPC2_32_BO_ST_DA_POSTINC:
+CHECK_REG_PAIR(r1);
 gen_st_2regs_64(cpu_gpr_a[r1+1], cpu_gpr_a[r1], cpu_gpr_a[r2], ctx);
 tcg_gen_addi_tl(cpu_gpr_a[r2], cpu_gpr_a[r2], off10);
 break;
 case OPC2_32_BO_ST_DA_PREINC:
+CHECK_REG_PAIR(r1);
 temp = tcg_temp_new();
 tcg_gen_addi_tl(temp, cpu_gpr_a[r2], off10);
 gen_st_2regs_64(cpu_gpr_a[r1+1], cpu_gpr_a[r1], temp, ctx);
@@ -4804,7 +4824,7 @@ static void 
decode_bo_addrmode_bitreverse_circular(CPUTriCoreState *env,
 temp = tcg_temp_new();
 temp2 = tcg_temp_new();
 temp3 = tcg_const_i32(off10);
-
+CHECK_REG_PAIR(r2);
 tcg_gen_ext16u_tl(temp, cpu_gpr_a[r2+1]);
 tcg_gen_add_tl(temp2, cpu_gpr_a[r2], temp);
 
@@ -4836,10 +4856,12 @@ static void 
decode_bo_addrmode_bitreverse_circular(CPUTriCoreState *env,
 gen_helper_circ_update(cpu_gpr_a[r2+1], cpu_gpr_a[r2+1], temp3);
 break;
 case OPC2_32_BO_ST_D_BR:
+CHECK_REG_PAIR(r1);
 gen_st_2regs_64(cpu_gpr_d[r1+1], cpu_gpr_d[r1], temp2, ctx);
 gen_helper_br_update(cpu_gpr_a[r2+1], cpu_gpr_a[r2+1]);
 break;
 case OPC2_32_BO_ST_D_CIRC:
+CHECK_REG_PAIR(r1);
 tcg_gen_qemu_st_tl(cpu_gpr_d[r1], 

[Qemu-devel] [PATCH 2/5] target-tricore: Save the pc before CSA operations for exceptions

2016-02-11 Thread Bastian Koppelmann
Exceptions that can occur during CSA operations need the PC as
the return address of the exception.

Signed-off-by: Bastian Koppelmann 
---
 target-tricore/translate.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/target-tricore/translate.c b/target-tricore/translate.c
index 721878d..775d4c6 100644
--- a/target-tricore/translate.c
+++ b/target-tricore/translate.c
@@ -3336,6 +3336,8 @@ static void gen_compute_branch(DisasContext *ctx, 
uint32_t opc, int r1,
 break;
 case OPC1_32_B_CALL:
 case OPC1_16_SB_CALL:
+/* save pc for the exception return address */
+gen_save_pc(ctx->pc);
 gen_helper_1arg(call, ctx->next_pc);
 gen_goto_tb(ctx, 0, ctx->pc + offset * 2);
 break;
@@ -3408,6 +3410,8 @@ static void gen_compute_branch(DisasContext *ctx, 
uint32_t opc, int r1,
 break;
 case OPC2_32_SYS_RET:
 case OPC2_16_SR_RET:
+/* save pc for the exception return address */
+gen_save_pc(ctx->pc);
 gen_helper_ret(cpu_env);
 tcg_gen_exit_tb(0);
 break;
@@ -3782,6 +3786,8 @@ static void decode_sc_opc(DisasContext *ctx, int op1)
 tcg_gen_andi_tl(cpu_gpr_d[15], cpu_gpr_d[15], const16);
 break;
 case OPC1_16_SC_BISR:
+/* save pc for the exception return address */
+gen_save_pc(ctx->pc);
 gen_helper_1arg(bisr, const16 & 0xff);
 break;
 case OPC1_16_SC_LD_A:
@@ -3897,6 +3903,8 @@ static void decode_sr_system(CPUTriCoreState *env, 
DisasContext *ctx)
 gen_compute_branch(ctx, op2, 0, 0, 0, 0);
 break;
 case OPC2_16_SR_RFE:
+/* save pc for the exception return address */
+gen_save_pc(ctx->pc);
 gen_helper_rfe(cpu_env);
 tcg_gen_exit_tb(0);
 ctx->bstate = BS_BRANCH;
@@ -7895,6 +7903,8 @@ static void decode_sys_interrupts(CPUTriCoreState *env, 
DisasContext *ctx)
 gen_fret(ctx);
 break;
 case OPC2_32_SYS_RFE:
+/* save pc for the exception return address */
+gen_save_pc(ctx->pc);
 gen_helper_rfe(cpu_env);
 tcg_gen_exit_tb(0);
 ctx->bstate = BS_BRANCH;
@@ -7917,9 +7927,13 @@ static void decode_sys_interrupts(CPUTriCoreState *env, 
DisasContext *ctx)
 }
 break;
 case OPC2_32_SYS_RSLCX:
+/* save pc for the exception return address */
+gen_save_pc(ctx->pc);
 gen_helper_rslcx(cpu_env);
 break;
 case OPC2_32_SYS_SVLCX:
+/* save pc for the exception return address */
+gen_save_pc(ctx->pc);
 gen_helper_svlcx(cpu_env);
 break;
 case OPC2_32_SYS_RESTORE:
-- 
2.7.1




[Qemu-devel] [PATCH 4/5] target-tricore: add illegal opcode trap generation

2016-02-11 Thread Bastian Koppelmann
Signed-off-by: Bastian Koppelmann 
---
 target-tricore/translate.c | 175 -
 1 file changed, 156 insertions(+), 19 deletions(-)

diff --git a/target-tricore/translate.c b/target-tricore/translate.c
index 775d4c6..83e0927 100644
--- a/target-tricore/translate.c
+++ b/target-tricore/translate.c
@@ -3567,7 +3567,7 @@ static void gen_compute_branch(DisasContext *ctx, 
uint32_t opc, int r1,
 }
 break;
 default:
-printf("Branch Error at %x\n", ctx->pc);
+generate_trap(ctx, TRAPC_INSN_ERR, TIN2_IOPC);
 }
 ctx->bstate = BS_BRANCH;
 }
@@ -3642,7 +3642,9 @@ static void decode_src_opc(CPUTriCoreState *env, 
DisasContext *ctx, int op1)
 if (tricore_feature(env, TRICORE_FEATURE_16)) {
 tcg_gen_movi_tl(cpu_gpr_d[r1], const4);
 tcg_gen_sari_tl(cpu_gpr_d[r1+1], cpu_gpr_d[r1], 31);
-} /* TODO: else raise illegal opcode trap */
+} else {
+generate_trap(ctx, TRAPC_INSN_ERR, TIN2_IOPC);
+}
 break;
 case OPC1_16_SRC_SH:
 gen_shi(cpu_gpr_d[r1], cpu_gpr_d[r1], const4);
@@ -3650,6 +3652,8 @@ static void decode_src_opc(CPUTriCoreState *env, 
DisasContext *ctx, int op1)
 case OPC1_16_SRC_SHA:
 gen_shaci(cpu_gpr_d[r1], cpu_gpr_d[r1], const4);
 break;
+default:
+generate_trap(ctx, TRAPC_INSN_ERR, TIN2_IOPC);
 }
 }
 
@@ -3733,6 +3737,8 @@ static void decode_srr_opc(DisasContext *ctx, int op1)
 case OPC1_16_SRR_XOR:
 tcg_gen_xor_tl(cpu_gpr_d[r1], cpu_gpr_d[r1], cpu_gpr_d[r2]);
 break;
+default:
+generate_trap(ctx, TRAPC_INSN_ERR, TIN2_IOPC);
 }
 }
 
@@ -3772,6 +3778,8 @@ static void decode_ssr_opc(DisasContext *ctx, int op1)
 tcg_gen_qemu_st_tl(cpu_gpr_d[r1], cpu_gpr_a[r2], ctx->mem_idx, 
MO_LEUL);
 tcg_gen_addi_tl(cpu_gpr_a[r2], cpu_gpr_a[r2], 4);
 break;
+default:
+generate_trap(ctx, TRAPC_INSN_ERR, TIN2_IOPC);
 }
 }
 
@@ -3811,6 +3819,8 @@ static void decode_sc_opc(DisasContext *ctx, int op1)
 case OPC1_16_SC_SUB_A:
 tcg_gen_subi_tl(cpu_gpr_a[10], cpu_gpr_a[10], const16);
 break;
+default:
+generate_trap(ctx, TRAPC_INSN_ERR, TIN2_IOPC);
 }
 }
 
@@ -3851,6 +3861,8 @@ static void decode_slr_opc(DisasContext *ctx, int op1)
 tcg_gen_qemu_ld_tl(cpu_gpr_d[r1], cpu_gpr_a[r2], ctx->mem_idx, 
MO_LESL);
 tcg_gen_addi_tl(cpu_gpr_a[r2], cpu_gpr_a[r2], 4);
 break;
+default:
+generate_trap(ctx, TRAPC_INSN_ERR, TIN2_IOPC);
 }
 }
 
@@ -3888,6 +3900,8 @@ static void decode_sro_opc(DisasContext *ctx, int op1)
 case OPC1_16_SRO_ST_W:
 gen_offset_st(ctx, cpu_gpr_d[15], cpu_gpr_a[r2], address * 4, MO_LESL);
 break;
+default:
+generate_trap(ctx, TRAPC_INSN_ERR, TIN2_IOPC);
 }
 }
 
@@ -3914,6 +3928,9 @@ static void decode_sr_system(CPUTriCoreState *env, 
DisasContext *ctx)
 break;
 case OPC2_16_SR_FRET:
 gen_fret(ctx);
+break;
+default:
+generate_trap(ctx, TRAPC_INSN_ERR, TIN2_IOPC);
 }
 }
 
@@ -3956,6 +3973,8 @@ static void decode_sr_accu(CPUTriCoreState *env, 
DisasContext *ctx)
 case OPC2_16_SR_SAT_HU:
 gen_saturate_u(cpu_gpr_d[r1], cpu_gpr_d[r1], 0x);
 break;
+default:
+generate_trap(ctx, TRAPC_INSN_ERR, TIN2_IOPC);
 }
 }
 
@@ -4166,6 +4185,8 @@ static void decode_16Bit_opc(CPUTriCoreState *env, 
DisasContext *ctx)
 r1 = MASK_OP_SR_S1D(ctx->opcode);
 tcg_gen_not_tl(cpu_gpr_d[r1], cpu_gpr_d[r1]);
 break;
+default:
+generate_trap(ctx, TRAPC_INSN_ERR, TIN2_IOPC);
 }
 }
 
@@ -4200,6 +4221,8 @@ static void decode_abs_ldw(CPUTriCoreState *env, 
DisasContext *ctx)
 case OPC2_32_ABS_LD_W:
 tcg_gen_qemu_ld_tl(cpu_gpr_d[r1], temp, ctx->mem_idx, MO_LESL);
 break;
+default:
+generate_trap(ctx, TRAPC_INSN_ERR, TIN2_IOPC);
 }
 
 tcg_temp_free(temp);
@@ -4231,6 +4254,8 @@ static void decode_abs_ldb(CPUTriCoreState *env, 
DisasContext *ctx)
 case OPC2_32_ABS_LD_HU:
 tcg_gen_qemu_ld_tl(cpu_gpr_d[r1], temp, ctx->mem_idx, MO_LEUW);
 break;
+default:
+generate_trap(ctx, TRAPC_INSN_ERR, TIN2_IOPC);
 }
 
 tcg_temp_free(temp);
@@ -4256,6 +4281,8 @@ static void decode_abs_ldst_swap(CPUTriCoreState *env, 
DisasContext *ctx)
 case OPC2_32_ABS_SWAP_W:
 gen_swap(ctx, r1, temp);
 break;
+default:
+generate_trap(ctx, TRAPC_INSN_ERR, TIN2_IOPC);
 }
 
 tcg_temp_free(temp);
@@ -4282,6 +4309,8 @@ static void decode_abs_ldst_context(CPUTriCoreState *env, 
DisasContext *ctx)
 case OPC2_32_ABS_STUCX:
 gen_helper_1arg(stucx, EA_ABS_FORMAT(off18));
 break;
+default:
+generate_trap(ctx, TRAPC_INSN_ERR, TIN2_IOPC);
 }
 }
 
@@ -4311,7 +4340,8 @@ static void decode_abs_store(CPUTriCoreState 

[Qemu-devel] [PATCH v2 5/6] target-arm: Implement MDCR_EL3.TDA and MDCR_EL2.TDA traps

2016-02-11 Thread Peter Maydell
Implement the debug register traps controlled by MDCR_EL2.TDA
and MDCR_EL3.TDA.

Signed-off-by: Peter Maydell 
Reviewed-by: Sergey Fedorov 
---
 target-arm/helper.c | 39 ++-
 1 file changed, 30 insertions(+), 9 deletions(-)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index 11eb77a..e2b7238 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -421,6 +421,24 @@ static CPAccessResult access_tdra(CPUARMState *env, const 
ARMCPRegInfo *ri,
 return CP_ACCESS_OK;
 }
 
+/* Check for traps to general debug registers, which are controlled
+ * by MDCR_EL2.TDA for EL2 and MDCR_EL3.TDA for EL3.
+ */
+static CPAccessResult access_tda(CPUARMState *env, const ARMCPRegInfo *ri,
+  bool isread)
+{
+int el = arm_current_el(env);
+
+if (el < 2 && (env->cp15.mdcr_el2 & MDCR_TDA)
+&& !arm_is_secure_below_el3(env)) {
+return CP_ACCESS_TRAP_EL2;
+}
+if (el < 3 && (env->cp15.mdcr_el3 & MDCR_TDA)) {
+return CP_ACCESS_TRAP_EL3;
+}
+return CP_ACCESS_OK;
+}
+
 static void dacr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t 
value)
 {
 ARMCPU *cpu = arm_env_get_cpu(env);
@@ -3384,7 +3402,8 @@ static const ARMCPRegInfo el3_no_el2_cp_reginfo[] = {
   .access = PL2_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
 { .name = "MDCR_EL2", .state = ARM_CP_STATE_BOTH,
   .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 1,
-  .access = PL2_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
+  .access = PL2_RW, .accessfn = access_tda,
+  .type = ARM_CP_CONST, .resetvalue = 0 },
 { .name = "HPFAR_EL2", .state = ARM_CP_STATE_BOTH,
   .opc0 = 3, .opc1 = 4, .crn = 6, .crm = 0, .opc2 = 4,
   .access = PL2_RW, .accessfn = access_el3_aa32ns_aa64any,
@@ -3803,7 +3822,7 @@ static const ARMCPRegInfo debug_cp_reginfo[] = {
 /* Monitor debug system control register; the 32-bit alias is DBGDSCRext. 
*/
 { .name = "MDSCR_EL1", .state = ARM_CP_STATE_BOTH,
   .cp = 14, .opc0 = 2, .opc1 = 0, .crn = 0, .crm = 2, .opc2 = 2,
-  .access = PL1_RW,
+  .access = PL1_RW, .accessfn = access_tda,
   .fieldoffset = offsetof(CPUARMState, cp15.mdscr_el1),
   .resetvalue = 0 },
 /* MDCCSR_EL0, aka DBGDSCRint. This is a read-only mirror of MDSCR_EL1.
@@ -3812,7 +3831,7 @@ static const ARMCPRegInfo debug_cp_reginfo[] = {
 { .name = "MDCCSR_EL0", .state = ARM_CP_STATE_BOTH,
   .cp = 14, .opc0 = 2, .opc1 = 0, .crn = 0, .crm = 1, .opc2 = 0,
   .type = ARM_CP_ALIAS,
-  .access = PL1_R,
+  .access = PL1_R, .accessfn = access_tda,
   .fieldoffset = offsetof(CPUARMState, cp15.mdscr_el1), },
 { .name = "OSLAR_EL1", .state = ARM_CP_STATE_BOTH,
   .cp = 14, .opc0 = 2, .opc1 = 0, .crn = 1, .crm = 0, .opc2 = 4,
@@ -3834,7 +3853,8 @@ static const ARMCPRegInfo debug_cp_reginfo[] = {
  */
 { .name = "DBGVCR",
   .cp = 14, .opc1 = 0, .crn = 0, .crm = 7, .opc2 = 0,
-  .access = PL1_RW, .type = ARM_CP_NOP },
+  .access = PL1_RW, .accessfn = access_tda,
+  .type = ARM_CP_NOP },
 REGINFO_SENTINEL
 };
 
@@ -4099,7 +4119,8 @@ static void define_debug_regs(ARMCPU *cpu)
 int wrps, brps, ctx_cmps;
 ARMCPRegInfo dbgdidr = {
 .name = "DBGDIDR", .cp = 14, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 0,
-.access = PL0_R, .type = ARM_CP_CONST, .resetvalue = cpu->dbgdidr,
+.access = PL0_R, .accessfn = access_tda,
+.type = ARM_CP_CONST, .resetvalue = cpu->dbgdidr,
 };
 
 /* Note that all these register fields hold "number of Xs minus 1". */
@@ -4130,13 +4151,13 @@ static void define_debug_regs(ARMCPU *cpu)
 ARMCPRegInfo dbgregs[] = {
 { .name = "DBGBVR", .state = ARM_CP_STATE_BOTH,
   .cp = 14, .opc0 = 2, .opc1 = 0, .crn = 0, .crm = i, .opc2 = 4,
-  .access = PL1_RW,
+  .access = PL1_RW, .accessfn = access_tda,
   .fieldoffset = offsetof(CPUARMState, cp15.dbgbvr[i]),
   .writefn = dbgbvr_write, .raw_writefn = raw_write
 },
 { .name = "DBGBCR", .state = ARM_CP_STATE_BOTH,
   .cp = 14, .opc0 = 2, .opc1 = 0, .crn = 0, .crm = i, .opc2 = 5,
-  .access = PL1_RW,
+  .access = PL1_RW, .accessfn = access_tda,
   .fieldoffset = offsetof(CPUARMState, cp15.dbgbcr[i]),
   .writefn = dbgbcr_write, .raw_writefn = raw_write
 },
@@ -4149,13 +4170,13 @@ static void define_debug_regs(ARMCPU *cpu)
 ARMCPRegInfo dbgregs[] = {
 { .name = "DBGWVR", .state = ARM_CP_STATE_BOTH,
   .cp = 14, .opc0 = 2, .opc1 = 0, .crn = 0, .crm = i, .opc2 = 6,
-  .access = PL1_RW,
+  .access = PL1_RW, .accessfn = access_tda,
   .fieldoffset = offsetof(CPUARMState, cp15.dbgwvr[i]),
   .writefn = dbgwvr_write, .raw_writefn = raw_write
  

Re: [Qemu-devel] [PATCH 2/3] qdev-monitor: add missing aliases for virtio-{9p, balloon, rng, scsi}

2016-02-11 Thread Sascha Silbe
Dear Conny,

Cornelia Huck  writes:

> On Thu, 11 Feb 2016 10:01:35 +0100
> Markus Armbruster  wrote:
>
>> Sascha Silbe  writes:
>
>> > This leaves out
>> > virtio-{gpu,input,input-hid,input-host,keyboard,mouse,tablet} because
>> > they're currently only implemented using PCI, so there's no immediate
>> > value in having them. It would nevertheless make sense to include them
>> > so they can get used already and will start to Just Work™ on s390x
>> > once a CCW implementation appears. I can post the corresponding patch
>> > if there's any interest.
>> 
>> I guess that's for the virtio people to decide.  I'm cc'ing some.
>
> What would the error look like if one decided to use e.g. virtio-gpu on
> s390x? If the error is more specific (i.e. virtio-gpu-ccw does not
> exist vs. virtio-gpu does not exist), I think adding the aliases makes
> sense: the user sees what is actually missing.

Interesting point. Indeed, if we already define the matching -ccw
aliases, the error message may be slightly more useful:

silbe@oc4731375738:~/recoverable/qemu$ s390x-softmmu/qemu-system-s390x -device 
virtio-gpu
qemu-system-s390x: -device virtio-gpu: 'virtio-gpu-ccw' is not a valid device 
model name

Though we should probably at least add a comment to the alias list
mentioning that this is intentional. We might even want to adjust
qdev_get_device_class() to print a more specific error message in this
case.

Sascha
-- 
Softwareentwicklung Sascha Silbe, Niederhofenstraße 5/1, 71229 Leonberg
https://se-silbe.de/
USt-IdNr. DE281696641




[Qemu-devel] [PATCH v2 1/6] target-arm: correct CNTFRQ access rights

2016-02-11 Thread Peter Maydell
Correct some corner cases we were getting wrong for
CNTFRQ access rights:
 * should UNDEF from 32-bit Secure EL1
 * only writable from the highest implemented exception level,
   which might not be EL1 now

To clarify the code, provide a new utility function
arm_highest_el() which returns the highest implemented
exception level.

Signed-off-by: Peter Maydell 
---
Rewritten to use arm_highest_el() to improve clarity
---
 target-arm/cpu.h| 12 
 target-arm/helper.c | 29 ++---
 2 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 5137632..afbf366 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -1255,6 +1255,18 @@ static inline bool cptype_valid(int cptype)
 #define PL1_RW (PL1_R | PL1_W)
 #define PL0_RW (PL0_R | PL0_W)
 
+/* Return the highest implemented Exception Level */
+static inline int arm_highest_el(CPUARMState *env)
+{
+if (arm_feature(env, ARM_FEATURE_EL3)) {
+return 3;
+}
+if (arm_feature(env, ARM_FEATURE_EL2)) {
+return 2;
+}
+return 1;
+}
+
 /* Return the current Exception Level (as per ARMv8; note that this differs
  * from the ARMv7 Privilege Level).
  */
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 2f9db72..4d27c00 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -1218,10 +1218,33 @@ static const ARMCPRegInfo v6k_cp_reginfo[] = {
 static CPAccessResult gt_cntfrq_access(CPUARMState *env, const ARMCPRegInfo 
*ri,
bool isread)
 {
-/* CNTFRQ: not visible from PL0 if both PL0PCTEN and PL0VCTEN are zero */
-if (arm_current_el(env) == 0 && !extract32(env->cp15.c14_cntkctl, 0, 2)) {
-return CP_ACCESS_TRAP;
+/* CNTFRQ: not visible from PL0 if both PL0PCTEN and PL0VCTEN are zero.
+ * Writable only at the highest implemented exception level.
+ */
+int el = arm_current_el(env);
+
+switch (el) {
+case 0:
+if (!extract32(env->cp15.c14_cntkctl, 0, 2)) {
+return CP_ACCESS_TRAP;
+}
+break;
+case 1:
+if (!isread && ri->state == ARM_CP_STATE_AA32 &&
+arm_is_secure_below_el3(env)) {
+/* Accesses from 32-bit Secure EL1 UNDEF (*not* trap to EL3!) */
+return CP_ACCESS_TRAP_UNCATEGORIZED;
+}
+break;
+case 2:
+case 3:
+break;
 }
+
+if (!isread && el < arm_highest_el(env)) {
+return CP_ACCESS_TRAP_UNCATEGORIZED;
+}
+
 return CP_ACCESS_OK;
 }
 
-- 
1.9.1




[Qemu-devel] broken HMP command: info mtree

2016-02-11 Thread Igor Mammedov
executing 'info mtree' from monitor prompt causes infinite loop
printing it over and over.

to reproduce build current master adn run:

qemu-system-x86_64 -monitor stdio

and then execute 'info mtree' in monitor prompt



[Qemu-devel] [PATCH 3/5] target-tricore: add context managment trap generation

2016-02-11 Thread Bastian Koppelmann
Signed-off-by: Bastian Koppelmann 
---
 target-tricore/op_helper.c | 27 ---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/target-tricore/op_helper.c b/target-tricore/op_helper.c
index fbe2be0..d7aafd1 100644
--- a/target-tricore/op_helper.c
+++ b/target-tricore/op_helper.c
@@ -2404,11 +2404,13 @@ void helper_call(CPUTriCoreState *env, uint32_t next_pc)
 /* if (FCX == 0) trap(FCU); */
 if (env->FCX == 0) {
 /* FCU trap */
+generate_trap(env, TRAPC_CTX_MNG, TIN3_FCU);
 }
 /* if (PSW.CDE) then if (cdc_increment()) then trap(CDO); */
 if (psw & MASK_PSW_CDE) {
 if (cdc_increment()) {
 /* CDO trap */
+generate_trap(env, TRAPC_CTX_MNG, TIN3_CDO);
 }
 }
 /* PSW.CDE = 1;*/
@@ -2444,6 +2446,7 @@ void helper_call(CPUTriCoreState *env, uint32_t next_pc)
 /* if (tmp_FCX == LCX) trap(FCD);*/
 if (tmp_FCX == env->LCX) {
 /* FCD trap */
+generate_trap(env, TRAPC_CTX_MNG, TIN3_FCD);
 }
 psw_write(env, psw);
 }
@@ -2456,18 +2459,25 @@ void helper_ret(CPUTriCoreState *env)
 
 psw = psw_read(env);
  /* if (PSW.CDE) then if (cdc_decrement()) then trap(CDU);*/
-if (env->PSW & MASK_PSW_CDE) {
-if (cdc_decrement(&(env->PSW))) {
+if (psw & MASK_PSW_CDE) {
+if (cdc_decrement()) {
 /* CDU trap */
+psw_write(env, psw);
+generate_trap(env, TRAPC_CTX_MNG, TIN3_CDU);
 }
 }
 /*   if (PCXI[19: 0] == 0) then trap(CSU); */
 if ((env->PCXI & 0xf) == 0) {
 /* CSU trap */
+psw_write(env, psw);
+generate_trap(env, TRAPC_CTX_MNG, TIN3_CSU);
 }
 /* if (PCXI.UL == 0) then trap(CTYP); */
 if ((env->PCXI & MASK_PCXI_UL) == 0) {
 /* CTYP trap */
+cdc_increment(); /* restore to the start of helper */
+psw_write(env, psw);
+generate_trap(env, TRAPC_CTX_MNG, TIN3_CTYP);
 }
 /* PC = {A11 [31: 1], 1’b0}; */
 env->PC = env->gpr_a[11] & 0xfffe;
@@ -2502,6 +2512,7 @@ void helper_bisr(CPUTriCoreState *env, uint32_t const9)
 
 if (env->FCX == 0) {
 /* FCU trap */
+   generate_trap(env, TRAPC_CTX_MNG, TIN3_FCU);
 }
 
 tmp_FCX = env->FCX;
@@ -2533,6 +2544,7 @@ void helper_bisr(CPUTriCoreState *env, uint32_t const9)
 
 if (tmp_FCX == env->LCX) {
 /* FCD trap */
+generate_trap(env, TRAPC_CTX_MNG, TIN3_FCD);
 }
 }
 
@@ -2544,14 +2556,17 @@ void helper_rfe(CPUTriCoreState *env)
 /* if (PCXI[19: 0] == 0) then trap(CSU); */
 if ((env->PCXI & 0xf) == 0) {
 /* raise csu trap */
+generate_trap(env, TRAPC_CTX_MNG, TIN3_CSU);
 }
 /* if (PCXI.UL == 0) then trap(CTYP); */
 if ((env->PCXI & MASK_PCXI_UL) == 0) {
 /* raise CTYP trap */
+generate_trap(env, TRAPC_CTX_MNG, TIN3_CTYP);
 }
 /* if (!cdc_zero() AND PSW.CDE) then trap(NEST); */
 if (!cdc_zero(&(env->PSW)) && (env->PSW & MASK_PSW_CDE)) {
-/* raise MNG trap */
+/* raise NEST trap */
+generate_trap(env, TRAPC_CTX_MNG, TIN3_NEST);
 }
 env->PC = env->gpr_a[11] & ~0x1;
 /* ICR.IE = PCXI.PIE; */
@@ -2627,6 +2642,7 @@ void helper_svlcx(CPUTriCoreState *env)
 
 if (env->FCX == 0) {
 /* FCU trap */
+generate_trap(env, TRAPC_CTX_MNG, TIN3_FCU);
 }
 /* tmp_FCX = FCX; */
 tmp_FCX = env->FCX;
@@ -2657,6 +2673,7 @@ void helper_svlcx(CPUTriCoreState *env)
 /* if (tmp_FCX == LCX) trap(FCD);*/
 if (tmp_FCX == env->LCX) {
 /* FCD trap */
+generate_trap(env, TRAPC_CTX_MNG, TIN3_FCD);
 }
 }
 
@@ -2668,6 +2685,7 @@ void helper_svucx(CPUTriCoreState *env)
 
 if (env->FCX == 0) {
 /* FCU trap */
+generate_trap(env, TRAPC_CTX_MNG, TIN3_FCU);
 }
 /* tmp_FCX = FCX; */
 tmp_FCX = env->FCX;
@@ -2698,6 +2716,7 @@ void helper_svucx(CPUTriCoreState *env)
 /* if (tmp_FCX == LCX) trap(FCD);*/
 if (tmp_FCX == env->LCX) {
 /* FCD trap */
+generate_trap(env, TRAPC_CTX_MNG, TIN3_FCD);
 }
 }
 
@@ -2708,10 +2727,12 @@ void helper_rslcx(CPUTriCoreState *env)
 /*   if (PCXI[19: 0] == 0) then trap(CSU); */
 if ((env->PCXI & 0xf) == 0) {
 /* CSU trap */
+generate_trap(env, TRAPC_CTX_MNG, TIN3_CSU);
 }
 /* if (PCXI.UL == 1) then trap(CTYP); */
 if ((env->PCXI & MASK_PCXI_UL) != 0) {
 /* CTYP trap */
+generate_trap(env, TRAPC_CTX_MNG, TIN3_CTYP);
 }
 /* EA = {PCXI.PCXS, 6'b0, PCXI.PCXO, 6'b0}; */
 ea = ((env->PCXI & MASK_PCXI_PCXS) << 12) +
-- 
2.7.1




Re: [Qemu-devel] [PATCH 6/9] pc: acpi: create MADT.lapic entries only for valid lapics

2016-02-11 Thread Eduardo Habkost
On Fri, Feb 05, 2016 at 05:14:41PM +0100, Igor Mammedov wrote:
> On Fri, 5 Feb 2016 13:28:31 -0200
> Eduardo Habkost  wrote:
> 
> > On Thu, Feb 04, 2016 at 12:47:32PM +0100, Igor Mammedov wrote:
> > > do not assume that all lapics in range 0..apic_id_limit
> > > are valid and do not create lapic entries for not
> > > possible lapics in MADT.
> > > 
> > > Signed-off-by: Igor Mammedov   
> > 
> > Reviewed-by: Eduardo Habkost 
> > 
> > But there's one minor suggestion below:
> > 
> > > ---
> > >  hw/i386/acpi-build.c | 21 ++---
> > >  1 file changed, 14 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > > index df13c7d..9eeeffa 100644
> > > --- a/hw/i386/acpi-build.c
> > > +++ b/hw/i386/acpi-build.c
> > > @@ -361,9 +361,11 @@ build_fadt(GArray *table_data, GArray *linker, 
> > > AcpiPmInfo *pm,
> > >  }
> > >  
> > >  static void
> > > -build_madt(GArray *table_data, GArray *linker, AcpiCpuInfo *cpu,
> > > -   PcGuestInfo *guest_info)
> > > +build_madt(GArray *table_data, GArray *linker,
> > > +   MachineState *machine, PcGuestInfo *guest_info)
> > >  {
> > > +MachineClass *mc = MACHINE_GET_CLASS(machine);
> > > +GArray *apic_id_list = mc->possible_cpu_arch_ids();
> > >  int madt_start = table_data->len;
> > >  
> > >  AcpiMultipleApicTable *madt;
> > > @@ -376,18 +378,23 @@ build_madt(GArray *table_data, GArray *linker, 
> > > AcpiCpuInfo *cpu,
> > >  madt->local_apic_address = cpu_to_le32(APIC_DEFAULT_ADDRESS);
> > >  madt->flags = cpu_to_le32(1);
> > >  
> > > -for (i = 0; i < guest_info->apic_id_limit; i++) {
> > > +for (i = 0; i < apic_id_list->len; i++) {
> > >  AcpiMadtProcessorApic *apic = acpi_data_push(table_data, sizeof 
> > > *apic);
> > > +CPUArchId id = FETCH_CPU_ARCH_ID(apic_id_list, i);
> > > +int apic_id = id.arch_id;
> > > +
> > >  apic->type = ACPI_APIC_PROCESSOR;
> > >  apic->length = sizeof(*apic);
> > > -apic->processor_id = i;
> > > -apic->local_apic_id = i;
> > > -if (test_bit(i, cpu->found_cpus)) {
> > > +apic->processor_id = apic_id;
> > > +apic->local_apic_id = apic_id;
> > > +if (id.cpu != NULL) {  
> > 
> > This seems to be the only place where CPUArchId.cpu is being used
> > (see my previous suggestion about making possible_cpu_arch_ids()
> > return just an uint64_t list).
> > 
> > Also, using the existing found_cpus bitmap is more efficient than
> > making multiple calls to qemu_get_cpu_by_arch_id(). I wouldn't
> > mind keeping the bitmap.
> found_cpus bitmap is not better than (id.cpu != NULL) check
> the cost of filling both is about the same.

The cost doesn't look the same. Populating found_cpus should be
O(smp_cpus)[1], and is being done only once. Filling id.cpu in
pc_possible_cpu_arch_ids() is O(max_cpus*smp_cpus), and it is
called multiple times.

[1] I just noticed it is actually O(size_of_qom_tree), but it
is still linear, and could be changed to O(smp_cpus).

> The issue I have with bitmap is that it's harder to generalize
> CPU hotplug code with it, while with possible_cpu_arch_ids()
> returned array I have a list of CPUs to work with without any
> assumptions on position in bitmap or array.
> Also bitmap scales worse than a list of CPUs if ID space
> is sparse and if ID is quite big.

Yes, I agree that a list is better depending on how the arch ID
space is used.

> That's why I'm dropping bitmap and switching to a list of
> IDs which in worst case is upto max_cpus.

I think the possible_cpu_arch_ids() interface looks good, maybe
we just need to optimize it.

But I'm not sure if we need to optimize it now, or if we can live
with the inefficient code and optimize it later. I won't complain
if we do it later, if we warn about it in the commit message or
comments.

-- 
Eduardo



[Qemu-devel] cache.direct

2016-02-11 Thread Jignasha Vithalani
How to set cache.direct = on if using aio=native with qemu 2.3
while mounting with nbd


Re: [Qemu-devel] [PATCH 7/9] pc: acpi: drop not needed intermediate bitmap cpu->found_cpus

2016-02-11 Thread Eduardo Habkost
On Fri, Feb 05, 2016 at 05:44:49PM +0100, Igor Mammedov wrote:
> On Fri, 5 Feb 2016 17:19:50 +0100
> Igor Mammedov  wrote:
> 
> > On Fri, 5 Feb 2016 13:39:07 -0200
> > Eduardo Habkost  wrote:
> > 
> > > On Thu, Feb 04, 2016 at 12:47:33PM +0100, Igor Mammedov wrote:  
> > > > cpu->found_cpus bitmap is used for setting present
> > > > flag in CPON AML package at start up. But it takes
> > > > a bunch of code to fill bitmap and cloud be simplified
> > > > by calling qemu_get_cpu_by_arch_id(apic_id) directly.
> > > > 
> > > > Hence do so and remove not used anymore bitmap
> > > > with related utilities, which saves us ~32LOC
> > > > and also would simplify consolidating APCI parts
> > > > of CPU hotplug.
> > > > 
> > > > Signed-off-by: Igor Mammedov 
> > > 
> > > This makes the code loops through all smp_cpus CPUs max_cpus
> > > times, instead of just looping through the smp_cpus CPUs once.  
> > Yep, that looks bad.
> > I'll redo it using possible_cpu_arch_ids(), it is a little
> > bit bigger refactoring.
> I think I'll make bitmap local to build_processor_devices() as it's
> needed only for building CPON package, that way it will go along
> with related legacy hotplug and won't get in the way of new hotplug.

What about pc_possible_cpu_arch_ids() itself? It has exactly the
same problem: it looks at CPU objects smp_cpus*max_cpus times, to
fill the CPUArchId.cpu field. If that's a problem for
build_processor_devices(), I assume that's a problem for all
other possible_cpu_arch_ids() callers?

-- 
Eduardo



Re: [Qemu-devel] [PATCH v2 1/6] target-arm: correct CNTFRQ access rights

2016-02-11 Thread Sergey Fedorov
On 11.02.2016 19:03, Peter Maydell wrote:
> Correct some corner cases we were getting wrong for
> CNTFRQ access rights:
>  * should UNDEF from 32-bit Secure EL1
>  * only writable from the highest implemented exception level,
>which might not be EL1 now
>
> To clarify the code, provide a new utility function
> arm_highest_el() which returns the highest implemented
> exception level.
>
> Signed-off-by: Peter Maydell 

Reviewed-by: Sergey Fedorov 

> ---
> Rewritten to use arm_highest_el() to improve clarity
> ---
>  target-arm/cpu.h| 12 
>  target-arm/helper.c | 29 ++---
>  2 files changed, 38 insertions(+), 3 deletions(-)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 5137632..afbf366 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -1255,6 +1255,18 @@ static inline bool cptype_valid(int cptype)
>  #define PL1_RW (PL1_R | PL1_W)
>  #define PL0_RW (PL0_R | PL0_W)
>  
> +/* Return the highest implemented Exception Level */
> +static inline int arm_highest_el(CPUARMState *env)
> +{
> +if (arm_feature(env, ARM_FEATURE_EL3)) {
> +return 3;
> +}
> +if (arm_feature(env, ARM_FEATURE_EL2)) {
> +return 2;
> +}
> +return 1;
> +}
> +
>  /* Return the current Exception Level (as per ARMv8; note that this differs
>   * from the ARMv7 Privilege Level).
>   */
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 2f9db72..4d27c00 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -1218,10 +1218,33 @@ static const ARMCPRegInfo v6k_cp_reginfo[] = {
>  static CPAccessResult gt_cntfrq_access(CPUARMState *env, const ARMCPRegInfo 
> *ri,
> bool isread)
>  {
> -/* CNTFRQ: not visible from PL0 if both PL0PCTEN and PL0VCTEN are zero */
> -if (arm_current_el(env) == 0 && !extract32(env->cp15.c14_cntkctl, 0, 2)) 
> {
> -return CP_ACCESS_TRAP;
> +/* CNTFRQ: not visible from PL0 if both PL0PCTEN and PL0VCTEN are zero.
> + * Writable only at the highest implemented exception level.
> + */
> +int el = arm_current_el(env);
> +
> +switch (el) {
> +case 0:
> +if (!extract32(env->cp15.c14_cntkctl, 0, 2)) {
> +return CP_ACCESS_TRAP;
> +}
> +break;
> +case 1:
> +if (!isread && ri->state == ARM_CP_STATE_AA32 &&
> +arm_is_secure_below_el3(env)) {
> +/* Accesses from 32-bit Secure EL1 UNDEF (*not* trap to EL3!) */
> +return CP_ACCESS_TRAP_UNCATEGORIZED;
> +}
> +break;
> +case 2:
> +case 3:
> +break;
>  }
> +
> +if (!isread && el < arm_highest_el(env)) {
> +return CP_ACCESS_TRAP_UNCATEGORIZED;
> +}
> +
>  return CP_ACCESS_OK;
>  }
>  




Re: [Qemu-devel] [PATCH v7 3/5] acpi: pc: add fw_cfg device node to ssdt

2016-02-11 Thread Gabriel L. Somlo
On Thu, Feb 11, 2016 at 04:19:59PM +0100, Igor Mammedov wrote:
> On Wed, 10 Feb 2016 15:41:38 -0500
> "Gabriel L. Somlo"  wrote:
> 
> > Add a fw_cfg device node to the ACPI SSDT. While the guest-side
> > firmware can't utilize this information (since it has to access
> > the hard-coded fw_cfg device to extract ACPI tables to begin with),
> > having fw_cfg listed in ACPI will help the guest kernel keep a more
> > accurate inventory of in-use IO port regions.
> subj and commit msg:
> s/SSDT/DSDT/

Thanks for catching that, got it lined up for v8 :)

> > 
> > Signed-off-by: Gabriel Somlo 
> > Reviewed-by: Laszlo Ersek 
> > Reviewed-by: Marc Marí 
> > ---
> >  hw/i386/acpi-build.c | 29 +
> >  1 file changed, 29 insertions(+)
> > 
> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > index 4554eb8..4762fd2 100644
> > --- a/hw/i386/acpi-build.c
> > +++ b/hw/i386/acpi-build.c
> > @@ -2190,6 +2190,35 @@ build_dsdt(GArray *table_data, GArray *linker,
> >  aml_append(scope, aml_name_decl("_S5", pkg));
> >  aml_append(dsdt, scope);
> >  
> > +/* create fw_cfg node, unconditionally */
> Will that unconditionally make all Windows guests ask for driver for unknown 
> device?

That didn't happen in my tests. With _STA set to 0xB, we have the
bit representing "device shown in the UI" set to "off", and so Windows
(XP and Win7 in my tests) completely ignore it.

Originally I was asked (by Eduardo, IIRC) to make insertion of the
fw_cfg acpi node conditional on the machine version, but then later we
collectively agreed to no longer require that, so there wasn't an if (...)
condition to put in front of the { ... } block anymore. I replaced the
condition with the comment that says "add unconditionally"; I can change
the wording on that if it's confusing, but I'd like to keep the extra
curly braces to match the way all other surrounding ACPI nodes are added.

Thanks,
--Gabriel

> > +{
> > +/* when using port i/o, the 8-bit data register *always* overlaps
> > + * with half of the 16-bit control register. Hence, the total size
> > + * of the i/o region used is FW_CFG_CTL_SIZE; when using DMA, the
> > + * DMA control register is located at FW_CFG_DMA_IO_BASE + 4 */
> > +uint8_t io_size = object_property_get_bool(OBJECT(pcms->fw_cfg),
> > +   "dma_enabled", NULL) ?
> > +  ROUND_UP(FW_CFG_CTL_SIZE, 4) + 
> > sizeof(dma_addr_t) :
> > +  FW_CFG_CTL_SIZE;
> > +
> > +scope = aml_scope("\\_SB");
> > +dev = aml_device("FWCF");
> > +
> > +aml_append(dev, aml_name_decl("_HID", aml_string("QEMU0002")));
> > +
> > +/* device present, functioning, decoding, not shown in UI */
> > +aml_append(dev, aml_name_decl("_STA", aml_int(0xB)));
> > +
> > +crs = aml_resource_template();
> > +aml_append(crs,
> > +aml_io(AML_DECODE16, FW_CFG_IO_BASE, FW_CFG_IO_BASE, 0x01, 
> > io_size)
> > +);
> > +aml_append(dev, aml_name_decl("_CRS", crs));
> > +
> > +aml_append(scope, dev);
> > +aml_append(dsdt, scope);
> > +}
> > +
> >  if (misc->applesmc_io_base) {
> >  scope = aml_scope("\\_SB.PCI0.ISA");
> >  dev = aml_device("SMC");
> 



  1   2   >