Re: [Qemu-devel] [RFC v1] Add declarations for hierarchical memory region API

[Avi Kivity]

On 05/20/2011 05:06 PM, Richard Henderson wrote:

Is this structure honestly any better than 4 function pointers?
I can't see that it is, myself.



That was requested by Anthony.  And in fact we have two bits of 
information per access size, one is whether the access is allowed or 
not, the other is whether we want to use a larger or smaller access size 
function (useful for auto-splitting a 64-bit access into two 32-bit 
accesses for example).


--
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [Qemu-devel] [RFC v1] Add declarations for hierarchical memory region API

[Avi Kivity]

On 05/20/2011 08:59 PM, Blue Swirl wrote:

On Thu, May 19, 2011 at 5:12 PM, Avi Kivity  wrote:
>  The memory API separates the attributes of a memory region (its size, how
>  reads or writes are handled, dirty logging, and coalescing) from where it
>  is mapped and whether it is enabled.  This allows a device to configure
>  a memory region once, then hand it off to its parent bus to map it according
>  to the bus configuration.
>
>  Hierarchical registration also allows a device to compose a region out of
>  a number of sub-regions with different properties; for example some may be
>  RAM while others may be MMIO.
>
>  +/* Destroy a memory region.  The memory becomes inaccessible. */
>  +void memory_region_destroy(MemoryRegion *mr);

Doesn't the lower priority region become accessible instead in some cases?


If we require that _add_subregion() and _del_subregion() be paired, then no.

--
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [Qemu-devel] [RFC] Memory API

[Avi Kivity]

On 05/20/2011 08:30 PM, Blue Swirl wrote:

Another case would be the cache-as-ram mode for some x86 CPUs, which
Coreboot people would like to see IIRC.


That's probably best handled as a cache emulation layer, as this is not 
associated with any specific address range.


--
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [Qemu-devel] [RFC] Memory API

[Avi Kivity]

On 05/20/2011 05:51 PM, Anthony Liguori wrote:

Of course there is overlap. PCI BARs overlap each other, the VGA windows
and ROM overlap RAM.



Here's what I'm still struggling with:

If children normally overlap their parents, but child priorities are 
always less than their parents, then what's the benefit of having 
anything more than two priorities settings.


As far as I can understand it, a priority of 0 means "let children 
windows overlap" whereas a priority of 1 means "don't let children 
windows overlap".


Is there a use-case for a priority above 1 and if so, what does it mean?


Children always overlap their parents.  Priorities are among children of 
the same parent.


I expect there won't be a use case for priority 2, but that's not 
because of some inherent property of the design; the hardware designers 
simply haven't got around to designing such whacky hardware.


Note that a container is transparent, so layering several containers on 
top of each other simply generates a flattening of the tree.   You can 
have an opaque container by having a lowest-priority child that covers 
the entire address space.



That's what *exactl* what priority means. Which device is in front, and
which is in the back.


Why not use registration order to resolve this type of conflict?  What 
are the use cases to use priorities where registration order wouldn't 
be adequate?


Registration order is not something you want in a declarative API.  
There is a non-priority equivalent, and that is to unregister 
(_del_subregion) all subregions in the contended region, except the one 
you want.


That doesn't work for PCI, since the "contended region" isn't known.  
You want RAM to hide PCI BARs (or perhaps vice versa), you need to tell 
the system which one takes precedence.






There is no need to have centralized logic to decide this.



I think you're completely missing the point of my proposal.


I'm struggling to find the mental model for priorities.  I may just be 
dense here but the analogy of transparent window ordering isn't 
helping me.




If you like, you can think of "priority" as "explicit registration 
order".  The advantage is that is works dynamically, if a region is 
unregistered and re-registered, that doesn't mess up your registration 
order.



--
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [Qemu-devel] [RFC] Memory API

[Avi Kivity]

On 05/20/2011 02:57 PM, Gleb Natapov wrote:

On Fri, May 20, 2011 at 11:59:58AM +0300, Avi Kivity wrote:
>  On 05/19/2011 07:27 PM, Gleb Natapov wrote:
>  >>   Think of how a window manager folds windows with priorities onto a
>  >>   flat framebuffer.
>  >>
>  >>   You do a depth-first walk of the tree.  For each child list, you
>  >>   iterate it from the lowest to highest priority, allowing later
>  >>   subregions override earlier subregions.
>  >>
>  >I do not think that window manager is a good analogy. Window can
>  >overlap with only its siblings. In our memory tree each final node may
>  >overlap with any other node in the tree.
>  >
>
>  Transparent windows.
>
No, still not that. Think about child windows that resides outside of its
parent windows on screen. In our memory region terms think about PCI BAR
is registered to overlap with RAM at address 0x1000 for instance. PCI
BAR memory region and RAM memory region are on very different branches
of the global tree.


Right.  But what's the problem with that?

Which one takes precedence is determined by the priorities of the RAM 
subregion vs. the PCI bus subregion.


--
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [Qemu-devel] [RFC] Memory API

[Avi Kivity]

On 05/20/2011 06:59 PM, Jan Kiszka wrote:

>
>  Jan had mentioned previously about registering a new temporary window.
>  I assume the registration always gets highest_priority++, or do you have
>  to explicitly specify that PCI container gets priority=1?

The latter.

And I really prefer to have this explicit over deriving the priority
from the registration order. That's way too fragile/unhandy. If you
decide to replace a region of lower priority later on, you need to
reregister everything at that level.


Exactly.

--
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [Qemu-devel] [RFC] Memory API

[Avi Kivity]

On 05/20/2011 02:25 PM, Gleb Natapov wrote:

>
>  A) Removing regions will change significantly. So far this is done by
>  setting a region to IO_MEM_UNASSIGNED, keeping truncation. With the new
>  API that will be a true removal which will additionally restore hidden
>  regions.
>
And what problem do you expect may arise from that? Currently accessing
such region after unassign will result in undefined behaviour, so this
code is non working today, you can't make it worse.



If the conversion were perfect then yes.  However there is a possibility 
that the conversion will not be perfect.


It's also good to have to have the code document its intentions.  If you 
see _overlap() you know there is dynamic address decoding going on, or 
something clever.



>  B) Uncontrolled overlapping is a bug that should be caught by the core,
>  and a new API is a perfect chance to do this.
>
Well, this will indeed introduce the difference in behaviour :) The guest
that ran before will abort now. Are you actually aware of any such
overlaps in the current code base?


Put a BAR over another BAR, then unmap it.


But if priorities are gona stay why not fail if two regions with the
same priority overlap? If that happens it means that the memory creation
didn't pass the point where conflict should have been resolved (by
assigning different priorities) and this means that overlap is
unintentional, no?


It may be intentional, as in the case of PCI, or PAM (though you can do 
PAM without priorities, by removing all but one of the subregions in the 
area).



I am starting to see how you can represent all this local decisions as
priority numbers and then travel this weighted tree to find what memory
region should be accessed (memory registration _has_ to be hierarchical
for that to work in meaningful way).


Priorities don't travel up the tree.  They're used to resolve local 
conflicts *only*.



  I still don't see why it is better
than flattening the tree in the point of conflict.


How do you decide which subregion wins?

>  Not necessarily. It depends on how much added value buses like PCI or
>  ISA or whatever can offer for managing I/O regions. For some purposes,
>  it may as well be fine to just call the memory_* service directly and
>  pass the result of some operation to the bus API later on.
Depend on what memory_* service you are talking about. Just creating
unattached memory region is OK. But if two independent pieces of code
want to map two different memory regions into the same phys address I do
not see who will resolve the conflict.


They have to ask the bus to _add_subregion().  Only the bus knows about 
the priorities (or the bus can ask them to create the subregions).



>
>  >  PCI
>  >  device will call PCI subsystem. PCI subsystem, instead of assigning
>  >  arbitrary priorities to all overlappings,
>
>  Again: PCI will _not_ assign arbitrary priorities but only
>  MEMORY_REGION_DEFAULT_PRIORITY, likely 0.
That is as arbitrary as it can get. Just assigning
MEMORY_REGION_DEFAULT_PRIORITY/2^0xfff will work equally well, so what
is not arbitrary about that number?


That's just splitting hairs.  Array indexes start from zero, an 
arbitrary but convenient number.



BTW why wouldn't PCI layer assign different priorities to overlapping
regions to let the core know which one should be actually available? Why
leave this decision to the core if it is clearly belongs to PCI?


You mean overlapping BARs?  If PCI wants BAR 1 to override BAR 2, then 
it can indicate it with priorities.  If it doesn't want to, it can use 
the same priority for all regions.



>
>  That does not specify how the PCI bridge or the chipset will tell that
>  overlapping resolution lib _how_ overlapping regions shall be translated
>  into a flat representation. And precisely here come priorities into
>  play. It is the way to tell that lib either "region A shall override
>  region B" if A has higher prio or "if region A and B overlap, do
>  whatever you want" if both have the same prio.
>
Yep! And the question is why shouldn't this be done on the level that
knows most about the conflict but propagated to the core. I am not
arguing that priorities do not exists! Obviously they are. I am
questioning the usefulness of priorities be part of memory core API.



The chipset knows about the priorities.  How to communicate them to the 
core?


- at runtime, with hierarchical dispatch of ->read() and ->write(): 
slow, and doesn't work at all for RAM.

- using registration order: fragile
- using priorities

We need to get the information out of the chipset and into the core, so 
the core can make use of it (like flattening the tree to produce kvm slots).


--
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [Qemu-devel] [RFC] Memory API

[Avi Kivity]

On 05/20/2011 03:08 PM, Gleb Natapov wrote:

On Fri, May 20, 2011 at 12:10:22PM +0300, Avi Kivity wrote:
>  On 05/19/2011 09:22 PM, Gleb Natapov wrote:
>  >>
>  >>   BARs may overlap with other BARs or with RAM. That's well-known, so PCI
>  >>   bridged need to register their regions with the _overlap variant
>  >>   unconditionally. In contrast to the current PhysPageDesc mechanism, the
>  >With what priority?
>
>  It doesn't matter, since the spec doesn't define priorities among PCI BARs.
>
And among PCI BAR and memory (the case the question above referred too).


One of them gets priority 0, the other gets priority 1.  Depending on 
who should win according to the spec.




>  >If it needs to call _overlap unconditionally why not
>  >always call _overlap and drop not _overlap variant?
>
>  Other uses need non-overlapping registration.
And who prohibit them from creating one?


Nothing.


>
>  >>
>  >>   And they do not need to. The APIC regions will be managed by the per-CPU
>  >>   region management, reusing the tool box we need for all bridges. It will
>  >>   register the APIC page with a priority higher than the default one, thus
>  >>   overriding everything that comes from the host bridge. I think that
>  >>   reflects pretty well real machine behaviour.
>  >>
>  >What is "higher"? How does it know that priority is high enough?
>
>  It is well known that 1>  0, for example.
>
That is if you have global scale. In the case I am asking about you do
not. Even if PCI will register memory region that overlaps APIC address
with priority 1000 APIC memory region should still be able to override
it even with priority 0. Voila 1000<  0? Where is your sarcasm now? :)


This can be done in two ways:

1. Assign APIC the highest priority.  Priority is not determined by the 
guest, but by qemu. Problem solved.

2. Use a hierarchy:


root
  |
  +-- RAM (prio 0)
  |
  +-- PCI (prio 1, lots of children)
  |
  +-- APIC (prio 2)


Nothing under the PCI subregion can obscure the APIC.



But Jan already answered this one. Actually what really matters is the
place of the node in a topology, not priority.


You need priority for the children of the same parent.


  But then for all of this
to make sense registration has to be hierarchical.


Well, that's the whole point.

--
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [Qemu-devel] mouse doesn't work on guest OS

[John Haxby]

On 22 May 2011, at 00:32, Brad Hards wrote:

>> 
>> My problem is
>> that, mouse is hanged in the middle of the screen.
> I still don't understand the problem. I'm guessing you see the cursor in the 
> guest, but the host mouse isn't having any effect on that guest cursor.

I do :-)

I'm pretty sure that this is a problem whereby the mouse is advertising both 
absolute and relative positioning to Xorg, but is actually only sending 
absolute events -- which Xorg is ignoring.

Check /var/log/Xorg.0.log (or whatever Xorg is logging to) and look for "mouse" 
or "evdev" or "ignor".   If I'm right then you need to amend the config of the 
X server.   I don't have the details to hand, but it's in the Xorg man page.  
Or just google for the relevant message you find in the log file.

jch

[Qemu-devel] [Bug 786440] [NEW] qcow2 double free

[Andrew Kroll]

Public bug reported:

version 0.14.1 when using qcow2 images, after some time, glibc detects a
double free or corruption.

** Affects: qemu
 Importance: Undecided
 Status: New

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

Title:
  qcow2 double free

Status in QEMU:
  New

Bug description:
  version 0.14.1 when using qcow2 images, after some time, glibc detects
  a double free or corruption.

Re: [Qemu-devel] [RFC] Memory API

[Gleb Natapov]

On Sun, May 22, 2011 at 10:37:48AM +0300, Avi Kivity wrote:
> On 05/20/2011 02:57 PM, Gleb Natapov wrote:
> >On Fri, May 20, 2011 at 11:59:58AM +0300, Avi Kivity wrote:
> >>  On 05/19/2011 07:27 PM, Gleb Natapov wrote:
> >>  >>   Think of how a window manager folds windows with priorities onto a
> >>  >>   flat framebuffer.
> >>  >>
> >>  >>   You do a depth-first walk of the tree.  For each child list, you
> >>  >>   iterate it from the lowest to highest priority, allowing later
> >>  >>   subregions override earlier subregions.
> >>  >>
> >>  >I do not think that window manager is a good analogy. Window can
> >>  >overlap with only its siblings. In our memory tree each final node may
> >>  >overlap with any other node in the tree.
> >>  >
> >>
> >>  Transparent windows.
> >>
> >No, still not that. Think about child windows that resides outside of its
> >parent windows on screen. In our memory region terms think about PCI BAR
> >is registered to overlap with RAM at address 0x1000 for instance. PCI
> >BAR memory region and RAM memory region are on very different branches
> >of the global tree.
> 
> Right.  But what's the problem with that?
> 
None, unless you want to make PCI BAR visible at address 0x1000 (like what
will happen today) the case above. 

> Which one takes precedence is determined by the priorities of the
> RAM subregion vs. the PCI bus subregion.
> 
Yes, and that is why PCI subsystem or platform code can't directly uses
memory API to register PCI memory region/RAM memory region respectively,
because they wouldn't know what priorities to specify. Only chipset code
knows, so the RAM/PCI memory registration should go through chipset
code, but even chipset code doesn't know everything. It knows nothing
about cpu local memory regions for instance, so all registrations should
go through system bus in the end. Is this how API suppose to be used?

--
Gleb.

Re: [Qemu-devel] [RFC] Memory API

[Avi Kivity]

On 05/22/2011 11:06 AM, Gleb Natapov wrote:

On Sun, May 22, 2011 at 10:37:48AM +0300, Avi Kivity wrote:
>  On 05/20/2011 02:57 PM, Gleb Natapov wrote:
>  >On Fri, May 20, 2011 at 11:59:58AM +0300, Avi Kivity wrote:
>  >>   On 05/19/2011 07:27 PM, Gleb Natapov wrote:
>  >>   >>Think of how a window manager folds windows with priorities onto a
>  >>   >>flat framebuffer.
>  >>   >>
>  >>   >>You do a depth-first walk of the tree.  For each child list, you
>  >>   >>iterate it from the lowest to highest priority, allowing later
>  >>   >>subregions override earlier subregions.
>  >>   >>
>  >>   >I do not think that window manager is a good analogy. Window can
>  >>   >overlap with only its siblings. In our memory tree each final node may
>  >>   >overlap with any other node in the tree.
>  >>   >
>  >>
>  >>   Transparent windows.
>  >>
>  >No, still not that. Think about child windows that resides outside of its
>  >parent windows on screen. In our memory region terms think about PCI BAR
>  >is registered to overlap with RAM at address 0x1000 for instance. PCI
>  >BAR memory region and RAM memory region are on very different branches
>  >of the global tree.
>
>  Right.  But what's the problem with that?
>
None, unless you want to make PCI BAR visible at address 0x1000 (like what
will happen today) the case above.


There is no problem.  If the PCI bus priority is higher than RAM 
priority, then PCI BARs will override RAM.



>  Which one takes precedence is determined by the priorities of the
>  RAM subregion vs. the PCI bus subregion.
>
Yes, and that is why PCI subsystem or platform code can't directly uses
memory API to register PCI memory region/RAM memory region respectively,
because they wouldn't know what priorities to specify. Only chipset code
knows, so the RAM/PCI memory registration should go through chipset
code,


Correct.  Chipset code creates RAM and PCI regions, and gives the PCI 
region to the PCI bus.  Devices give BAR subregions to the PCI bus.  The 
PCI bus does the registration.



  but even chipset code doesn't know everything. It knows nothing
about cpu local memory regions for instance, so all registrations should
go through system bus in the end. Is this how API suppose to be used?


Yes.  Every point where a decision is made on how to route memory 
accesses is a modelled as a container node.


--
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

[Qemu-devel] [Bug 786442] [NEW] GCC -O2 causes segfaults

[Andrew Kroll]

Public bug reported:

unless compiled without optimizations, no system may be ran except the default 
with -kvm-enabled
I had to modify config-host.mak and remove -O2 from CFLAGS to be able to work 
without kvm.

GCC 4.4.4 qemu-0.14.1
***NOTE: this has been an issue for several versions.

** Affects: qemu
 Importance: Undecided
 Status: New

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

Title:
  GCC -O2 causes segfaults

Status in QEMU:
  New

Bug description:
  unless compiled without optimizations, no system may be ran except the 
default with -kvm-enabled
  I had to modify config-host.mak and remove -O2 from CFLAGS to be able to work 
without kvm.

  GCC 4.4.4 qemu-0.14.1
  ***NOTE: this has been an issue for several versions.

Re: [Qemu-devel] [RFC] Memory API

[Gleb Natapov]

On Sun, May 22, 2011 at 10:50:22AM +0300, Avi Kivity wrote:
> On 05/20/2011 02:25 PM, Gleb Natapov wrote:
> >>
> >>  A) Removing regions will change significantly. So far this is done by
> >>  setting a region to IO_MEM_UNASSIGNED, keeping truncation. With the new
> >>  API that will be a true removal which will additionally restore hidden
> >>  regions.
> >>
> >And what problem do you expect may arise from that? Currently accessing
> >such region after unassign will result in undefined behaviour, so this
> >code is non working today, you can't make it worse.
> >
> 
> If the conversion were perfect then yes.  However there is a
> possibility that the conversion will not be perfect.
> 
> It's also good to have to have the code document its intentions.  If
> you see _overlap() you know there is dynamic address decoding going
> on, or something clever.
> 
> >>  B) Uncontrolled overlapping is a bug that should be caught by the core,
> >>  and a new API is a perfect chance to do this.
> >>
> >Well, this will indeed introduce the difference in behaviour :) The guest
> >that ran before will abort now. Are you actually aware of any such
> >overlaps in the current code base?
> 
> Put a BAR over another BAR, then unmap it.
> 
_overlap will not help with that. PCI BARs can overlap, so _overlap will
be used to register them. You do not what to abort qemu when guest
configure overlapping PCI BARs don't you?

> >But if priorities are gona stay why not fail if two regions with the
> >same priority overlap? If that happens it means that the memory creation
> >didn't pass the point where conflict should have been resolved (by
> >assigning different priorities) and this means that overlap is
> >unintentional, no?
> 
> It may be intentional, as in the case of PCI, or PAM (though you can
> do PAM without priorities, by removing all but one of the subregions
> in the area).
> 
When two PCI BARs overlap, somebody somewhere has to decide which one
of them will be visible. You can register both of them with same priority
and let the core to decide this at the time it calculates flattened view or
you can assign different priorities at PCI layer. In the later case you do
not need _overlap API.

> >I am starting to see how you can represent all this local decisions as
> >priority numbers and then travel this weighted tree to find what memory
> >region should be accessed (memory registration _has_ to be hierarchical
> >for that to work in meaningful way).
> 
> Priorities don't travel up the tree.  They're used to resolve local
> conflicts *only*.
Yes, that what I mean by "traveling weighted tree". At each node you
look at local priorities to decide where to move.

> 
> >  I still don't see why it is better
> >than flattening the tree in the point of conflict.
> 
> How do you decide which subregion wins?
The same way you decide what priority you assign to each region. You
do that at the point where you know the priorities. Suppose chipset has
RAM from 0x to 0x1fff and PCI from 0x2000 0x2fff and PCI layer tires
to map a BAR from 0x1500 to 0x3500. Since chipset code knows that RAM
has higher priority and it knows where PCI windows ends it can create two
memory regions from that. One is RAM from 0x to 0x1fff another PCI BAR
from 0x2000 to 0x2fff. How your tree will look like in this case BTW?


> >>  Not necessarily. It depends on how much added value buses like PCI or
> >>  ISA or whatever can offer for managing I/O regions. For some purposes,
> >>  it may as well be fine to just call the memory_* service directly and
> >>  pass the result of some operation to the bus API later on.
> >Depend on what memory_* service you are talking about. Just creating
> >unattached memory region is OK. But if two independent pieces of code
> >want to map two different memory regions into the same phys address I do
> >not see who will resolve the conflict.
> 
> They have to ask the bus to _add_subregion().  Only the bus knows
> about the priorities (or the bus can ask them to create the
> subregions).
Yes, that what I wrote and Jan responded to. Buses all the way down.

> 
> >>
> >>  >  PCI
> >>  >  device will call PCI subsystem. PCI subsystem, instead of assigning
> >>  >  arbitrary priorities to all overlappings,
> >>
> >>  Again: PCI will _not_ assign arbitrary priorities but only
> >>  MEMORY_REGION_DEFAULT_PRIORITY, likely 0.
> >That is as arbitrary as it can get. Just assigning
> >MEMORY_REGION_DEFAULT_PRIORITY/2^0xfff will work equally well, so what
> >is not arbitrary about that number?
> 
> That's just splitting hairs.  Array indexes start from zero, an
> arbitrary but convenient number.
The point is that instead of assigning priorities PCI subsystem may
resolve the conflict before passing registration to a chipset.

> 
> >BTW why wouldn't PCI layer assign different priorities to overlapping
> >regions to let the core know which one should be actually available? Why
> >leave this decision to the core if it is clearly belongs to PCI?
> 

Re: [Qemu-devel] [RFC] Memory API

[Gleb Natapov]

On Sun, May 22, 2011 at 11:09:08AM +0300, Avi Kivity wrote:
> On 05/22/2011 11:06 AM, Gleb Natapov wrote:
> >On Sun, May 22, 2011 at 10:37:48AM +0300, Avi Kivity wrote:
> >>  On 05/20/2011 02:57 PM, Gleb Natapov wrote:
> >>  >On Fri, May 20, 2011 at 11:59:58AM +0300, Avi Kivity wrote:
> >>  >>   On 05/19/2011 07:27 PM, Gleb Natapov wrote:
> >>  >>   >>Think of how a window manager folds windows with priorities 
> >> onto a
> >>  >>   >>flat framebuffer.
> >>  >>   >>
> >>  >>   >>You do a depth-first walk of the tree.  For each child list, 
> >> you
> >>  >>   >>iterate it from the lowest to highest priority, allowing later
> >>  >>   >>subregions override earlier subregions.
> >>  >>   >>
> >>  >>   >I do not think that window manager is a good analogy. Window can
> >>  >>   >overlap with only its siblings. In our memory tree each final node 
> >> may
> >>  >>   >overlap with any other node in the tree.
> >>  >>   >
> >>  >>
> >>  >>   Transparent windows.
> >>  >>
> >>  >No, still not that. Think about child windows that resides outside of its
> >>  >parent windows on screen. In our memory region terms think about PCI BAR
> >>  >is registered to overlap with RAM at address 0x1000 for instance. PCI
> >>  >BAR memory region and RAM memory region are on very different branches
> >>  >of the global tree.
> >>
> >>  Right.  But what's the problem with that?
> >>
> >None, unless you want to make PCI BAR visible at address 0x1000 (like what
> >will happen today) the case above.
> 
> There is no problem.  If the PCI bus priority is higher than RAM
> priority, then PCI BARs will override RAM.
> 
So if memory region has no subregion that covers part of the region
lower prio region is used? Now the same with the pictures:

-- root
  -- PCI 0x0 - 0x2 (prio 0)
-- BAR A 0x1000 - 0x1fff
-- BAR B 0x2 - 0x20fff
  -- RAM 0x0 - 0x1 (prio 1)
  
In the tree above at address 0x0 PCI has higher priority, but since
there is no subregion at this range next prio region is used instead
(RAM). Is this correct? If yes how core knows that container is
transparent like that (RAM container is not)?

> >>  Which one takes precedence is determined by the priorities of the
> >>  RAM subregion vs. the PCI bus subregion.
> >>
> >Yes, and that is why PCI subsystem or platform code can't directly uses
> >memory API to register PCI memory region/RAM memory region respectively,
> >because they wouldn't know what priorities to specify. Only chipset code
> >knows, so the RAM/PCI memory registration should go through chipset
> >code,
> 
> Correct.  Chipset code creates RAM and PCI regions, and gives the
> PCI region to the PCI bus.  Devices give BAR subregions to the PCI
> bus.  The PCI bus does the registration.
OK. What happens if device tries to create subregion outside of PCI
region provided by chipset to PCI bus?

> 
> >  but even chipset code doesn't know everything. It knows nothing
> >about cpu local memory regions for instance, so all registrations should
> >go through system bus in the end. Is this how API suppose to be used?
> 
> Yes.  Every point where a decision is made on how to route memory
> accesses is a modelled as a container node.
> 
Excellent. I would argue that this is exactly the point where an
overlapping can be resolved too :)

--
Gleb.

Re: [Qemu-devel] [RFC v1] Add declarations for hierarchical memory region API

[Blue Swirl]

On Sun, May 22, 2011 at 9:45 AM, Avi Kivity  wrote:
> On 05/20/2011 08:59 PM, Blue Swirl wrote:
>>
>> On Thu, May 19, 2011 at 5:12 PM, Avi Kivity  wrote:
>> >  The memory API separates the attributes of a memory region (its size,
>> > how
>> >  reads or writes are handled, dirty logging, and coalescing) from where
>> > it
>> >  is mapped and whether it is enabled.  This allows a device to configure
>> >  a memory region once, then hand it off to its parent bus to map it
>> > according
>> >  to the bus configuration.
>> >
>> >  Hierarchical registration also allows a device to compose a region out
>> > of
>> >  a number of sub-regions with different properties; for example some may
>> > be
>> >  RAM while others may be MMIO.
>
>> >  +void memory_region_set_log(MemoryRegion *mr, bool log, unsigned
>> > client);
>> >  +/* Enable memory coalescing for the region.  MMIO ->write callbacks
>> > may be
>> >  + * delayed until a non-coalesced MMIO is issued.
>> >  + */
>> >  +void memory_region_set_coalescing(MemoryRegion *mr);
>> >  +/* Enable memory coalescing for a sub-range of the region.  MMIO
>> > ->write
>> >  + * callbacks may be delayed until a non-coalesced MMIO is issued.
>> >  + */
>> >  +void memory_region_add_coalescing(MemoryRegion *mr,
>> >  +                                  target_phys_addr_t offset,
>> >  +                                  target_phys_addr_t size);
>> >  +/* Disable MMIO coalescing for the region. */
>> >  +void memory_region_clear_coalescing(MemoryRegion *mr);
>>
>> Perhaps the interface could be more generic, like
>> +void memory_region_set_property(MemoryRegion *mr, unsigned flags);
>> +void memory_region_clear_property(MemoryRegion *mr, unsigned flags);
>>
>
> Coalescing is a complex property, not just a boolean attribute.  We probably
> will have a number of boolean attributes later, though.

But what is the difference between adding coalescing to an area and
setting the bit property 'coalescing' to an area? At least what you
propose now is not so complex that it couldn't be handled as a single
bit.

>> >  + * conflicts are resolved by having a higher @priority hide a lower
>> > @priority.
>> >  + * Subregions without priority are taken as @priority 0.
>> >  + */
>> >  +void memory_region_add_subregion_overlap(MemoryRegion *mr,
>> >  +                                         target_phys_addr_t offset,
>> >  +                                         MemoryRegion *subregion,
>> >  +                                         unsigned priority);
>> >  +/* Remove a subregion. */
>> >  +void memory_region_del_subregion(MemoryRegion *mr,
>> >  +                                 MemoryRegion *subregion);
>>
>> What would the subregions be used for?
>
> Subregions describe the flow of data through the memory bus.  We'd have a
> subregion for the PCI bus, with its own subregions for various BARs, with
> some having subregions for dispatching different MMIO types within the BAR.
>
> This allows, for example, the PCI layer to move a BAR without the PCI device
> knowing anything about it.

But why can't a first class region be used for that?

Re: [Qemu-devel] [RFC] live snapshot, live merge, live block migration

[Dor Laor]

On 05/20/2011 03:19 PM, Stefan Hajnoczi wrote:

I'm interested in what the API for snapshots would look like.
Specifically how does user software do the following:
1. Create a snapshot
2. Delete a snapshot
3. List snapshots
4. Access data from a snapshot


There are plenty of options there:
 - Run a (unrelated) VM and hotplug the snapshot as additional disk
 - Use v2v (libguestfs)
 - Boot the VM w/ RO
 - Plenty more


5. Restore a VM from a snapshot
6. Get the dirty blocks list (for incremental backup)


It might be needed for additional proposes like efficient delta sync 
across sites or any other storage operation (dedup, etc)




We've discussed image format-level approaches but I think the scope of
the API should cover several levels at which snapshots are
implemented:
1. Image format - image file snapshot (Jes, Jagane)
2. Host file system - ext4 and btrfs snapshots
3. Storage system - LVM or SAN volume snapshots

It will be hard to take advantage of more efficient host file system
or storage system snapshots if they are not designed in now.


I agree but it can also be a chicken and the egg problem.
Actually 1/2/3/5 are already working today regardless of live snapshots.


Is anyone familiar enough with the libvirt storage APIs to draft an
extension that adds snapshot support?  I will take a stab at it if no
one else want to try it.


I added libvirt-list and Ayal Baron from vdsm.
What you're asking is even beyond snapshots, it's the whole management 
of VM images. Doing the above operations is simple but for enterprise 
virtualization solution you'll need to lock the NFS/SAN images, handle 
failures of VM/SAN/Mgmt, keep the snapshots info in mgmt DB, etc.


Today it is managed by a combination of rhev-m/vdsm and libvirt.
I agree it would have been nice to get such common single entry point 
interface.




Stefan

[Qemu-devel] [Bug 618533] Re: OpenSolaris guest fails to see the Solaris partitions of a physical disk in qemu-kvm-9999 (GIT)

[Brad Hards]

Did this change get submitted? Is it still an issue?

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

Title:
  OpenSolaris guest fails to see the Solaris partitions of a physical
  disk in qemu-kvm- (GIT)

Status in QEMU:
  New

Bug description:
  # qemu-kvm --version
  QEMU emulator version 0.13.50 (qemu-kvm-devel), Copyright (c) 2003-2008 
Fabrice Bellard

  The following disk is presented to guest as IDE disk with /dev/sdd as
  path.

  # fdisk -l /dev/sdd

  Disk /dev/sdd: 750.2 GB, 750156374016 bytes
  255 heads, 63 sectors/track, 91201 cylinders, total 1465149168 sectors
  Units = sectors of 1 * 512 = 512 bytes
  Sector size (logical/physical): 512 bytes / 512 bytes
  I/O size (minimum/optimal): 512 bytes / 512 bytes
  Disk identifier: 0x7f3a03aa

 Device Boot  Start End  Blocks   Id  System
  /dev/sdd1  63 7887914 3943926   1b  Hidden W95 FAT32
  /dev/sdd2 7887915   980736119   486424102+  83  Linux
  /dev/sdd3   *   980736120  108315049451207187+  bf  Solaris
  /dev/sdd4  1083150495  1465144064   1909967855  Extended
  /dev/sdd5  1083150558  110774600912297726   83  Linux
  /dev/sdd6  1107746073  1465144064   1786989967  HPFS/NTFS

  The prtvtoc output is attached from both VirtualBox and Qemu-KVM. As
  can be seen in the screenshots, a valid Solaris partition table (which
  is inside the /dev/sdd3, marked 'bf' in Linux fdisk output) is not
  seen in Qemu but seen in Virtualbox.

  This makes the guest unbootable in Qemu because the root FS is not
  found but its bootable in Virtualbox.

Re: [Qemu-devel] [RFC] Memory API

[Jan Kiszka]

On 2011-05-22 10:41, Gleb Natapov wrote:
>> The chipset knows about the priorities.  How to communicate them to
>> the core?
>>
>> - at runtime, with hierarchical dispatch of ->read() and ->write():
>> slow, and doesn't work at all for RAM.
>> - using registration order: fragile
>> - using priorities
>>
> - by resolving overlapping and registering flattened list with the core.
>   (See example above).

[Registration would happens with the help of the core against the next
higher layer.]

To do this, you need to
 - open-code the resolution logic at every level (very bad idea)
 - provide library services to obtain a flattened representation

Please try to specify such an API without any parameters that are
priority-like.

Jan



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 02/11] Delete unused tb_invalidate_page_range

[Blue Swirl]

On Tue, May 17, 2011 at 5:42 PM, Richard Henderson  wrote:
> On 05/14/2011 12:37 PM, Blue Swirl wrote:
>> tb_invalidate_page_range() was intended to be used to invalidate an area of 
>> a TB
>> which the guest explicitly flushes from i-cache. However, QEMU detects writes
>> to code areas where TBs have been generated, so this has never been useful.
>>
>> Delete the function, adjust callers.
>>
>> Signed-off-by: Blue Swirl 
>
> Reviewed-by: Richard Henderson 

Thanks for the review, applied.

[Qemu-devel] [PATCH v2 1/2] Generalize -machine command line option

[Jan Kiszka]

From: Jan Kiszka 

-machine somehow suggests that it selects the machine, but it doesn't.
Fix that before this command is set in stone.

Actually, -machine should supersede -M and allow to introduce arbitrary
per-machine options to the command line. That will change the internal
realization again, but we will be able to keep the user interface
stable.

CC: Anthony PERARD 
Signed-off-by: Jan Kiszka 
---

Changes in v2:
 - fix regression of -M my factoring out machine_parse and using it for
   both old and new command.

 qemu-config.c   |5 +
 qemu-options.hx |   20 +++-
 vl.c|   39 ---
 3 files changed, 44 insertions(+), 20 deletions(-)

diff --git a/qemu-config.c b/qemu-config.c
index 5d7ffa2..01751b4 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -452,9 +452,14 @@ QemuOptsList qemu_option_rom_opts = {
 
 static QemuOptsList qemu_machine_opts = {
 .name = "machine",
+.implied_opt_name = "type",
 .head = QTAILQ_HEAD_INITIALIZER(qemu_machine_opts.head),
 .desc = {
 {
+.name = "type",
+.type = QEMU_OPT_STRING,
+.help = "emulated machine"
+}, {
 .name = "accel",
 .type = QEMU_OPT_STRING,
 .help = "accelerator list",
diff --git a/qemu-options.hx b/qemu-options.hx
index 82e085a..0dbc028 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2033,13 +2033,23 @@ if KVM support is enabled when compiling.
 ETEXI
 
 DEF("machine", HAS_ARG, QEMU_OPTION_machine, \
-"-machine accel=accel1[:accel2]use an accelerator (kvm,xen,tcg), 
default is tcg\n", QEMU_ARCH_ALL)
+"-machine [type=]name[,prop[=value][,...]]\n"
+"selects emulated machine (-machine ? for list)\n"
+"property accel=accel1[:accel2[:...]] selects 
accelerator\n"
+"supported accelerators are kvm, xen, tcg (default: 
tcg)\n",
+QEMU_ARCH_ALL)
 STEXI
-@item -machine accel=@var{accels}
+@item -machine [type=]@var{name}[,prop=@var{value}[,...]]
 @findex -machine
-This is use to enable an accelerator, in kvm,xen,tcg.
-By default, it use only tcg. If there a more than one accelerator
-specified, the next one is used if the first don't work.
+Select the emulated machine by @var{name}. Use @code{-machine ?} to list
+available machines. Supported machine properties are:
+@table @option
+@item accel=@var{accels1}[:@var{accels2}[:...]]
+This is used to enable an accelerator. Depending on the target architecture,
+kvm, xen, or tcg can be available. By default, tcg is used. If there is more
+than one accelerator specified, the next one is used if the previous one fails
+to initialize.
+@end table
 ETEXI
 
 DEF("xen-domid", HAS_ARG, QEMU_OPTION_xen_domid,
diff --git a/vl.c b/vl.c
index b362871..a346c1e 100644
--- a/vl.c
+++ b/vl.c
@@ -1891,6 +1891,27 @@ static int debugcon_parse(const char *devname)
 return 0;
 }
 
+static QEMUMachine *machine_parse(const char *name)
+{
+QEMUMachine *m, *machine = NULL;
+
+if (name) {
+machine = find_machine(name);
+}
+if (machine) {
+return machine;
+}
+printf("Supported machines are:\n");
+for (m = first_machine; m != NULL; m = m->next) {
+if (m->alias) {
+printf("%-10s %s (alias of %s)\n", m->alias, m->desc, m->name);
+}
+printf("%-10s %s%s\n", m->name, m->desc,
+   m->is_default ? " (default)" : "");
+}
+exit(!name || *name != '?');
+}
+
 static int tcg_init(void)
 {
 return 0;
@@ -2144,20 +2165,7 @@ int main(int argc, char **argv, char **envp)
 }
 switch(popt->index) {
 case QEMU_OPTION_M:
-machine = find_machine(optarg);
-if (!machine) {
-QEMUMachine *m;
-printf("Supported machines are:\n");
-for(m = first_machine; m != NULL; m = m->next) {
-if (m->alias)
-printf("%-10s %s (alias of %s)\n",
-   m->alias, m->desc, m->name);
-printf("%-10s %s%s\n",
-   m->name, m->desc,
-   m->is_default ? " (default)" : "");
-}
-exit(*optarg != '?');
-}
+machine = machine_parse(optarg);
 break;
 case QEMU_OPTION_cpu:
 /* hw initialization will check this */
@@ -2675,11 +2683,12 @@ int main(int argc, char **argv, char **envp)
 case QEMU_OPTION_machine:
 olist = qemu_find_opts("machine");
 qemu_opts_reset(olist);
-opts = qemu_opts_parse(olist, optarg, 0);
+opts = qemu_opts_parse(olist, optarg, 1);
 if (!opts) {
 fprintf(stderr, "parse error: %s\n", optarg);
 exit(1);
   

[Qemu-devel] [PATCH 1/2] cpu-exec: prepare for user and softmmu split

[Blue Swirl]

There is little in common with user and softmmu versions of cpu_resume_signal(),
split them.

Fix coding style for the user emulator part.

Signed-off-by: Blue Swirl 
---
 cpu-exec.c |  303 ++--
 1 files changed, 172 insertions(+), 131 deletions(-)

diff --git a/cpu-exec.c b/cpu-exec.c
index 16a223e..f197ff9 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -64,21 +64,31 @@ void cpu_loop_exit(void)
 /* exit the current TB from a signal handler. The host registers are
restored in a state compatible with the CPU emulator
  */
+#if defined(CONFIG_SOFTMMU)
+void cpu_resume_from_signal(CPUState *env1, void *puc)
+{
+env = env1;
+
+/* XXX: restore cpu registers saved in host registers */
+
+env->exception_index = -1;
+longjmp(env->jmp_env, 1);
+}
+
+#else
+
 void cpu_resume_from_signal(CPUState *env1, void *puc)
 {
-#if !defined(CONFIG_SOFTMMU)
 #ifdef __linux__
 struct ucontext *uc = puc;
 #elif defined(__OpenBSD__)
 struct sigcontext *uc = puc;
 #endif
-#endif

 env = env1;

 /* XXX: restore cpu registers saved in host registers */

-#if !defined(CONFIG_SOFTMMU)
 if (puc) {
 /* XXX: use siglongjmp ? */
 #ifdef __linux__
@@ -91,10 +101,10 @@ void cpu_resume_from_signal(CPUState *env1, void *puc)
 sigprocmask(SIG_SETMASK, &uc->sc_mask, NULL);
 #endif
 }
-#endif
 env->exception_index = -1;
 longjmp(env->jmp_env, 1);
 }
+#endif

 /* Execute the code without caching the generated code. An interpreter
could be used if available. */
@@ -751,9 +761,11 @@ void cpu_x86_frstor(CPUX86State *s, target_ulong
ptr, int data32)
 #if !defined(CONFIG_SOFTMMU)

 #if defined(TARGET_I386)
-#define EXCEPTION_ACTION raise_exception_err(env->exception_index,
env->error_code)
+#define EXCEPTION_ACTION\
+raise_exception_err(env->exception_index, env->error_code)
 #else
-#define EXCEPTION_ACTION cpu_loop_exit()
+#define EXCEPTION_ACTION\
+cpu_loop_exit()
 #endif

 /* 'pc' is the host PC at which the exception was raised. 'address' is
@@ -767,8 +779,9 @@ static inline int handle_cpu_signal(unsigned long
pc, unsigned long address,
 TranslationBlock *tb;
 int ret;

-if (cpu_single_env)
+if (cpu_single_env) {
 env = cpu_single_env; /* XXX: find a correct solution for
multithread */
+}
 #if defined(DEBUG_SIGNAL)
 qemu_printf("qemu: SIGSEGV pc=0x%08lx address=%08lx w=%d oldset=0x%08lx\n",
 pc, address, is_write, *(unsigned long *)old_set);
@@ -780,10 +793,12 @@ static inline int handle_cpu_signal(unsigned
long pc, unsigned long address,

 /* see if it is an MMU fault */
 ret = cpu_handle_mmu_fault(env, address, is_write, MMU_USER_IDX, 0);
-if (ret < 0)
+if (ret < 0) {
 return 0; /* not an MMU fault */
-if (ret == 0)
+}
+if (ret == 0) {
 return 1; /* the MMU fault was handled without causing real
CPU fault */
+}
 /* now we have a real cpu fault */
 tb = tb_find_pc(pc);
 if (tb) {
@@ -804,43 +819,43 @@ static inline int handle_cpu_signal(unsigned
long pc, unsigned long address,
 #if defined(__i386__)

 #if defined(__APPLE__)
-# include 
-
-# define EIP_sig(context)  (*((unsigned long*)&(context)->uc_mcontext->ss.eip))
-# define TRAP_sig(context)((context)->uc_mcontext->es.trapno)
-# define ERROR_sig(context)   ((context)->uc_mcontext->es.err)
-# define MASK_sig(context)((context)->uc_sigmask)
-#elif defined (__NetBSD__)
-# include 
-
-# define EIP_sig(context) ((context)->uc_mcontext.__gregs[_REG_EIP])
-# define TRAP_sig(context)((context)->uc_mcontext.__gregs[_REG_TRAPNO])
-# define ERROR_sig(context)   ((context)->uc_mcontext.__gregs[_REG_ERR])
-# define MASK_sig(context)((context)->uc_sigmask)
-#elif defined (__FreeBSD__) || defined(__DragonFly__)
-# include 
-
-# define EIP_sig(context)  (*((unsigned long*)&(context)->uc_mcontext.mc_eip))
-# define TRAP_sig(context)((context)->uc_mcontext.mc_trapno)
-# define ERROR_sig(context)   ((context)->uc_mcontext.mc_err)
-# define MASK_sig(context)((context)->uc_sigmask)
+#include 
+
+#define EIP_sig(context)  (*((unsigned long *)&(context)->uc_mcontext->ss.eip))
+#define TRAP_sig(context)((context)->uc_mcontext->es.trapno)
+#define ERROR_sig(context)   ((context)->uc_mcontext->es.err)
+#define MASK_sig(context)((context)->uc_sigmask)
+#elif defined(__NetBSD__)
+#include 
+
+#define EIP_sig(context) ((context)->uc_mcontext.__gregs[_REG_EIP])
+#define TRAP_sig(context)((context)->uc_mcontext.__gregs[_REG_TRAPNO])
+#define ERROR_sig(context)   ((context)->uc_mcontext.__gregs[_REG_ERR])
+#define MASK_sig(context)((context)->uc_sigmask)
+#elif defined(__FreeBSD__) || defined(__DragonFly__)
+#include 
+
+#define EIP_sig(context)  (*((unsigned long *)&(context)->uc_mcontext.mc_eip))
+#define TRAP_sig(context)((context)->uc_mcontext.mc_trapno)
+

[Qemu-devel] [PATCH 6/9] cpu-exec: unify do_interrupt call

[Blue Swirl]

Now that all targets use common function signature for do_interrupt(),
there is no
need for the #ifdeffery anymore.

Signed-off-by: Blue Swirl 
---
 cpu-exec.c |   26 --
 1 files changed, 0 insertions(+), 26 deletions(-)

diff --git a/cpu-exec.c b/cpu-exec.c
index 4aa75fd..dd4f5f5 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -272,33 +272,7 @@ int cpu_exec(CPUState *env1)
 ret = env->exception_index;
 break;
 #else
-#if defined(TARGET_I386)
-do_interrupt(env);
-#elif defined(TARGET_PPC)
-do_interrupt(env);
-#elif defined(TARGET_LM32)
-do_interrupt(env);
-#elif defined(TARGET_MICROBLAZE)
-do_interrupt(env);
-#elif defined(TARGET_MIPS)
-do_interrupt(env);
-#elif defined(TARGET_SPARC)
-do_interrupt(env);
-#elif defined(TARGET_ARM)
 do_interrupt(env);
-#elif defined(TARGET_UNICORE32)
-do_interrupt(env);
-#elif defined(TARGET_SH4)
-   do_interrupt(env);
-#elif defined(TARGET_ALPHA)
-do_interrupt(env);
-#elif defined(TARGET_CRIS)
-do_interrupt(env);
-#elif defined(TARGET_M68K)
-do_interrupt(env);
-#elif defined(TARGET_S390X)
-do_interrupt(env);
-#endif
 env->exception_index = -1;
 #endif
 }
-- 
1.6.2.4
From ce91ff4b4e8ee9fd7b4bd8e66424a2ea65dbad5b Mon Sep 17 00:00:00 2001
Message-Id: 
In-Reply-To: 
References: 
From: Blue Swirl 
Date: Sat, 21 May 2011 08:17:08 +
Subject: [PATCH 6/9] cpu-exec: unify do_interrupt call

Now that all targets use common function signature for do_interrupt(), there is no
need for the #ifdeffery anymore.

Signed-off-by: Blue Swirl 
---
 cpu-exec.c |   26 --
 1 files changed, 0 insertions(+), 26 deletions(-)

diff --git a/cpu-exec.c b/cpu-exec.c
index 4aa75fd..dd4f5f5 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -272,33 +272,7 @@ int cpu_exec(CPUState *env1)
 ret = env->exception_index;
 break;
 #else
-#if defined(TARGET_I386)
-do_interrupt(env);
-#elif defined(TARGET_PPC)
-do_interrupt(env);
-#elif defined(TARGET_LM32)
-do_interrupt(env);
-#elif defined(TARGET_MICROBLAZE)
-do_interrupt(env);
-#elif defined(TARGET_MIPS)
-do_interrupt(env);
-#elif defined(TARGET_SPARC)
-do_interrupt(env);
-#elif defined(TARGET_ARM)
 do_interrupt(env);
-#elif defined(TARGET_UNICORE32)
-do_interrupt(env);
-#elif defined(TARGET_SH4)
-		do_interrupt(env);
-#elif defined(TARGET_ALPHA)
-do_interrupt(env);
-#elif defined(TARGET_CRIS)
-do_interrupt(env);
-#elif defined(TARGET_M68K)
-do_interrupt(env);
-#elif defined(TARGET_S390X)
-do_interrupt(env);
-#endif
 env->exception_index = -1;
 #endif
 }
-- 
1.7.2.5

[Qemu-devel] [PATCH/RFC 0/9] AREG0 series

[Blue Swirl]

This is still RFC. All except 9 seem to be OK, but 9 needs review.
M68k patch (5) is untested.

Patch 9 seems to be OK for x86_64 and Sparc64, but i386 crashes for
some reason I can't see. Other architectures are untested, especially
ia64 could be wrong.

Blue Swirl (9):
  cpu_loop_exit: avoid using AREG0
  sparc: fix coding style of the area to be moved
  sparc: move do_interrupt to helper.c
  x86: use caller supplied CPUState for interrupt related stuff
  m68k: use caller supplied CPUState for interrupt related stuff
  cpu-exec: unify do_interrupt call
  exec.h: fix coding style of the area to be moved
  Move cpu_has_work and cpu_pc_from_tb to cpu.h
  cpu-exec.c: avoid AREG0 use

 Makefile.target   |2 +-
 cpu-exec.c|  147 ++--
 exec-all.h|3 +-
 hw/alpha_palcode.c|2 +-
 target-alpha/cpu.h|   12 ++
 target-alpha/exec.h   |   10 --
 target-alpha/op_helper.c  |4 +-
 target-arm/cpu.h  |   13 ++
 target-arm/exec.h |   12 --
 target-arm/op_helper.c|6 +-
 target-cris/cpu.h |   11 ++
 target-cris/exec.h|   11 --
 target-cris/op_helper.c   |4 +-
 target-i386/cpu.h |   57 +
 target-i386/exec.h|   56 -
 target-i386/op_helper.c   |  105 ++---
 target-lm32/cpu.h |   13 ++
 target-lm32/exec.h|   11 --
 target-lm32/op_helper.c   |6 +-
 target-m68k/cpu.h |   15 +++-
 target-m68k/exec.h|   11 --
 target-m68k/op_helper.c   |   35 +-
 target-microblaze/cpu.h   |   13 ++
 target-microblaze/exec.h  |   11 --
 target-microblaze/op_helper.c |4 +-
 target-mips/cpu.h |   24 
 target-mips/exec.h|   22 
 target-mips/op_helper.c   |4 +-
 target-ppc/cpu.h  |   12 ++
 target-ppc/exec.h |   11 --
 target-ppc/op_helper.c|2 +-
 target-sh4/cpu.h  |   13 ++
 target-sh4/exec.h |   11 --
 target-sh4/op_helper.c|   10 +-
 target-sparc/cpu.h|   18 +++
 target-sparc/exec.h   |   16 ---
 target-sparc/helper.c |  242 +++
 target-sparc/op_helper.c  |  253 ++--
 target-unicore32/cpu.h|6 +
 target-unicore32/exec.h   |6 -
 target-unicore32/op_helper.c  |2 +-
 tcg/arm/tcg-target.c  |4 +-
 tcg/hppa/tcg-target.c |4 +-
 tcg/i386/tcg-target.c |4 +-
 tcg/ia64/tcg-target.c |5 +-
 tcg/mips/tcg-target.c |4 +-
 tcg/ppc/tcg-target.c  |3 +-
 tcg/ppc64/tcg-target.c|3 +-
 tcg/s390/tcg-target.c |5 +-
 tcg/sparc/tcg-target.c|4 +-
 tcg/tcg.h |7 +-
 user-exec.c   |   11 +-
 xen-mapcache-stub.c   |1 +
 53 files changed, 676 insertions(+), 605 deletions(-)

[Qemu-devel] [PATCH 1/9] cpu_loop_exit: avoid using AREG0

[Blue Swirl]

Make cpu_loop_exit() take a parameter for CPUState instead of relying
on global env.

Signed-off-by: Blue Swirl 
---
 cpu-exec.c|   16 
 exec-all.h|2 +-
 hw/alpha_palcode.c|2 +-
 target-alpha/op_helper.c  |4 ++--
 target-arm/op_helper.c|6 +++---
 target-cris/op_helper.c   |4 ++--
 target-i386/op_helper.c   |   16 
 target-lm32/op_helper.c   |6 +++---
 target-m68k/op_helper.c   |6 +++---
 target-microblaze/op_helper.c |4 ++--
 target-mips/op_helper.c   |4 ++--
 target-ppc/op_helper.c|2 +-
 target-sh4/op_helper.c|   10 +-
 target-sparc/op_helper.c  |6 +++---
 target-unicore32/op_helper.c  |2 +-
 user-exec.c   |   11 ++-
 16 files changed, 51 insertions(+), 50 deletions(-)

diff --git a/cpu-exec.c b/cpu-exec.c
index 6ddd8dd..caf0dbe 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -38,10 +38,10 @@ int qemu_cpu_has_work(CPUState *env)
 return cpu_has_work(env);
 }

-void cpu_loop_exit(void)
+void cpu_loop_exit(CPUState *env1)
 {
-env->current_tb = NULL;
-longjmp(env->jmp_env, 1);
+env1->current_tb = NULL;
+longjmp(env1->jmp_env, 1);
 }

 /* exit the current TB from a signal handler. The host registers are
@@ -328,7 +328,7 @@ int cpu_exec(CPUState *env1)
 if (interrupt_request & CPU_INTERRUPT_DEBUG) {
 env->interrupt_request &= ~CPU_INTERRUPT_DEBUG;
 env->exception_index = EXCP_DEBUG;
-cpu_loop_exit();
+cpu_loop_exit(env);
 }
 #if defined(TARGET_ARM) || defined(TARGET_SPARC) || defined(TARGET_MIPS) || \
 defined(TARGET_PPC) || defined(TARGET_ALPHA) || defined(TARGET_CRIS) || \
@@ -337,7 +337,7 @@ int cpu_exec(CPUState *env1)
 env->interrupt_request &= ~CPU_INTERRUPT_HALT;
 env->halted = 1;
 env->exception_index = EXCP_HLT;
-cpu_loop_exit();
+cpu_loop_exit(env);
 }
 #endif
 #if defined(TARGET_I386)
@@ -345,7 +345,7 @@ int cpu_exec(CPUState *env1)
 svm_check_intercept(SVM_EXIT_INIT);
 do_cpu_init(env);
 env->exception_index = EXCP_HALTED;
-cpu_loop_exit();
+cpu_loop_exit(env);
 } else if (interrupt_request & CPU_INTERRUPT_SIPI) {
 do_cpu_sipi(env);
 } else if (env->hflags2 & HF2_GIF_MASK) {
@@ -538,7 +538,7 @@ int cpu_exec(CPUState *env1)
 if (unlikely(env->exit_request)) {
 env->exit_request = 0;
 env->exception_index = EXCP_INTERRUPT;
-cpu_loop_exit();
+cpu_loop_exit(env);
 }
 #if defined(DEBUG_DISAS) || defined(CONFIG_DEBUG_EXEC)
 if (qemu_loglevel_mask(CPU_LOG_TB_CPU)) {
@@ -621,7 +621,7 @@ int cpu_exec(CPUState *env1)
 }
 env->exception_index = EXCP_INTERRUPT;
 next_tb = 0;
-cpu_loop_exit();
+cpu_loop_exit(env);
 }
 }
 }
diff --git a/exec-all.h b/exec-all.h
index cf3a704..1862428 100644
--- a/exec-all.h
+++ b/exec-all.h
@@ -91,7 +91,7 @@ TranslationBlock *tb_gen_code(CPUState *env,
   target_ulong pc, target_ulong cs_base, int flags,
   int cflags);
 void cpu_exec_init(CPUState *env);
-void QEMU_NORETURN cpu_loop_exit(void);
+void QEMU_NORETURN cpu_loop_exit(CPUState *env1);
 int page_unprotect(target_ulong address, unsigned long pc, void *puc);
 void tb_invalidate_phys_page_range(tb_page_addr_t start, tb_page_addr_t end,
int is_cpu_write_access);
diff --git a/hw/alpha_palcode.c b/hw/alpha_palcode.c
index 033b542..49719f6 100644
--- a/hw/alpha_palcode.c
+++ b/hw/alpha_palcode.c
@@ -96,7 +96,7 @@ static void do_swappal (CPUState *env, uint64_t palid)
 env->pal_handler = NULL;
 env->ipr[IPR_PAL_BASE] = palid;
 env->pc = env->ipr[IPR_PAL_BASE];
-cpu_loop_exit();
+cpu_loop_exit(env);
 }
 }

diff --git a/target-alpha/op_helper.c b/target-alpha/op_helper.c
index 4ccb10b..d50977c 100644
--- a/target-alpha/op_helper.c
+++ b/target-alpha/op_helper.c
@@ -29,7 +29,7 @@ void QEMU_NORETURN helper_excp (int excp, int error)
 {
 env->exception_index = excp;
 env->error_code = error;
-cpu_loop_exit();
+cpu_loop_exit(env);
 }

 uint64_t helper_load_pcc (void)
@@ -1378,7 +1378,7 @@ void tlb_fill (target_ulong addr, int is_write,
int mmu_idx, void *re

[Qemu-devel] [PATCH 2/9] sparc: fix coding style of the area to be moved

[Blue Swirl]

Before the next patch, fix coding style of the areas affected.

Signed-off-by: Blue Swirl 
---
 target-sparc/op_helper.c |   31 +--
 1 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/target-sparc/op_helper.c b/target-sparc/op_helper.c
index 491a9f3..6a4220a 100644
--- a/target-sparc/op_helper.c
+++ b/target-sparc/op_helper.c
@@ -4137,25 +4137,26 @@ trap_state* cpu_tsptr(CPUState* env)
 void do_interrupt(CPUState *env)
 {
 int intno = env->exception_index;
-trap_state* tsptr;
+trap_state *tsptr;

 #ifdef DEBUG_PCALL
 if (qemu_loglevel_mask(CPU_LOG_INT)) {
 static int count;
 const char *name;

-if (intno < 0 || intno >= 0x180)
+if (intno < 0 || intno >= 0x180) {
 name = "Unknown";
-else if (intno >= 0x100)
+} else if (intno >= 0x100) {
 name = "Trap Instruction";
-else if (intno >= 0xc0)
+} else if (intno >= 0xc0) {
 name = "Window Fill";
-else if (intno >= 0x80)
+} else if (intno >= 0x80) {
 name = "Window Spill";
-else {
+} else {
 name = excp_names[intno];
-if (!name)
+if (!name) {
 name = "Unknown";
+}
 }

 qemu_log("%6d: %s (v=%04x) pc=%016" PRIx64 " npc=%016" PRIx64
@@ -4171,7 +4172,7 @@ void do_interrupt(CPUState *env)

 qemu_log("   code=");
 ptr = (uint8_t *)env->pc;
-for(i = 0; i < 16; i++) {
+for (i = 0; i < 16; i++) {
 qemu_log(" %02x", ldub(ptr + i));
 }
 qemu_log("\n");
@@ -4191,8 +4192,9 @@ void do_interrupt(CPUState *env)
 env->tl++;
 } else {
 env->pstate |= PS_RED;
-if (env->tl < env->maxtl)
+if (env->tl < env->maxtl) {
 env->tl++;
+}
 }
 tsptr = cpu_tsptr(env);

@@ -4277,14 +4279,15 @@ void do_interrupt(CPUState *env)
 static int count;
 const char *name;

-if (intno < 0 || intno >= 0x100)
+if (intno < 0 || intno >= 0x100) {
 name = "Unknown";
-else if (intno >= 0x80)
+} else if (intno >= 0x80) {
 name = "Trap Instruction";
-else {
+} else {
 name = excp_names[intno];
-if (!name)
+if (!name) {
 name = "Unknown";
+}
 }

 qemu_log("%6d: %s (v=%02x) pc=%08x npc=%08x SP=%08x\n",
@@ -4299,7 +4302,7 @@ void do_interrupt(CPUState *env)

 qemu_log("   code=");
 ptr = (uint8_t *)env->pc;
-for(i = 0; i < 16; i++) {
+for (i = 0; i < 16; i++) {
 qemu_log(" %02x", ldub(ptr + i));
 }
 qemu_log("\n");
-- 
1.6.2.4
From 64fd5d62397b501a85fdc2d451ff825935b6e51b Mon Sep 17 00:00:00 2001
Message-Id: <64fd5d62397b501a85fdc2d451ff825935b6e51b.1306062376.git.blauwir...@gmail.com>
In-Reply-To: 
References: 
From: Blue Swirl 
Date: Sat, 21 May 2011 11:41:50 +
Subject: [PATCH 2/9] sparc: fix coding style of the area to be moved

Before the next patch, fix coding style of the areas affected.

Signed-off-by: Blue Swirl 
---
 target-sparc/op_helper.c |   31 +--
 1 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/target-sparc/op_helper.c b/target-sparc/op_helper.c
index 491a9f3..6a4220a 100644
--- a/target-sparc/op_helper.c
+++ b/target-sparc/op_helper.c
@@ -4137,25 +4137,26 @@ trap_state* cpu_tsptr(CPUState* env)
 void do_interrupt(CPUState *env)
 {
 int intno = env->exception_index;
-trap_state* tsptr;
+trap_state *tsptr;
 
 #ifdef DEBUG_PCALL
 if (qemu_loglevel_mask(CPU_LOG_INT)) {
 static int count;
 const char *name;
 
-if (intno < 0 || intno >= 0x180)
+if (intno < 0 || intno >= 0x180) {
 name = "Unknown";
-else if (intno >= 0x100)
+} else if (intno >= 0x100) {
 name = "Trap Instruction";
-else if (intno >= 0xc0)
+} else if (intno >= 0xc0) {
 name = "Window Fill";
-else if (intno >= 0x80)
+} else if (intno >= 0x80) {
 name = "Window Spill";
-else {
+} else {
 name = excp_names[intno];
-if (!name)
+if (!name) {
 name = "Unknown";
+}
 }
 
 qemu_log("%6d: %s (v=%04x) pc=%016" PRIx64 " npc=%016" PRIx64
@@ -4171,7 +4172,7 @@ void do_interrupt(CPUState *env)
 
 qemu_log("   code=");
 ptr = (uint8_t *)env->pc;
-for(i = 0; i < 16; i++) {
+for (i = 0; i < 16; i++) {
 qemu_log(" %02x", ldub(ptr + i));
 }
 qemu_log("\n");
@@ -4191,8 +4192,9 @@ void do_interrupt(CPUState *env)
 env->tl++;
 } else {
 env->pstate |= PS_RED;
-if (env->tl < env-

[Qemu-devel] [PATCH 4/9] x86: use caller supplied CPUState for interrupt related stuff

[Blue Swirl]

Several x86 specific functions are called from cpu-exec.c with the
assumption that global env register is valid. This will be changed
later, so make the functions use caller supplied CPUState parameter.

It would be cleaner to move the functions to helper.c, but there are
quite a lot of dependencies between do_interrupt() and other functions.

Add helpers for svm_check_intercept() and cpu_cc_compute_all() instead
of calling the helper (which uses global env, AREG0) directly.

Signed-off-by: Blue Swirl 
---
 cpu-exec.c  |   41 --
 target-i386/cpu.h   |   10 +
 target-i386/exec.h  |   10 -
 target-i386/op_helper.c |   89 ++
 4 files changed, 106 insertions(+), 44 deletions(-)

diff --git a/cpu-exec.c b/cpu-exec.c
index caf0dbe..1adfc29 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -267,26 +267,13 @@ int cpu_exec(CPUState *env1)
which will be handled outside the cpu execution
loop */
 #if defined(TARGET_I386)
-do_interrupt_user(env->exception_index,
-  env->exception_is_int,
-  env->error_code,
-  env->exception_next_eip);
-/* successfully delivered */
-env->old_exception = -1;
+do_interrupt(env);
 #endif
 ret = env->exception_index;
 break;
 #else
 #if defined(TARGET_I386)
-/* simulate a real cpu exception. On i386, it can
-   trigger new exceptions, but we do not handle
-   double or triple faults yet. */
-do_interrupt(env->exception_index,
- env->exception_is_int,
- env->error_code,
- env->exception_next_eip, 0);
-/* successfully delivered */
-env->old_exception = -1;
+do_interrupt(env);
 #elif defined(TARGET_PPC)
 do_interrupt(env);
 #elif defined(TARGET_LM32)
@@ -342,7 +329,7 @@ int cpu_exec(CPUState *env1)
 #endif
 #if defined(TARGET_I386)
 if (interrupt_request & CPU_INTERRUPT_INIT) {
-svm_check_intercept(SVM_EXIT_INIT);
+svm_check_intercept(env, SVM_EXIT_INIT);
 do_cpu_init(env);
 env->exception_index = EXCP_HALTED;
 cpu_loop_exit(env);
@@ -351,19 +338,19 @@ int cpu_exec(CPUState *env1)
 } else if (env->hflags2 & HF2_GIF_MASK) {
 if ((interrupt_request & CPU_INTERRUPT_SMI) &&
 !(env->hflags & HF_SMM_MASK)) {
-svm_check_intercept(SVM_EXIT_SMI);
+svm_check_intercept(env, SVM_EXIT_SMI);
 env->interrupt_request &= ~CPU_INTERRUPT_SMI;
-do_smm_enter();
+do_smm_enter(env);
 next_tb = 0;
 } else if ((interrupt_request & CPU_INTERRUPT_NMI) &&
!(env->hflags2 & HF2_NMI_MASK)) {
 env->interrupt_request &= ~CPU_INTERRUPT_NMI;
 env->hflags2 |= HF2_NMI_MASK;
-do_interrupt(EXCP02_NMI, 0, 0, 0, 1);
+do_interrupt_x86_hardirq(env, EXCP02_NMI, 1);
 next_tb = 0;
} else if (interrupt_request & CPU_INTERRUPT_MCE) {
 env->interrupt_request &= ~CPU_INTERRUPT_MCE;
-do_interrupt(EXCP12_MCHK, 0, 0, 0, 0);
+do_interrupt_x86_hardirq(env, EXCP12_MCHK, 0);
 next_tb = 0;
 } else if ((interrupt_request & CPU_INTERRUPT_HARD) &&
(((env->hflags2 & HF2_VINTR_MASK) &&
@@ -372,7 +359,7 @@ int cpu_exec(CPUState *env1)
  (env->eflags & IF_MASK &&
   !(env->hflags &
HF_INHIBIT_IRQ_MASK) {
 int intno;
-svm_check_intercept(SVM_EXIT_INTR);
+svm_check_intercept(env, SVM_EXIT_INTR);
 env->interrupt_request &=
~(CPU_INTERRUPT_HARD | CPU_INTERRUPT_VIRQ);
 intno = cpu_get_pic_interrupt(env);
 qemu_log_mask(CPU_LOG_TB_IN_ASM,
"Servicing hardware INT=0x%02x\n", intno);
@@ -381,7 +368,7 @@ int cpu_exec(CPUState *env1)
 env = cpu_single_env;
 #define env cpu_single_env
 #endif
-

[Qemu-devel] [PATCH 5/9] m68k: use caller supplied CPUState for interrupt related stuff

[Blue Swirl]

Pass CPUState to do_interrupt(). This is needed by later patches.

It would be cleaner to move the function to helper.c, but there are
a few dependencies between do_interrupt() and other functions.

Signed-off-by: Blue Swirl 
---
 cpu-exec.c  |4 ++--
 target-m68k/cpu.h   |3 ++-
 target-m68k/op_helper.c |   29 ++---
 3 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/cpu-exec.c b/cpu-exec.c
index 1adfc29..4aa75fd 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -295,7 +295,7 @@ int cpu_exec(CPUState *env1)
 #elif defined(TARGET_CRIS)
 do_interrupt(env);
 #elif defined(TARGET_M68K)
-do_interrupt(0);
+do_interrupt(env);
 #elif defined(TARGET_S390X)
 do_interrupt(env);
 #endif
@@ -503,7 +503,7 @@ int cpu_exec(CPUState *env1)
provide/save the vector when the interrupt is
first signalled.  */
 env->exception_index = env->pending_vector;
-do_interrupt(1);
+do_interrupt_m68k_hardirq(env);
 next_tb = 0;
 }
 #elif defined(TARGET_S390X) && !defined(CONFIG_USER_ONLY)
diff --git a/target-m68k/cpu.h b/target-m68k/cpu.h
index b025b66..4d8ba28 100644
--- a/target-m68k/cpu.h
+++ b/target-m68k/cpu.h
@@ -119,7 +119,8 @@ void m68k_tcg_init(void);
 CPUM68KState *cpu_m68k_init(const char *cpu_model);
 int cpu_m68k_exec(CPUM68KState *s);
 void cpu_m68k_close(CPUM68KState *s);
-void do_interrupt(int is_hw);
+void do_interrupt(CPUState *env1);
+void do_interrupt_m68k_hardirq(CPUState *env1);
 /* you can call this signal handler from your SIGBUS and SIGSEGV
signal handlers to inform the virtual CPU of exceptions. non zero
is returned if the signal was handled by the virtual CPU.  */
diff --git a/target-m68k/op_helper.c b/target-m68k/op_helper.c
index 084a182..237fc4c 100644
--- a/target-m68k/op_helper.c
+++ b/target-m68k/op_helper.c
@@ -21,9 +21,13 @@

 #if defined(CONFIG_USER_ONLY)

-void do_interrupt(int is_hw)
+void do_interrupt(CPUState *env1)
+{
+env1->exception_index = -1;
+}
+
+void do_interrupt_m68k_hardirq(CPUState *env1)
 {
-env->exception_index = -1;
 }

 #else
@@ -90,7 +94,7 @@ static void do_rte(void)
 env->aregs[7] = sp + 8;
 }

-void do_interrupt(int is_hw)
+static void do_interrupt_all(int is_hw)
 {
 uint32_t sp;
 uint32_t fmt;
@@ -155,6 +159,25 @@ void do_interrupt(int is_hw)
 env->pc = ldl_kernel(env->vbr + vector);
 }

+void do_interrupt(CPUState *env1)
+{
+CPUState *saved_env;
+
+saved_env = env;
+env = env1;
+do_interrupt_all(0);
+env = saved_env;
+}
+
+void do_interrupt_m68k_hardirq(CPUState *env1)
+{
+CPUState *saved_env;
+
+saved_env = env;
+env = env1;
+do_interrupt_all(1);
+env = saved_env;
+}
 #endif

 static void raise_exception(int tt)
-- 
1.6.2.4
From b14ad2f68c593655a372eb08be163a538ca4ca93 Mon Sep 17 00:00:00 2001
Message-Id: 
In-Reply-To: 
References: 
From: Blue Swirl 
Date: Sat, 21 May 2011 07:55:24 +
Subject: [PATCH 5/9] m68k: use caller supplied CPUState for interrupt related stuff

Pass CPUState to do_interrupt(). This is needed by later patches.

It would be cleaner to move the function to helper.c, but there are
a few dependencies between do_interrupt() and other functions.

Signed-off-by: Blue Swirl 
---
 cpu-exec.c  |4 ++--
 target-m68k/cpu.h   |3 ++-
 target-m68k/op_helper.c |   29 ++---
 3 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/cpu-exec.c b/cpu-exec.c
index 1adfc29..4aa75fd 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -295,7 +295,7 @@ int cpu_exec(CPUState *env1)
 #elif defined(TARGET_CRIS)
 do_interrupt(env);
 #elif defined(TARGET_M68K)
-do_interrupt(0);
+do_interrupt(env);
 #elif defined(TARGET_S390X)
 do_interrupt(env);
 #endif
@@ -503,7 +503,7 @@ int cpu_exec(CPUState *env1)
provide/save the vector when the interrupt is
first signalled.  */
 env->exception_index = env->pending_vector;
-do_interrupt(1);
+do_interrupt_m68k_hardirq(env);
 next_tb = 0;
 }
 #elif defined(TARGET_S390X) && !defined(CONFIG_USER_ONLY)
diff --git a/target-m68k/cpu.h b/target-m68k/cpu.h
index b025b66..4d8ba28 100644
--- a/target-m68k/cpu.h
+++ b/target-m68k/cpu.h
@@ -119,7 +119,8 @@ void m68k_tcg_init(void);
 CPUM68KState *cpu_m68k_init(const char *cpu_model);
 int cpu_m68k_exec(CPUM68KState *s);
 void cpu_m68k_close(CPUM68KState *s);
-void do_interrupt(int is_hw);
+void do_interrupt(CPUState *env1);
+void do_interrupt_m68k_hardirq(CPUState *env1);
 /* you can call this signal handler from your SIGBUS and SIGSEGV
si

[Qemu-devel] [PATCH 3/9] sparc: move do_interrupt to helper.c

[Blue Swirl]

do_interrupt() was mixing CPUState pointer passed from caller
and global env (AREG0).

Fix by moving the function to helper.c. Introduce a helper for calling
change_pstate() safely from outside of execution context.

Signed-off-by: Blue Swirl 
---
 target-sparc/cpu.h   |4 +
 target-sparc/exec.h  |2 -
 target-sparc/helper.c|  242 
 target-sparc/op_helper.c |  250 ++
 4 files changed, 256 insertions(+), 242 deletions(-)

diff --git a/target-sparc/cpu.h b/target-sparc/cpu.h
index 320530e..eff006c 100644
--- a/target-sparc/cpu.h
+++ b/target-sparc/cpu.h
@@ -474,6 +474,7 @@ target_ulong cpu_get_ccr(CPUState *env1);
 void cpu_put_ccr(CPUState *env1, target_ulong val);
 target_ulong cpu_get_cwp64(CPUState *env1);
 void cpu_put_cwp64(CPUState *env1, int cwp);
+void cpu_change_pstate(CPUState *env1, uint32_t new_pstate);
 #endif
 int cpu_cwp_inc(CPUState *env1, int cwp);
 int cpu_cwp_dec(CPUState *env1, int cwp);
@@ -656,4 +657,7 @@ static inline void cpu_get_tb_cpu_state(CPUState
*env, target_ulong *pc,
 #endif
 }

+/* helper.c */
+void do_interrupt(CPUState *env);
+
 #endif
diff --git a/target-sparc/exec.h b/target-sparc/exec.h
index f5c221e..becdaf5 100644
--- a/target-sparc/exec.h
+++ b/target-sparc/exec.h
@@ -13,8 +13,6 @@ register struct CPUSPARCState *env asm(AREG0);
 #endif /* !defined(CONFIG_USER_ONLY) */

 /* op_helper.c */
-void do_interrupt(CPUState *env);
-
 static inline int cpu_has_work(CPUState *env1)
 {
 return (env1->interrupt_request & CPU_INTERRUPT_HARD) &&
diff --git a/target-sparc/helper.c b/target-sparc/helper.c
index b2d4d70..14d8efa 100644
--- a/target-sparc/helper.c
+++ b/target-sparc/helper.c
@@ -730,6 +730,248 @@ target_phys_addr_t
cpu_get_phys_page_debug(CPUState *env, target_ulong addr)
 }
 #endif

+#ifdef TARGET_SPARC64
+#ifdef DEBUG_PCALL
+static const char * const excp_names[0x80] = {
+[TT_TFAULT] = "Instruction Access Fault",
+[TT_TMISS] = "Instruction Access MMU Miss",
+[TT_CODE_ACCESS] = "Instruction Access Error",
+[TT_ILL_INSN] = "Illegal Instruction",
+[TT_PRIV_INSN] = "Privileged Instruction",
+[TT_NFPU_INSN] = "FPU Disabled",
+[TT_FP_EXCP] = "FPU Exception",
+[TT_TOVF] = "Tag Overflow",
+[TT_CLRWIN] = "Clean Windows",
+[TT_DIV_ZERO] = "Division By Zero",
+[TT_DFAULT] = "Data Access Fault",
+[TT_DMISS] = "Data Access MMU Miss",
+[TT_DATA_ACCESS] = "Data Access Error",
+[TT_DPROT] = "Data Protection Error",
+[TT_UNALIGNED] = "Unaligned Memory Access",
+[TT_PRIV_ACT] = "Privileged Action",
+[TT_EXTINT | 0x1] = "External Interrupt 1",
+[TT_EXTINT | 0x2] = "External Interrupt 2",
+[TT_EXTINT | 0x3] = "External Interrupt 3",
+[TT_EXTINT | 0x4] = "External Interrupt 4",
+[TT_EXTINT | 0x5] = "External Interrupt 5",
+[TT_EXTINT | 0x6] = "External Interrupt 6",
+[TT_EXTINT | 0x7] = "External Interrupt 7",
+[TT_EXTINT | 0x8] = "External Interrupt 8",
+[TT_EXTINT | 0x9] = "External Interrupt 9",
+[TT_EXTINT | 0xa] = "External Interrupt 10",
+[TT_EXTINT | 0xb] = "External Interrupt 11",
+[TT_EXTINT | 0xc] = "External Interrupt 12",
+[TT_EXTINT | 0xd] = "External Interrupt 13",
+[TT_EXTINT | 0xe] = "External Interrupt 14",
+[TT_EXTINT | 0xf] = "External Interrupt 15",
+};
+#endif
+
+void do_interrupt(CPUState *env)
+{
+int intno = env->exception_index;
+trap_state *tsptr;
+
+#ifdef DEBUG_PCALL
+if (qemu_loglevel_mask(CPU_LOG_INT)) {
+static int count;
+const char *name;
+
+if (intno < 0 || intno >= 0x180) {
+name = "Unknown";
+} else if (intno >= 0x100) {
+name = "Trap Instruction";
+} else if (intno >= 0xc0) {
+name = "Window Fill";
+} else if (intno >= 0x80) {
+name = "Window Spill";
+} else {
+name = excp_names[intno];
+if (!name) {
+name = "Unknown";
+}
+}
+
+qemu_log("%6d: %s (v=%04x) pc=%016" PRIx64 " npc=%016" PRIx64
+" SP=%016" PRIx64 "\n",
+count, name, intno,
+env->pc,
+env->npc, env->regwptr[6]);
+log_cpu_state(env, 0);
+#if 0
+{
+int i;
+uint8_t *ptr;
+
+qemu_log("   code=");
+ptr = (uint8_t *)env->pc;
+for (i = 0; i < 16; i++) {
+qemu_log(" %02x", ldub(ptr + i));
+}
+qemu_log("\n");
+}
+#endif
+count++;
+}
+#endif
+#if !defined(CONFIG_USER_ONLY)
+if (env->tl >= env->maxtl) {
+cpu_abort(env, "Trap 0x%04x while trap level (%d) >= MAXTL (%d),"
+  " Error state", env->exception_index, env->tl, env->maxtl);
+return;
+}
+#endif
+if (env->tl < env->maxtl - 1) {
+env->tl++;
+} else {
+env->pstate |= PS_RED;
+

[Qemu-devel] [PATCH 7/9] exec.h: fix coding style of the area to be moved

[Blue Swirl]

Before the next patch, fix coding style of the areas affected.

Signed-off-by: Blue Swirl 
---
 target-alpha/exec.h  |2 +-
 target-arm/exec.h|4 ++--
 target-cris/exec.h   |2 +-
 target-m68k/exec.h   |2 +-
 target-microblaze/exec.h |2 +-
 target-ppc/exec.h|2 +-
 target-sh4/exec.h|2 +-
 7 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/target-alpha/exec.h b/target-alpha/exec.h
index 6ae96d1..26c3a3a 100644
--- a/target-alpha/exec.h
+++ b/target-alpha/exec.h
@@ -39,7 +39,7 @@ register struct CPUAlphaState *env asm(AREG0);

 static inline int cpu_has_work(CPUState *env)
 {
-return (env->interrupt_request & CPU_INTERRUPT_HARD);
+return env->interrupt_request & CPU_INTERRUPT_HARD;
 }

 static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
diff --git a/target-arm/exec.h b/target-arm/exec.h
index 44e1b55..ef497d8 100644
--- a/target-arm/exec.h
+++ b/target-arm/exec.h
@@ -28,8 +28,8 @@ register struct CPUARMState *env asm(AREG0);

 static inline int cpu_has_work(CPUState *env)
 {
-return (env->interrupt_request &
-(CPU_INTERRUPT_FIQ | CPU_INTERRUPT_HARD | CPU_INTERRUPT_EXITTB));
+return env->interrupt_request &
+(CPU_INTERRUPT_FIQ | CPU_INTERRUPT_HARD | CPU_INTERRUPT_EXITTB);
 }

 #if !defined(CONFIG_USER_ONLY)
diff --git a/target-cris/exec.h b/target-cris/exec.h
index 2d5d297..491f17c 100644
--- a/target-cris/exec.h
+++ b/target-cris/exec.h
@@ -30,7 +30,7 @@ register struct CPUCRISState *env asm(AREG0);

 static inline int cpu_has_work(CPUState *env)
 {
-return (env->interrupt_request & (CPU_INTERRUPT_HARD | CPU_INTERRUPT_NMI));
+return env->interrupt_request & (CPU_INTERRUPT_HARD | CPU_INTERRUPT_NMI);
 }

 static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
diff --git a/target-m68k/exec.h b/target-m68k/exec.h
index 91daa6b..f63e849 100644
--- a/target-m68k/exec.h
+++ b/target-m68k/exec.h
@@ -30,7 +30,7 @@ register struct CPUM68KState *env asm(AREG0);

 static inline int cpu_has_work(CPUState *env)
 {
-return (env->interrupt_request & (CPU_INTERRUPT_HARD));
+return env->interrupt_request & (CPU_INTERRUPT_HARD);
 }

 static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
diff --git a/target-microblaze/exec.h b/target-microblaze/exec.h
index 1efff30..af102d6 100644
--- a/target-microblaze/exec.h
+++ b/target-microblaze/exec.h
@@ -29,7 +29,7 @@ register struct CPUMBState *env asm(AREG0);

 static inline int cpu_has_work(CPUState *env)
 {
-return (env->interrupt_request & (CPU_INTERRUPT_HARD | CPU_INTERRUPT_NMI));
+return env->interrupt_request & (CPU_INTERRUPT_HARD | CPU_INTERRUPT_NMI);
 }

 static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
diff --git a/target-ppc/exec.h b/target-ppc/exec.h
index f87847a..fbc0a06 100644
--- a/target-ppc/exec.h
+++ b/target-ppc/exec.h
@@ -34,7 +34,7 @@ register struct CPUPPCState *env asm(AREG0);

 static inline int cpu_has_work(CPUState *env)
 {
-return (msr_ee && (env->interrupt_request & CPU_INTERRUPT_HARD));
+return msr_ee && (env->interrupt_request & CPU_INTERRUPT_HARD);
 }


diff --git a/target-sh4/exec.h b/target-sh4/exec.h
index 9f1c1f6..e52838c 100644
--- a/target-sh4/exec.h
+++ b/target-sh4/exec.h
@@ -29,7 +29,7 @@ register struct CPUSH4State *env asm(AREG0);

 static inline int cpu_has_work(CPUState *env)
 {
-return (env->interrupt_request & CPU_INTERRUPT_HARD);
+return env->interrupt_request & CPU_INTERRUPT_HARD;
 }

 #ifndef CONFIG_USER_ONLY
-- 
1.6.2.4
From 5a9a4488f189c97314ad81061499691214034493 Mon Sep 17 00:00:00 2001
Message-Id: <5a9a4488f189c97314ad81061499691214034493.1306062376.git.blauwir...@gmail.com>
In-Reply-To: 
References: 
From: Blue Swirl 
Date: Sat, 21 May 2011 12:16:05 +
Subject: [PATCH 7/9] exec.h: fix coding style of the area to be moved

Before the next patch, fix coding style of the areas affected.

Signed-off-by: Blue Swirl 
---
 target-alpha/exec.h  |2 +-
 target-arm/exec.h|4 ++--
 target-cris/exec.h   |2 +-
 target-m68k/exec.h   |2 +-
 target-microblaze/exec.h |2 +-
 target-ppc/exec.h|2 +-
 target-sh4/exec.h|2 +-
 7 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/target-alpha/exec.h b/target-alpha/exec.h
index 6ae96d1..26c3a3a 100644
--- a/target-alpha/exec.h
+++ b/target-alpha/exec.h
@@ -39,7 +39,7 @@ register struct CPUAlphaState *env asm(AREG0);
 
 static inline int cpu_has_work(CPUState *env)
 {
-return (env->interrupt_request & CPU_INTERRUPT_HARD);
+return env->interrupt_request & CPU_INTERRUPT_HARD;
 }
 
 static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
diff --git a/target-arm/exec.h b/target-arm/exec.h
index 44e1b55..ef497d8 100644
--- a/target-arm/exec.h
+++ b/target-arm/exec.h
@@ -28,8 +28,8 @@ register struct CPUARMState *env asm(AREG0);
 
 static inline int cpu_has_work(CPUState *env)
 {
-return (env-

[Qemu-devel] [PATCH 9/9] cpu-exec.c: avoid AREG0 use

[Blue Swirl]

Make functions take a parameter for CPUState instead of relying
on global env. Pass CPUState pointer to TCG prologue, which moves
it to AREG0.

Revert the hacks to avoid AREG0 use on Sparc hosts.

Move cpu_has_work() and cpu_pc_from_tb() from exec.h to cpu.h.

Compile the file without HELPER_CFLAGS.

Signed-off-by: Blue Swirl 
---
 Makefile.target|2 +-
 cpu-exec.c |   74 +---
 tcg/arm/tcg-target.c   |4 ++-
 tcg/hppa/tcg-target.c  |4 ++-
 tcg/i386/tcg-target.c  |4 ++-
 tcg/ia64/tcg-target.c  |5 ++-
 tcg/mips/tcg-target.c  |4 +-
 tcg/ppc/tcg-target.c   |3 +-
 tcg/ppc64/tcg-target.c |3 +-
 tcg/s390/tcg-target.c  |5 ++-
 tcg/sparc/tcg-target.c |4 +-
 tcg/tcg.h  |7 ++--
 12 files changed, 48 insertions(+), 71 deletions(-)

diff --git a/Makefile.target b/Makefile.target
index 602d50d..b5fda44 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -94,7 +94,7 @@ tcg/tcg.o: cpu.h

 # HELPER_CFLAGS is used for all the code compiled with static register
 # variables
-%_helper.o cpu-exec.o user-exec.o: QEMU_CFLAGS += $(HELPER_CFLAGS)
+%_helper.o user-exec.o: QEMU_CFLAGS += $(HELPER_CFLAGS)

 # Note: this is a workaround. The real fix is to avoid compiling
 # cpu_signal_handler() in user-exec.c.
diff --git a/cpu-exec.c b/cpu-exec.c
index dd4f5f5..f5f9cc3 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -17,18 +17,12 @@
  * License along with this library; if not, see .
  */
 #include "config.h"
-#include "exec.h"
+#include "cpu.h"
 #include "disas.h"
 #include "tcg.h"
 #include "kvm.h"
 #include "qemu-barrier.h"

-#if defined(__sparc__) && !defined(CONFIG_SOLARIS)
-// Work around ugly bugs in glibc that mangle global register contents
-#undef env
-#define env cpu_single_env
-#endif
-
 int tb_invalidated_flag;

 //#define CONFIG_DEBUG_EXEC
@@ -38,20 +32,18 @@ int qemu_cpu_has_work(CPUState *env)
 return cpu_has_work(env);
 }

-void cpu_loop_exit(CPUState *env1)
+void cpu_loop_exit(CPUState *env)
 {
-env1->current_tb = NULL;
-longjmp(env1->jmp_env, 1);
+env->current_tb = NULL;
+longjmp(env->jmp_env, 1);
 }

 /* exit the current TB from a signal handler. The host registers are
restored in a state compatible with the CPU emulator
  */
 #if defined(CONFIG_SOFTMMU)
-void cpu_resume_from_signal(CPUState *env1, void *puc)
+void cpu_resume_from_signal(CPUState *env, void *puc)
 {
-env = env1;
-
 /* XXX: restore cpu registers saved in host registers */

 env->exception_index = -1;
@@ -61,7 +53,8 @@ void cpu_resume_from_signal(CPUState *env1, void *puc)

 /* Execute the code without caching the generated code. An interpreter
could be used if available. */
-static void cpu_exec_nocache(int max_cycles, TranslationBlock *orig_tb)
+static void cpu_exec_nocache(CPUState *env, int max_cycles,
+ TranslationBlock *orig_tb)
 {
 unsigned long next_tb;
 TranslationBlock *tb;
@@ -75,7 +68,7 @@ static void cpu_exec_nocache(int max_cycles,
TranslationBlock *orig_tb)
  max_cycles);
 env->current_tb = tb;
 /* execute the generated code */
-next_tb = tcg_qemu_tb_exec(tb->tc_ptr);
+next_tb = tcg_qemu_tb_exec(env, tb->tc_ptr);
 env->current_tb = NULL;

 if ((next_tb & 3) == 2) {
@@ -87,7 +80,8 @@ static void cpu_exec_nocache(int max_cycles,
TranslationBlock *orig_tb)
 tb_free(tb);
 }

-static TranslationBlock *tb_find_slow(target_ulong pc,
+static TranslationBlock *tb_find_slow(CPUState *env,
+  target_ulong pc,
   target_ulong cs_base,
   uint64_t flags)
 {
@@ -141,7 +135,7 @@ static TranslationBlock *tb_find_slow(target_ulong pc,
 return tb;
 }

-static inline TranslationBlock *tb_find_fast(void)
+static inline TranslationBlock *tb_find_fast(CPUState *env)
 {
 TranslationBlock *tb;
 target_ulong cs_base, pc;
@@ -154,7 +148,7 @@ static inline TranslationBlock *tb_find_fast(void)
 tb = env->tb_jmp_cache[tb_jmp_cache_hash_func(pc)];
 if (unlikely(!tb || tb->pc != pc || tb->cs_base != cs_base ||
  tb->flags != flags)) {
-tb = tb_find_slow(pc, cs_base, flags);
+tb = tb_find_slow(env, pc, cs_base, flags);
 }
 return tb;
 }
@@ -187,31 +181,22 @@ static void cpu_handle_debug_exception(CPUState *env)

 volatile sig_atomic_t exit_request;

-int cpu_exec(CPUState *env1)
+int cpu_exec(CPUState *env)
 {
-volatile host_reg_t saved_env_reg;
 int ret, interrupt_request;
 TranslationBlock *tb;
 uint8_t *tc_ptr;
 unsigned long next_tb;

-if (env1->halted) {
-if (!cpu_has_work(env1)) {
+if (env->halted) {
+if (!cpu_has_work(env)) {
 return EXCP_HALTED;
 }

-env1->halted = 0;
+env->halted = 0;
 }

-cpu_single_env = env1;
-
-/* the access to env be

[Qemu-devel] [PATCH 8/9] Move cpu_has_work and cpu_pc_from_tb to cpu.h

[Blue Swirl]

Move functions cpu_has_work() and cpu_pc_from_tb() from exec.h to cpu.h. This is
needed by later patches.

Signed-off-by: Blue Swirl 
---
 exec-all.h   |1 +
 target-alpha/cpu.h   |   12 +++
 target-alpha/exec.h  |   10 -
 target-arm/cpu.h |   13 
 target-arm/exec.h|   12 ---
 target-cris/cpu.h|   11 ++
 target-cris/exec.h   |   11 --
 target-i386/cpu.h|   47 ++
 target-i386/exec.h   |   46 -
 target-lm32/cpu.h|   13 
 target-lm32/exec.h   |   11 --
 target-m68k/cpu.h|   12 +++
 target-m68k/exec.h   |   11 --
 target-microblaze/cpu.h  |   13 
 target-microblaze/exec.h |   11 --
 target-mips/cpu.h|   24 +++
 target-mips/exec.h   |   22 -
 target-ppc/cpu.h |   12 +++
 target-ppc/exec.h|   11 --
 target-sh4/cpu.h |   13 
 target-sh4/exec.h|   11 --
 target-sparc/cpu.h   |   14 +
 target-sparc/exec.h  |   14 -
 target-unicore32/cpu.h   |6 +
 target-unicore32/exec.h  |6 -
 xen-mapcache-stub.c  |1 +
 26 files changed, 192 insertions(+), 176 deletions(-)

diff --git a/exec-all.h b/exec-all.h
index 1862428..e9533f3 100644
--- a/exec-all.h
+++ b/exec-all.h
@@ -40,6 +40,7 @@ typedef ram_addr_t tb_page_addr_t;
 #define DISAS_UPDATE  2 /* cpu state was modified dynamically */
 #define DISAS_TB_JUMP 3 /* only pc was modified statically */

+struct TranslationBlock;
 typedef struct TranslationBlock TranslationBlock;

 /* XXX: make safe guess about sizes */
diff --git a/target-alpha/cpu.h b/target-alpha/cpu.h
index 686fb4a..359b2b6 100644
--- a/target-alpha/cpu.h
+++ b/target-alpha/cpu.h
@@ -535,4 +535,16 @@ static inline void cpu_set_tls(CPUState *env,
target_ulong newtls)
 }
 #endif

+static inline int cpu_has_work(CPUState *env)
+{
+return env->interrupt_request & CPU_INTERRUPT_HARD;
+}
+
+#include "exec-all.h"
+
+static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
+{
+env->pc = tb->pc;
+}
+
 #endif /* !defined (__CPU_ALPHA_H__) */
diff --git a/target-alpha/exec.h b/target-alpha/exec.h
index 26c3a3a..f508996 100644
--- a/target-alpha/exec.h
+++ b/target-alpha/exec.h
@@ -37,14 +37,4 @@ register struct CPUAlphaState *env asm(AREG0);
 #include "softmmu_exec.h"
 #endif /* !defined(CONFIG_USER_ONLY) */

-static inline int cpu_has_work(CPUState *env)
-{
-return env->interrupt_request & CPU_INTERRUPT_HARD;
-}
-
-static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
-{
-env->pc = tb->pc;
-}
-
 #endif /* !defined (__ALPHA_EXEC_H__) */
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 01f5b57..bb54363 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -512,4 +512,17 @@ static inline void cpu_get_tb_cpu_state(CPUState
*env, target_ulong *pc,
 }
 }

+static inline int cpu_has_work(CPUState *env)
+{
+return env->interrupt_request &
+(CPU_INTERRUPT_FIQ | CPU_INTERRUPT_HARD | CPU_INTERRUPT_EXITTB);
+}
+
+#include "exec-all.h"
+
+static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
+{
+env->regs[15] = tb->pc;
+}
+
 #endif
diff --git a/target-arm/exec.h b/target-arm/exec.h
index ef497d8..68d6fb9 100644
--- a/target-arm/exec.h
+++ b/target-arm/exec.h
@@ -26,20 +26,8 @@ register struct CPUARMState *env asm(AREG0);
 #include "cpu.h"
 #include "exec-all.h"

-static inline int cpu_has_work(CPUState *env)
-{
-return env->interrupt_request &
-(CPU_INTERRUPT_FIQ | CPU_INTERRUPT_HARD | CPU_INTERRUPT_EXITTB);
-}
-
 #if !defined(CONFIG_USER_ONLY)
 #include "softmmu_exec.h"
 #endif

 void raise_exception(int);
-
-static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
-{
-env->regs[15] = tb->pc;
-}
-
diff --git a/target-cris/cpu.h b/target-cris/cpu.h
index 2bc35e4..adbc38b 100644
--- a/target-cris/cpu.h
+++ b/target-cris/cpu.h
@@ -268,4 +268,15 @@ static inline void cpu_get_tb_cpu_state(CPUState
*env, target_ulong *pc,
 #define cpu_list cris_cpu_list
 void cris_cpu_list(FILE *f, fprintf_function cpu_fprintf);

+static inline int cpu_has_work(CPUState *env)
+{
+return env->interrupt_request & (CPU_INTERRUPT_HARD | CPU_INTERRUPT_NMI);
+}
+
+#include "exec-all.h"
+
+static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
+{
+env->pc = tb->pc;
+}
 #endif
diff --git a/target-cris/exec.h b/target-cris/exec.h
index 491f17c..9838159 100644
--- a/target-cris/exec.h
+++ b/target-cris/exec.h
@@ -27,14 +27,3 @@ register struct CPUCRISState *env asm(AREG0);
 #if !defined(CONFIG_USER_ONLY)
 #include "softmmu_exec.h"
 #endif
-
-static inline int cpu_has_work(CPUState *env)
-{
-return env->interrupt_request & (CPU_INTERRUPT_HARD | CPU_INTERRUPT_NMI);
-}
-
-

Re: [Qemu-devel] [RFC] Memory API

[Avi Kivity]

On 05/22/2011 01:53 PM, Jan Kiszka wrote:

On 2011-05-22 10:41, Gleb Natapov wrote:
>>  The chipset knows about the priorities.  How to communicate them to
>>  the core?
>>
>>  - at runtime, with hierarchical dispatch of ->read() and ->write():
>>  slow, and doesn't work at all for RAM.
>>  - using registration order: fragile
>>  - using priorities
>>
>  - by resolving overlapping and registering flattened list with the core.
>(See example above).

[Registration would happens with the help of the core against the next
higher layer.]

To do this, you need to
  - open-code the resolution logic at every level (very bad idea)
  - provide library services to obtain a flattened representation

Please try to specify such an API without any parameters that are
priority-like.


Another way of saying the same thing:  having the chipset code resolve 
conflicts, and having the chipset code assign priorities, are 
equivalent.  But having priorities allows flattening to take place 
without further involvement of the chipset code.


--
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [Qemu-devel] [PATCH 7/9] exec.h: fix coding style of the area to be moved

[Jan Kiszka]

On 2011-05-22 13:18, Blue Swirl wrote:
> Before the next patch, fix coding style of the areas affected.
> 
> Signed-off-by: Blue Swirl 
> ---
>  target-alpha/exec.h  |2 +-
>  target-arm/exec.h|4 ++--
>  target-cris/exec.h   |2 +-
>  target-m68k/exec.h   |2 +-
>  target-microblaze/exec.h |2 +-
>  target-ppc/exec.h|2 +-
>  target-sh4/exec.h|2 +-
>  7 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/target-alpha/exec.h b/target-alpha/exec.h
> index 6ae96d1..26c3a3a 100644
> --- a/target-alpha/exec.h
> +++ b/target-alpha/exec.h
> @@ -39,7 +39,7 @@ register struct CPUAlphaState *env asm(AREG0);
> 
>  static inline int cpu_has_work(CPUState *env)
>  {
> -return (env->interrupt_request & CPU_INTERRUPT_HARD);
> +return env->interrupt_request & CPU_INTERRUPT_HARD;

Maybe this patch or the series is a good change to make cpu_has_work
return a bool?

Jan



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH] Remove unneeded kvm.h from cpu-exec.c

[Jan Kiszka]

From: Jan Kiszka 

This was obsoleted by 6792a57bf1.

Signed-off-by: Jan Kiszka 
---

Noticed while reading your split-up patches.

 cpu-exec.c |1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/cpu-exec.c b/cpu-exec.c
index 7323f3c..ce6288c 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -20,7 +20,6 @@
 #include "exec.h"
 #include "disas.h"
 #include "tcg.h"
-#include "kvm.h"
 #include "qemu-barrier.h"
 
 #if !defined(CONFIG_SOFTMMU)
-- 
1.7.1

Re: [Qemu-devel] [RFC v1] Add declarations for hierarchical memory region API

[Avi Kivity]

On 05/22/2011 12:32 PM, Blue Swirl wrote:

>>  >+void memory_region_add_coalescing(MemoryRegion *mr,
>>  >+  target_phys_addr_t offset,
>>  >+  target_phys_addr_t size);
>>  >+/* Disable MMIO coalescing for the region. */
>>  >+void memory_region_clear_coalescing(MemoryRegion *mr);
>>
>>  Perhaps the interface could be more generic, like
>>  +void memory_region_set_property(MemoryRegion *mr, unsigned flags);
>>  +void memory_region_clear_property(MemoryRegion *mr, unsigned flags);
>>
>
>  Coalescing is a complex property, not just a boolean attribute.  We probably
>  will have a number of boolean attributes later, though.

But what is the difference between adding coalescing to an area and
setting the bit property 'coalescing' to an area? At least what you
propose now is not so complex that it couldn't be handled as a single
bit.


Look at the API - add_coalescing() sets the coalescing property on a 
subrange of the memory region, not the entire region.


(motivation - hw/e1000.c).


>>  >+ * conflicts are resolved by having a higher @priority hide a lower
>>  >  @priority.
>>  >+ * Subregions without priority are taken as @priority 0.
>>  >+ */
>>  >+void memory_region_add_subregion_overlap(MemoryRegion *mr,
>>  >+ target_phys_addr_t offset,
>>  >+ MemoryRegion *subregion,
>>  >+ unsigned priority);
>>  >+/* Remove a subregion. */
>>  >+void memory_region_del_subregion(MemoryRegion *mr,
>>  >+ MemoryRegion *subregion);
>>
>>  What would the subregions be used for?
>
>  Subregions describe the flow of data through the memory bus.  We'd have a
>  subregion for the PCI bus, with its own subregions for various BARs, with
>  some having subregions for dispatching different MMIO types within the BAR.
>
>  This allows, for example, the PCI layer to move a BAR without the PCI device
>  knowing anything about it.

But why can't a first class region be used for that?


Subregions are first-class regions.  In fact all regions are subregions 
except the root.


It's a tree of regions, each level adding an offset, clipping, and 
perhaps other attributes, with the leaves providing actual memory (mmio 
or RAM).


--
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

[Qemu-devel] [PATCH] XBRLE page delta compression for live migration of large memory apps

[Shribman, Aidan]

Subject: [PATCH] XBRLE page delta compression for live migration of large 
memory apps
From: Aidan Shribman 

By using XBRLE (Xor Based Run-Length-Encoding) we can reduce required
bandwidth for transfering of dirty memory pages during live migration
migrate_set_cachesize 
migrate -x 
Qemu host: Ubuntu 10.10
Testing: live migration (w and w/o XBRLE) tested successfully.

Signed-off-by: Benoit Hudzia 
Signed-off-by: Petter Svard 
Signed-off-by: Aidan Shribman 

---

 arch_init.c   |  647 +
 block-migration.c |3 +-
 hmp-commands.hx   |   36 +++-
 hw/hw.h   |3 +-
 migration-exec.c  |6 +-
 migration-fd.c|6 +-
 migration-tcp.c   |6 +-
 migration-unix.c  |6 +-
 migration.c   |   33 +++-
 migration.h   |   23 ++-
 qmp-commands.hx   |   43 +++-
 savevm.c  |   13 +-
 sysemu.h  |3 +-
 13 files changed, 749 insertions(+), 79 deletions(-)

diff --git a/arch_init.c b/arch_init.c
index 4486925..069cd67 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -27,6 +27,7 @@
 #include 
 #include 
 #endif
+#include 
 #include "config.h"
 #include "monitor.h"
 #include "sysemu.h"
@@ -41,6 +42,24 @@
 #include "gdbstub.h"
 #include "hw/smbios.h"

+//#define DEBUG_ARCH_INIT
+#ifdef DEBUG_ARCH_INIT
+#define DPRINTF(fmt, ...) \
+do { fprintf(stdout, "arch_init: " fmt, ## __VA_ARGS__); } while (0)
+#else
+#define DPRINTF(fmt, ...) \
+do { } while (0)
+#endif
+
+//#define DEBUG_ARCH_INIT_CKSUM
+#ifdef DEBUG_ARCH_INIT_CKSUM
+#define PAGE_LOG(addr, pdata, fmt, ...) \
+do { page_log(addr, pdata, fmt, ## __VA_ARGS__); } while (0)
+#else
+#define PAGE_LOG(addr, pdata, fmt, ...) \
+do { } while (0)
+#endif
+
 #ifdef TARGET_SPARC
 int graphic_width = 1024;
 int graphic_height = 768;
@@ -88,6 +107,402 @@ const uint32_t arch_type = QEMU_ARCH;
 #define RAM_SAVE_FLAG_PAGE 0x08
 #define RAM_SAVE_FLAG_EOS  0x10
 #define RAM_SAVE_FLAG_CONTINUE 0x20
+#define RAM_SAVE_FLAG_XBRLE0x40
+
+/***/
+/* Page cache for storing previous pages as basis for XBRLE compression */
+#define CACHE_N_WAY 2 /* 2-way assossiative cache */
+
+typedef struct CacheItem {
+ram_addr_t it_addr;
+unsigned long it_age;
+uint8_t *it_data;
+} CacheItem;
+
+typedef struct CacheBucket {
+CacheItem bkt_item[CACHE_N_WAY];
+} CacheBucket;
+
+static CacheBucket *page_cache;
+static int64_t cache_num_buckets;
+static uint64_t cache_max_item_age;
+static int64_t cache_num_items;
+
+static void cache_init(ssize_t num_buckets);
+static void cache_fini(void);
+static int cache_is_cached(ram_addr_t addr);
+static int cache_get_oldest(CacheBucket *buck);
+static int cache_get_newest(CacheBucket *buck, ram_addr_t addr);
+static void cache_insert(ram_addr_t id, uint8_t *pdata);
+static unsigned long cache_get_cache_pos(ram_addr_t address);
+static CacheItem *cache_item_get(unsigned long pos, int item);
+
+/***/
+/* RAM Migration State */
+typedef struct ArchMigrationState {
+int use_xbrle;
+int64_t xbrle_cache_size;
+} ArchMigrationState;
+
+static ArchMigrationState arch_mig_state;
+
+void arch_set_params(int blk_enable, int shared_base, int use_xbrle,
+int64_t xbrle_cache_size, void *opaque)
+{
+arch_mig_state.use_xbrle = use_xbrle;
+arch_mig_state.xbrle_cache_size = xbrle_cache_size;
+}
+
+/***/
+/* XBRLE (Xor Based Run-Length Encoding) */
+typedef struct XBRLEHeader {
+uint8_t xh_flags;
+uint16_t xh_len;
+uint32_t xh_cksum;
+} XBRLEHeader;
+
+static int rle_encode(uint8_t *src, int slen, uint8_t *dst, int dlen);
+static int rle_decode(uint8_t *src, int slen, uint8_t *dst, int dlen);
+#ifdef DEBUG_ARCH_INIT_CKSUM
+static uint32_t page_cksum(uint8_t *buf);
+static void page_log(ram_addr_t addr, uint8_t *pdata, const char *fmt, ...);
+#endif
+
+/***/
+/* benchmarking */
+typedef struct BenchInfo {
+uint64_t normal_pages;
+uint64_t xbrle_pages;
+uint64_t xbrle_bytes;
+uint64_t xbrle_pages_aborted;
+uint64_t dup_pages;
+uint64_t iterations;
+} BenchInfo;
+
+static BenchInfo bench;
+
+/***/
+/* XBRLE page cache implementation */
+static CacheItem *cache_item_get(unsigned long pos, int item)
+{
+assert(page_cache);
+return &page_cache[pos].bkt_item[item];
+}
+
+#ifdef DEBUG_ARCH_INIT_CKSUM
+static int64_t cache_max_items(void)
+{
+return cache_num_buckets * CACHE_N_WAY;
+}
+#endif /* DEBUG_ARCH_INIT_CKSUM */
+
+static void cache_init(int64_t num_bytes)
+{
+int i;
+
+cache_num_items = 0;
+cache_max_item_age = 0;
+cache_num_buckets = num_bytes / (TARGET_PAGE_SIZE * CACHE_N_WAY);
+assert(cache_num_buckets);
+DPRINTF("Setting cache buckets to %ld\n", cache_num_buc

[Qemu-devel] [PATCH 2/2] Fix spelling in comment (additon -> addition)

[Stefan Weil]

Signed-off-by: Stefan Weil 
---
 tcg/tcg.h |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tcg/tcg.h b/tcg/tcg.h
index cecef63..183a446 100644
--- a/tcg/tcg.h
+++ b/tcg/tcg.h
@@ -129,7 +129,7 @@ typedef tcg_target_ulong TCGArg;
We use plain int by default to avoid this runtime overhead.
Users of tcg_gen_* don't need to know about any of this, and should
treat TCGv as an opaque type.
-   In additon we do typechecking for different types of variables.  TCGv_i32
+   In addition we do typechecking for different types of variables.  TCGv_i32
and TCGv_i64 are 32/64-bit variables respectively.  TCGv and TCGv_ptr
are aliases for target_ulong and host pointer sized values respectively.
  */
-- 
1.7.2.5

[Qemu-devel] [PATCH 1/2] pflash_cfi02: Fix a typo in debug code (TARGET_FMT_pld -> TARGET_FMT_plx)

[Stefan Weil]

Thanks to Tobias Hoffmann  for this patch.

Signed-off-by: Stefan Weil 
---
 hw/pflash_cfi02.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/pflash_cfi02.c b/hw/pflash_cfi02.c
index 8fdafe6..725cd1e 100644
--- a/hw/pflash_cfi02.c
+++ b/hw/pflash_cfi02.c
@@ -188,7 +188,7 @@ static uint32_t pflash_read (pflash_t *pfl, 
target_phys_addr_t offset,
 default:
 goto flash_read;
 }
-DPRINTF("%s: ID " TARGET_FMT_pld " %x\n", __func__, boff, ret);
+DPRINTF("%s: ID " TARGET_FMT_plx " %x\n", __func__, boff, ret);
 break;
 case 0xA0:
 case 0x10:
-- 
1.7.2.5

Re: [Qemu-devel] [RFC v1] Add declarations for hierarchical memory region API

[Blue Swirl]

On Sun, May 22, 2011 at 2:36 PM, Avi Kivity  wrote:
> On 05/22/2011 12:32 PM, Blue Swirl wrote:
>>
>> >>  >    +void memory_region_add_coalescing(MemoryRegion *mr,
>> >>  >    +                                  target_phys_addr_t offset,
>> >>  >    +                                  target_phys_addr_t size);
>> >>  >    +/* Disable MMIO coalescing for the region. */
>> >>  >    +void memory_region_clear_coalescing(MemoryRegion *mr);
>> >>
>> >>  Perhaps the interface could be more generic, like
>> >>  +void memory_region_set_property(MemoryRegion *mr, unsigned flags);
>> >>  +void memory_region_clear_property(MemoryRegion *mr, unsigned flags);
>> >>
>> >
>> >  Coalescing is a complex property, not just a boolean attribute.  We
>> > probably
>> >  will have a number of boolean attributes later, though.
>>
>> But what is the difference between adding coalescing to an area and
>> setting the bit property 'coalescing' to an area? At least what you
>> propose now is not so complex that it couldn't be handled as a single
>> bit.
>
> Look at the API - add_coalescing() sets the coalescing property on a
> subrange of the memory region, not the entire region.

Right, but doesn't the same apply to any other properties, they may
apply to a full range or just a subrange?

> (motivation - hw/e1000.c).
>
>> >>  >    + * conflicts are resolved by having a higher @priority hide a
>> >> lower
>> >>  >  @priority.
>> >>  >    + * Subregions without priority are taken as @priority 0.
>> >>  >    + */
>> >>  >    +void memory_region_add_subregion_overlap(MemoryRegion *mr,
>> >>  >    +                                         target_phys_addr_t
>> >> offset,
>> >>  >    +                                         MemoryRegion
>> >> *subregion,
>> >>  >    +                                         unsigned priority);
>> >>  >    +/* Remove a subregion. */
>> >>  >    +void memory_region_del_subregion(MemoryRegion *mr,
>> >>  >    +                                 MemoryRegion *subregion);
>> >>
>> >>  What would the subregions be used for?
>> >
>> >  Subregions describe the flow of data through the memory bus.  We'd have
>> > a
>> >  subregion for the PCI bus, with its own subregions for various BARs,
>> > with
>> >  some having subregions for dispatching different MMIO types within the
>> > BAR.
>> >
>> >  This allows, for example, the PCI layer to move a BAR without the PCI
>> > device
>> >  knowing anything about it.
>>
>> But why can't a first class region be used for that?
>
> Subregions are first-class regions.  In fact all regions are subregions
> except the root.

Oh, I see now. Maybe the comments should describe this. Or perhaps the
terms should be something like 'bus/bridge/root' and 'region' instead
of 'region' and 'subregion'?

> It's a tree of regions, each level adding an offset, clipping, and perhaps
> other attributes, with the leaves providing actual memory (mmio or RAM).

I thought that there are two classes of regions, like PCI device vs. a
single BAR.

Re: [Qemu-devel] [PATCH] block/rbd: Remove unused local variable

[Stefan Weil]

Am 07.05.2011 22:15, schrieb Stefan Weil:

cppcheck report:
rbd.c:246: style: Variable 'snap' is assigned a value that is never used

Remove snap and the related code.

Cc: Christian Brunner
Cc: Kevin Wolf
Signed-off-by: Stefan Weil
---
  block/rbd.c |4 
  1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/block/rbd.c b/block/rbd.c
index 249a590..5c7d44e 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -524,7 +524,6 @@ static int rbd_open(BlockDriverState *bs, const char 
*filename, int flags)
  RbdHeader1 *header;
  char pool[RBD_MAX_SEG_NAME_SIZE];
  char snap_buf[RBD_MAX_SEG_NAME_SIZE];
-char *snap = NULL;
  char *hbuf = NULL;
  int r;

@@ -533,9 +532,6 @@ static int rbd_open(BlockDriverState *bs, const char 
*filename, int flags)
s->name, sizeof(s->name))<  0) {
  return -EINVAL;
  }
-if (snap_buf[0] != '\0') {
-snap = snap_buf;
-}

  if ((r = rados_initialize(0, NULL))<  0) {
  error_report("error initializing");
   


What about this patch? Can it be applied to the block branch?

Regards,
Stefan W.

Re: [Qemu-devel] [PATCH 21/26] target-xtensa: implement unaligned exception option

[Max Filippov]

> > +static void gen_load_store_alignment(DisasContext *dc, int shift, TCGv_i32 
> > addr)
> > +{
> > +TCGv_i32 tmp = tcg_temp_local_new_i32();
> > +tcg_gen_mov_i32(tmp, addr);
> > +tcg_gen_andi_i32(addr, addr, ~0 << shift);
> > +if (option_enabled(dc, XTENSA_OPTION_UNALIGNED_EXCEPTION)) {
> > +int label = gen_new_label();
> > +tcg_gen_brcond_i32(TCG_COND_EQ, addr, tmp, label);
> > +gen_exception_cause_vaddr(dc, LOAD_STORE_ALIGNMENT_CAUSE, tmp);
> > +gen_set_label(label);
> > +}
> > +tcg_temp_free(tmp);
> > +}
> 
> This is not the correct method for this.  Set ALIGNED_ONLY before
> defining the softmmu_templates.  Define do_unaligned_access to raise
> the exception.  See e.g. target-sparc/op_helper.c.

There are three alignment possibilities for xtensa: no unaligned exception, 
unaligned exception and hardware alignment.
In the first case unaligned access silently goes to aligned address. It looks 
like it cannot be done via do_unaligned_access, can it?
In the third case most unaligned accesses are handled transparently by the 
hardware. But e.g. unaligned access by multiprocessor synchronization 
instructions still cause alignment exception. Do I need to implement a 
different alignment checking mechanism for those unhandled cases?

Thanks.
-- Max

Re: [Qemu-devel] [RFC v1] Add declarations for hierarchical memory region API

[Avi Kivity]

On 05/22/2011 03:06 PM, Blue Swirl wrote:

On Sun, May 22, 2011 at 2:36 PM, Avi Kivity  wrote:
>  On 05/22/2011 12:32 PM, Blue Swirl wrote:
>>
>>  >>>  +void memory_region_add_coalescing(MemoryRegion *mr,
>>  >>>  +  target_phys_addr_t offset,
>>  >>>  +  target_phys_addr_t size);
>>  >>>  +/* Disable MMIO coalescing for the region. */
>>  >>>  +void memory_region_clear_coalescing(MemoryRegion *mr);
>>  >>
>>  >>Perhaps the interface could be more generic, like
>>  >>+void memory_region_set_property(MemoryRegion *mr, unsigned flags);
>>  >>+void memory_region_clear_property(MemoryRegion *mr, unsigned flags);
>>  >>
>>  >
>>  >Coalescing is a complex property, not just a boolean attribute.  We
>>  >  probably
>>  >will have a number of boolean attributes later, though.
>>
>>  But what is the difference between adding coalescing to an area and
>>  setting the bit property 'coalescing' to an area? At least what you
>>  propose now is not so complex that it couldn't be handled as a single
>>  bit.
>
>  Look at the API - add_coalescing() sets the coalescing property on a
>  subrange of the memory region, not the entire region.

Right, but doesn't the same apply to any other properties, they may
apply to a full range or just a subrange?


We'll know when we have more properties.  I expect most will be region-wide.



>
>  Subregions are first-class regions.  In fact all regions are subregions
>  except the root.

Oh, I see now. Maybe the comments should describe this. Or perhaps the
terms should be something like 'bus/bridge/root' and 'region' instead
of 'region' and 'subregion'?


Problem is, memory_region_add_subregion() adds both sub-bridges and leaf 
regions.


It's quite possible that BAR 0 will be a leaf region, and BAR 1 will be 
a sub-bridge.


Can you suggest an alternative naming for the API?



>  It's a tree of regions, each level adding an offset, clipping, and perhaps
>  other attributes, with the leaves providing actual memory (mmio or RAM).

I thought that there are two classes of regions, like PCI device vs. a
single BAR.


It's true in a way, except the mapping is not 1:1.

--
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [Qemu-devel] [PATCH v7 3/4] piix_pci: optimize set irq path

[Stefan Weil]

Am 01.04.2011 13:43, schrieb Isaku Yamahata:

optimize irq routing in piix_pic.c which has been a TODO.
So far piix3 tracks each pirq level and checks whether a given pic pins is
asserted by seeing if each pirq is mapped into the pic pin.
This is independent on irq routing, but data path is on slow path.

Given that irq routing is rarely changed and asserting pic pins is on
data path, the path that asserts pic pins should be optimized and
chainging irq routing should be on slow path.
The new behavior with this patch series is to use bitmap which is 
addressed

by pirq and pic pins with a given irq routing.
When pirq is asserted, the bitmap is set and see if the pic pins is
asserted by checking the bitmaps.
When irq routing is changed, rebuild the bitmap and re-assert pic pins.

test:
- create VM with 4 e1000 nics in different pci slots
(i.e. fn=0 for each e1000)
Thus those e1000's INTA are connected to each PIRQ[A-D].
- run linux as guest and saw each devices triggers interrupt
by seeing /proc/interrupts. And then confirmed that each PIRQ[A-D]
surely asserted interrupts.
Because irq 10 and 11 are shared by 4 e1000's, it only one NIC is 
activated

with ifconfig ethN up/down when counting interrupts.

Cc: Michael S. Tsirkin 
Signed-off-by: Isaku Yamahata 
---
Changes v4 -> v5:
- fix piix_set_irq_pic()

Changes v3 -> v4:
- replace irq_num with pirq or pci_intx

Changes v1 -> v2:
- some minor clean ups
- commit log message
---
hw/piix_pci.c | 101 
+++-

1 files changed, 84 insertions(+), 17 deletions(-)

diff --git a/hw/piix_pci.c b/hw/piix_pci.c
index 35e420c..7ffb821 100644
--- a/hw/piix_pci.c
+++ b/hw/piix_pci.c

...


Hi,

I got a regression report which was obviously caused by this commit:

$ git bisect bad
ab431c283e7055bcd6fb622f212bb29e84a6a134 is the first bad commit
commit ab431c283e7055bcd6fb622f212bb29e84a6a134
Author: Isaku Yamahata 
Date:   Fri Apr 1 20:43:23 2011 +0900

My test scenario:

i386-softmmu/qemu -L pc-bios -cdrom tinycore_3.5.1.iso -m 256 -boot d 
-net nic -net user -net dump


* Boot the default configuration (tinycore, or tinycore debug).
* Wait until X Windows is up.
* Check the size of qemu*.pcap. If it is only a few bytes, the test failed.
  Emulated networking then does not work, Tinycore does not get an IP 
address via DHCP.


The original report used tinycore_3.6.iso and the latest w32 binaries 
from qemu.weilnetz.de
with the same result. It also says that other Linux distributions show 
the same problem

(Fedora-14-i686-Live-Desktop.iso).

Regards
Stefan W.

Re: [Qemu-devel] [RFC] Memory API

[Avi Kivity]

On 05/22/2011 11:59 AM, Gleb Natapov wrote:

>  There is no problem.  If the PCI bus priority is higher than RAM
>  priority, then PCI BARs will override RAM.
>
So if memory region has no subregion that covers part of the region
lower prio region is used? Now the same with the pictures:

-- root
   -- PCI 0x0 - 0x2 (prio 0)
 -- BAR A 0x1000 - 0x1fff
 -- BAR B 0x2 - 0x20fff
   -- RAM 0x0 - 0x1 (prio 1)

In the tree above at address 0x0 PCI has higher priority,


In a previous mail, I asserted that 1 > 0.  Under this assumption, PCI 
has lower priority.



but since
there is no subregion at this range next prio region is used instead
(RAM). Is this correct?


Assuming pci prio = 1 and ram prio = 0, yes.


If yes how core knows that container is
transparent like that (RAM container is not)?


All containers are transparent.  Only RAM and MMIO regions are not.


>  >>   Which one takes precedence is determined by the priorities of the
>  >>   RAM subregion vs. the PCI bus subregion.
>  >>
>  >Yes, and that is why PCI subsystem or platform code can't directly uses
>  >memory API to register PCI memory region/RAM memory region respectively,
>  >because they wouldn't know what priorities to specify. Only chipset code
>  >knows, so the RAM/PCI memory registration should go through chipset
>  >code,
>
>  Correct.  Chipset code creates RAM and PCI regions, and gives the
>  PCI region to the PCI bus.  Devices give BAR subregions to the PCI
>  bus.  The PCI bus does the registration.
OK. What happens if device tries to create subregion outside of PCI
region provided by chipset to PCI bus?


What does real hardware do?

The region API has an offset and size, and clips anything outside that.  
However PCI may be more complicated due to 64-bit BARs, we may need an 
extra API to control clipping.



>
>  >   but even chipset code doesn't know everything. It knows nothing
>  >about cpu local memory regions for instance, so all registrations should
>  >go through system bus in the end. Is this how API suppose to be used?
>
>  Yes.  Every point where a decision is made on how to route memory
>  accesses is a modelled as a container node.
>
Excellent. I would argue that this is exactly the point where an
overlapping can be resolved too :)


It is.

--
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [Qemu-devel] [PATCH v7 3/4] piix_pci: optimize set irq path

[TeLeMan]

I did a patch for it.
http://lists.gnu.org/archive/html/qemu-devel/2011-05/msg01239.html

On Sun, May 22, 2011 at 20:24, Stefan Weil  wrote:
>
> Hi,
>
> I got a regression report which was obviously caused by this commit:
>
> $ git bisect bad
> ab431c283e7055bcd6fb622f212bb29e84a6a134 is the first bad commit
> commit ab431c283e7055bcd6fb622f212bb29e84a6a134
> Author: Isaku Yamahata 
> Date:   Fri Apr 1 20:43:23 2011 +0900
>
> My test scenario:
>
> i386-softmmu/qemu -L pc-bios -cdrom tinycore_3.5.1.iso -m 256 -boot d -net
> nic -net user -net dump
>
> * Boot the default configuration (tinycore, or tinycore debug).
> * Wait until X Windows is up.
> * Check the size of qemu*.pcap. If it is only a few bytes, the test failed.
>  Emulated networking then does not work, Tinycore does not get an IP address
> via DHCP.
>
> The original report used tinycore_3.6.iso and the latest w32 binaries from
> qemu.weilnetz.de
> with the same result. It also says that other Linux distributions show the
> same problem
> (Fedora-14-i686-Live-Desktop.iso).
>
> Regards
> Stefan W.
>
>
>
>

Re: [Qemu-devel] [RFC v1] Add declarations for hierarchical memory region API

[Blue Swirl]

On Sun, May 22, 2011 at 3:18 PM, Avi Kivity  wrote:
> On 05/22/2011 03:06 PM, Blue Swirl wrote:
>>
>> On Sun, May 22, 2011 at 2:36 PM, Avi Kivity  wrote:
>> >  On 05/22/2011 12:32 PM, Blue Swirl wrote:
>> >>
>> >>  >>    >      +void memory_region_add_coalescing(MemoryRegion *mr,
>> >>  >>    >      +                                  target_phys_addr_t
>> >> offset,
>> >>  >>    >      +                                  target_phys_addr_t
>> >> size);
>> >>  >>    >      +/* Disable MMIO coalescing for the region. */
>> >>  >>    >      +void memory_region_clear_coalescing(MemoryRegion *mr);
>> >>  >>
>> >>  >>    Perhaps the interface could be more generic, like
>> >>  >>    +void memory_region_set_property(MemoryRegion *mr, unsigned
>> >> flags);
>> >>  >>    +void memory_region_clear_property(MemoryRegion *mr, unsigned
>> >> flags);
>> >>  >>
>> >>  >
>> >>  >    Coalescing is a complex property, not just a boolean attribute.
>> >>  We
>> >>  >  probably
>> >>  >    will have a number of boolean attributes later, though.
>> >>
>> >>  But what is the difference between adding coalescing to an area and
>> >>  setting the bit property 'coalescing' to an area? At least what you
>> >>  propose now is not so complex that it couldn't be handled as a single
>> >>  bit.
>> >
>> >  Look at the API - add_coalescing() sets the coalescing property on a
>> >  subrange of the memory region, not the entire region.
>>
>> Right, but doesn't the same apply to any other properties, they may
>> apply to a full range or just a subrange?
>
> We'll know when we have more properties.  I expect most will be region-wide.

Since we don't know about those yet, coalescing API could be like you
propose. Later it can be changed to the property API, or leave for
convenience.

>> >
>> >  Subregions are first-class regions.  In fact all regions are subregions
>> >  except the root.
>>
>> Oh, I see now. Maybe the comments should describe this. Or perhaps the
>> terms should be something like 'bus/bridge/root' and 'region' instead
>> of 'region' and 'subregion'?
>
> Problem is, memory_region_add_subregion() adds both sub-bridges and leaf
> regions.
>
> It's quite possible that BAR 0 will be a leaf region, and BAR 1 will be a
> sub-bridge.
>
> Can you suggest an alternative naming for the API?

How about
memory_region_container_init()
memory_region_add()

Re: [Qemu-devel] [RFC v1] Add declarations for hierarchical memory region API

[Avi Kivity]

On 05/22/2011 06:32 PM, Blue Swirl wrote:

>
>  Can you suggest an alternative naming for the API?

How about
memory_region_container_init()
memory_region_add()


I'm neutral.  If someone seconds this, I'll make it so.

--
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [Qemu-devel] [Bug 618533] Re: OpenSolaris guest fails to see the Solaris partitions of a physical disk in qemu-kvm-9999 (GIT)

[Blue Swirl]

On Sun, May 22, 2011 at 1:43 PM, Brad Hards  wrote:
> Did this change get submitted? Is it still an issue?

At least the patch hasn't been applied. I don't remember seeing it in the list.

Re: [Qemu-devel] [RFC] Memory API

[Anthony Liguori]

On 05/22/2011 02:38 AM, Avi Kivity wrote:

On 05/20/2011 06:59 PM, Jan Kiszka wrote:

>
> Jan had mentioned previously about registering a new temporary window.
> I assume the registration always gets highest_priority++, or do you
have
> to explicitly specify that PCI container gets priority=1?

The latter.

And I really prefer to have this explicit over deriving the priority
from the registration order. That's way too fragile/unhandy. If you
decide to replace a region of lower priority later on, you need to
reregister everything at that level.


Exactly.


So priority is equivalent to registration order except it's explicit?

So this is more palatable to me but I do prefer registration order since 
it promises less.


But that said, this is a relatively minor feature IMHO given that I now 
understand the scope so I'm not going to object strongly.


Regards,

Anthony Liguori

Re: [Qemu-devel] [RFC v1] Add declarations for hierarchical memory region API

[Anthony Liguori]

On 05/22/2011 01:38 AM, Avi Kivity wrote:

On 05/20/2011 05:31 PM, Anthony Liguori wrote:

Several alpha system chips MCE when accessed with incorrect sizes.
E.g. only 64-bit accesses are allowed.



But is this a characteristic of devices or is this a characteristic of
the chipset/CPU?


The chipset is modelled by a MemoryRegion too.



At any rate, I'm fairly sure it doesn't belong in the MemoryRegion
structure.



Since it isn't a global property, where does it belong?


The chipset should have an intercept in the dispatch path that enforces 
this (this assumes hierarchical dispatch).


Regards,

Anthony Liguori

Re: [Qemu-devel] [RFC v1] Add declarations for hierarchical memory region API

[Anthony Liguori]

On 05/22/2011 01:39 AM, Avi Kivity wrote:

On 05/20/2011 05:46 PM, Anthony Liguori wrote:

On 05/20/2011 09:40 AM, Richard Henderson wrote:

On 05/20/2011 07:31 AM, Anthony Liguori wrote:

But is this a characteristic of devices or is this a characteristic
of the chipset/CPU?


Chipset.


So if the chipset only allows accesses that are 64-bit, then you'll
want to have hierarchical dispatch filter non 64-bit accesses and
raise an MCE appropriately.

So you don't need anything in MemoryRegion, you need code in the
dispatch path.


MemoryRegion *is* the dispatch path. Only done declaratively so we can
flatten it whenever it changes.


We don't want dispatch to be 100% declarative.  That's what will cause 
the API to get horrendously ugly.


An example is PCI-bus level endianness conversion.  I also believe the 
Sparc IOMMU has an xor engine.


You could add a 'bool swap_endian' and an 'uint32_t xor_mask' in 
MemoryRegion but now you're adding a ton of platform specific knowledge 
to what should be an independent layer.


Regards,

Anthony Liguori

Re: [Qemu-devel] [RFC v1] Add declarations for hierarchical memory region API

[Avi Kivity]

On 05/22/2011 06:46 PM, Anthony Liguori wrote:

MemoryRegion *is* the dispatch path. Only done declaratively so we can
flatten it whenever it changes.



We don't want dispatch to be 100% declarative.  That's what will cause 
the API to get horrendously ugly.


An example is PCI-bus level endianness conversion.  I also believe the 
Sparc IOMMU has an xor engine.


You could add a 'bool swap_endian' and an 'uint32_t xor_mask' in 
MemoryRegion but now you're adding a ton of platform specific 
knowledge to what should be an independent layer.




Currently containers do not use the read/write function pointers.  We 
could make them (or perhaps others) act as transformations on the data 
as it passes.  So it's still declarative (the entire flow is known at 
registration time) but it doesn't embed platform magic.


Byteswap is sufficiently generic to add as a region property, IMO.

btw, wrt iommu emulation, the API finally allows us to determine the 
path between any two devices, so we can apply the right iommu 
transformations.


--
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [Qemu-devel] [RFC v1] Add declarations for hierarchical memory region API

[Avi Kivity]

On 05/22/2011 06:44 PM, Anthony Liguori wrote:




At any rate, I'm fairly sure it doesn't belong in the MemoryRegion
structure.



Since it isn't a global property, where does it belong?



The chipset should have an intercept in the dispatch path that 
enforces this (this assumes hierarchical dispatch).


So instead of region->ops->valid.*, region->ops->intercept()?

btw, that still doesn't require hierarchical dispatch.  If intercepts 
only check if the access is valid, it can still be flattened.


Hierarchical dispatch means that chipset callbacks get to choose which 
subregion callbacks are called, which isn't the case here.  If it were, 
it would be impossible to figure out the kvm slot layout.


--
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

[Qemu-devel] [Bug 723871] Re: qemu-kvm-0.14.0 Aborts with -vga qxl

[Boris Derzhavets]

apt-get install gobject-introspection libgirepository1.0-dev

./configure --prefix=/usr --build=x86_64-linux-gnu \
 --mandir=/usr/share/man --infodir=/usr/share/info --datadir=/usr/share \
 --sysconfdir=/etc --localstatedir=/var --libexecdir=/usr/bin \
 --with-python --disable-static --enable-introspection

. . . . . .

Spice-Gtk 0.6
==

prefix:   /usr
c compiler:   gcc -std=gnu99
c++ compiler: g++

Coroutine:ucontext
Audio:pulse
Target:   
SASL support: yes
Gtk:  2.0

Now type 'make' to build spice-gtk

make
checkinstall  --install=no
dpkg -i   --force-overwrite  ./spice-gtk_0.6-1_amd64.deb

Libraries and binaries been overwritten are exactly the same as before.
Spice session via Virt-Manager ( video "qxl")  is manageable via keyboard
before logging in (F14,Natty VM's). Mouse pointer appears and works stable
during all the time after login.

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

Title:
  qemu-kvm-0.14.0 Aborts with -vga qxl

Status in QEMU:
  Invalid
Status in “libvirt” package in Ubuntu:
  Triaged
Status in “qemu-kvm” package in Ubuntu:
  Fix Released

Bug description:
  Host CPU is Core i7 Q820.  KVM is from 2.6.35-gentoo-r5 kernel (x86_64).
  Host has spice-0.7.2 and spice-protocol-0.7.0.
  Guest is Windows XP SP3 with qxl driver 0.6.1, virtio-serial 1.1.6 and 
vdagent 0.6.3.

  qemu-kvm is started like so:
  qemu-system-x86_64 -cpu host -enable-kvm -pidfile /home/rick/qemu/hds/wxp.pid 
-drive 
file=/home/rick/qemu/hds/wxp.raw,if=virtio,media=disk,aio=native,snapshot=on -m 
768 -name WinXP -net nic,model=virtio -net user -localtime -usb -vga qxl 
-device virtio-serial -chardev spicevmc,name=vdagent,id=vdagent -device 
virtserialport,chardev=vdagent,name=com.redhat.spice.0 -spice 
port=1234,disable-ticketing -monitor stdio
  and crashes with:
  qemu-system-x86_64: /home/rick/qemu/src/qemu-kvm-0.14.0/qemu-kvm.c:1724: 
kvm_mutex_unlock: Assertion `!cpu_single_env' failed.
  Aborted

  If I use -no-kvm, it works fine.  If I use -vga std, it works fine.
  -enable-kvm and -vga qxl crashes.

[Qemu-devel] [Bug 723871] Re: qemu-kvm-0.14.0 Aborts with -vga qxl

[Boris Derzhavets]

# dpkg -i --force-overwrite ./spice-gtk_0.6-1_amd64.deb

Generates :-

root@boris-System-P5Q3:/usr/src/SPICE_GTK_0.6/WORK/usr# ls -CR
.:
bin  include  lib  share

./bin:
snappy  spicy

./include:
spice-client-glib-2.0  spice-client-gtk-2.0  spice-controller

./include/spice-client-glib-2.0:
channel-cursor.h   channel-main.h  spice-audio.h  spice-client.h
  spice-types.h
channel-display.h  channel-playback.h  spice-channel-enums.h  
spice-glib-enums.h  spice-util.h
channel-inputs.h   channel-record.hspice-channel.hspice-session.h

./include/spice-client-gtk-2.0:
spice-grabsequence.h  spice-widget-enums.h  spice-widget.h

./include/spice-controller:
spice-controller.h

./lib:
girepository-1.0   libspice-client-gtk-2.0.la
libspice-controller.so
libspice-client-glib-2.0.lalibspice-client-gtk-2.0.so
libspice-controller.so.0
libspice-client-glib-2.0.solibspice-client-gtk-2.0.so.1  
libspice-controller.so.0.0.0
libspice-client-glib-2.0.so.3  libspice-client-gtk-2.0.so.1.1.0  pkgconfig
libspice-client-glib-2.0.so.3.0.2  libspice-controller.lapython2.7

./lib/girepository-1.0:
SpiceClientGLib-2.0.typelib  SpiceClientGtk-2.0.typelib

./lib/pkgconfig:
spice-client-glib-2.0.pc  spice-client-gtk-2.0.pc  spice-controller.pc

./lib/python2.7:
dist-packages

./lib/python2.7/dist-packages:
SpiceClientGtk.la  SpiceClientGtk.so

./share:
doc  gir-1.0  gtk-doc  locale  vala

./share/doc:
spice-gtk

./share/doc/spice-gtk:
ABOUT-NLS  AUTHORS  COPYING  doc  NEWS  README  THANKS  TODO

./share/doc/spice-gtk/doc:
Makefile  Makefile.am  Makefile.in  reference

./share/doc/spice-gtk/doc/reference:
html  Makefile.am  spice-gtk-docs.xml   spice-gtk-sections.txt
Makefile  Makefile.in  spice-gtk-overrides.txt  spice-gtk.types

./share/doc/spice-gtk/doc/reference/html:
api-index-deprecated.html  home.png  SpiceChannel.html 
SpiceInputsChannel.html
api-index-full.htmlindex.htmlSpiceCursorChannel.html   
SpiceMainChannel.html
api-reference.html index.sgmlSpiceDisplayChannel.html  
SpicePlaybackChannel.html
application-support.html   left.png  SpiceDisplay.html 
SpiceRecordChannel.html
ch01.html  object-tree.html  spice-gtk.devhelp 
SpiceSession.html
ch02.html  right.png spice-gtk.devhelp2style.css
ch03.html  SpiceAudio.html   spice-gtk-Utilities.html  up.png

./share/gir-1.0:
SpiceClientGLib-2.0.gir  SpiceClientGtk-2.0.gir

./share/gtk-doc:
html

./share/gtk-doc/html:
spice-gtk

./share/gtk-doc/html/spice-gtk:
api-index-deprecated.html  home.png  SpiceChannel.html 
SpiceInputsChannel.html
api-index-full.htmlindex.htmlSpiceCursorChannel.html   
SpiceMainChannel.html
api-reference.html index.sgmlSpiceDisplayChannel.html  
SpicePlaybackChannel.html
application-support.html   left.png  SpiceDisplay.html 
SpiceRecordChannel.html
ch01.html  object-tree.html  spice-gtk.devhelp 
SpiceSession.html
ch02.html  right.png spice-gtk.devhelp2style.css
ch03.html  SpiceAudio.html   spice-gtk-Utilities.html  up.png

./share/locale:
fr

./share/locale/fr:
LC_MESSAGES

./share/locale/fr/LC_MESSAGES:
spice-gtk.mo

./share/vala:
vapi

./share/vala/vapi:
spice-protocol.vapi

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

Title:
  qemu-kvm-0.14.0 Aborts with -vga qxl

Status in QEMU:
  Invalid
Status in “libvirt” package in Ubuntu:
  Triaged
Status in “qemu-kvm” package in Ubuntu:
  Fix Released

Bug description:
  Host CPU is Core i7 Q820.  KVM is from 2.6.35-gentoo-r5 kernel (x86_64).
  Host has spice-0.7.2 and spice-protocol-0.7.0.
  Guest is Windows XP SP3 with qxl driver 0.6.1, virtio-serial 1.1.6 and 
vdagent 0.6.3.

  qemu-kvm is started like so:
  qemu-system-x86_64 -cpu host -enable-kvm -pidfile /home/rick/qemu/hds/wxp.pid 
-drive 
file=/home/rick/qemu/hds/wxp.raw,if=virtio,media=disk,aio=native,snapshot=on -m 
768 -name WinXP -net nic,model=virtio -net user -localtime -usb -vga qxl 
-device virtio-serial -chardev spicevmc,name=vdagent,id=vdagent -device 
virtserialport,chardev=vdagent,name=com.redhat.spice.0 -spice 
port=1234,disable-ticketing -monitor stdio
  and crashes with:
  qemu-system-x86_64: /home/rick/qemu/src/qemu-kvm-0.14.0/qemu-kvm.c:1724: 
kvm_mutex_unlock: Assertion `!cpu_single_env' failed.
  Aborted

  If I use -no-kvm, it works fine.  If I use -vga std, it works fine.
  -enable-kvm and -vga qxl crashes.

Re: [Qemu-devel] [PATCH 9/9] cpu-exec.c: avoid AREG0 use

[Blue Swirl]

On Sun, May 22, 2011 at 2:18 PM, Blue Swirl  wrote:
> Make functions take a parameter for CPUState instead of relying
> on global env. Pass CPUState pointer to TCG prologue, which moves
> it to AREG0.

I found the problem with this patch on i386, TCG assumes that its
caller is also using global env. Now i386 also works.

Updated patch attached, but here's the diff to previous version:

diff --git a/tcg/hppa/tcg-target.c b/tcg/hppa/tcg-target.c
index 294fc7a..7248520 100644
--- a/tcg/hppa/tcg-target.c
+++ b/tcg/hppa/tcg-target.c
@@ -1596,7 +1596,7 @@ static int tcg_target_callee_save_regs[] = {
 TCG_REG_R14,
 TCG_REG_R15,
 TCG_REG_R16,
-/* R17 is the global env, so no need to save.  */
+TCG_REG_R17, /* R17 is the global env.  */
 TCG_REG_R18
 };

diff --git a/tcg/i386/tcg-target.c b/tcg/i386/tcg-target.c
index ba031ab..72b3a48 100644
--- a/tcg/i386/tcg-target.c
+++ b/tcg/i386/tcg-target.c
@@ -1901,10 +1901,10 @@ static int tcg_target_callee_save_regs[] = {
 TCG_REG_RBX,
 TCG_REG_R12,
 TCG_REG_R13,
-/* TCG_REG_R14, */ /* Currently used for the global env. */
+TCG_REG_R14, /* Currently used for the global env. */
 TCG_REG_R15,
 #else
-/* TCG_REG_EBP, */ /* Currently used for the global env. */
+TCG_REG_EBP, /* Currently used for the global env. */
 TCG_REG_EBX,
 TCG_REG_ESI,
 TCG_REG_EDI,
diff --git a/tcg/mips/tcg-target.c b/tcg/mips/tcg-target.c
index a6b2457..cb2ab8b 100644
--- a/tcg/mips/tcg-target.c
+++ b/tcg/mips/tcg-target.c
@@ -1452,9 +1452,7 @@ static const TCGTargetOpDef mips_op_defs[] = {
 };

 static int tcg_target_callee_save_regs[] = {
-#if 0 /* used for the global env (TCG_AREG0), so no need to save */
-TCG_REG_S0,
-#endif
+TCG_REG_S0,   /* used for the global env (TCG_AREG0) */
 TCG_REG_S1,
 TCG_REG_S2,
 TCG_REG_S3,
diff --git a/tcg/ppc/tcg-target.c b/tcg/ppc/tcg-target.c
index dd2a85a..266e699 100644
--- a/tcg/ppc/tcg-target.c
+++ b/tcg/ppc/tcg-target.c
@@ -160,8 +160,7 @@ static const int tcg_target_callee_save_regs[] = {
 TCG_REG_R24,
 TCG_REG_R25,
 TCG_REG_R26,
-/* TCG_REG_R27, */ /* currently used for the global env, so no
-  need to save */
+TCG_REG_R27, /* currently used for the global env */
 TCG_REG_R28,
 TCG_REG_R29,
 TCG_REG_R30,
diff --git a/tcg/ppc64/tcg-target.c b/tcg/ppc64/tcg-target.c
index 83fa903..2e3cd2b 100644
--- a/tcg/ppc64/tcg-target.c
+++ b/tcg/ppc64/tcg-target.c
@@ -151,8 +151,7 @@ static const int tcg_target_callee_save_regs[] = {
 TCG_REG_R24,
 TCG_REG_R25,
 TCG_REG_R26,
-/* TCG_REG_R27, */ /* currently used for the global env, so no
-  need to save */
+TCG_REG_R27, /* currently used for the global env */
 TCG_REG_R28,
 TCG_REG_R29,
 TCG_REG_R30,

For ARM, the handcrafted instructions below need to be changed to save also r7:
/* stmdb sp!, { r4 - r6, r8 - r11, lr } */
tcg_out32(s, (COND_AL << 28) | 0x092d4f70);

/* ldmia sp!, { r4 - r6, r8 - r11, pc } */
tcg_out32(s, (COND_AL << 28) | 0x08bd8f70);

ia64 doesn't look like saving anything. Sparc is OK with the 'save'.

Maybe S390x already saves R10:
/* stmg %r6,%r15,48(%r15) (save registers) */
tcg_out_insn(s, RXY, STMG, TCG_REG_R6, TCG_REG_R15, TCG_REG_R15, 48);

/* lmg %r6,%r15,208(%r15) (restore registers) */
tcg_out_insn(s, RXY, LMG, TCG_REG_R6, TCG_REG_R15, TCG_REG_R15, 208);
From 73d13f250b70b625b12c26647d1b8746dcc0afb9 Mon Sep 17 00:00:00 2001
Message-Id: <73d13f250b70b625b12c26647d1b8746dcc0afb9.1306082850.git.blauwir...@gmail.com>
From: Blue Swirl 
Date: Sun, 15 May 2011 16:03:25 +
Subject: [PATCH] cpu-exec.c: avoid AREG0 use

Make functions take a parameter for CPUState instead of relying
on global env. Pass CPUState pointer to TCG prologue, which moves
it to AREG0.

Revert the hacks to avoid AREG0 use on Sparc hosts.

Move cpu_has_work() and cpu_pc_from_tb() from exec.h to cpu.h.

Compile the file without HELPER_CFLAGS.

Signed-off-by: Blue Swirl 
---
 Makefile.target|2 +-
 cpu-exec.c |   74 +---
 tcg/arm/tcg-target.c   |4 ++-
 tcg/hppa/tcg-target.c  |6 ++-
 tcg/i386/tcg-target.c  |8 +++--
 tcg/ia64/tcg-target.c  |5 ++-
 tcg/mips/tcg-target.c  |8 ++---
 tcg/ppc/tcg-target.c   |6 ++--
 tcg/ppc64/tcg-target.c |6 ++--
 tcg/s390/tcg-target.c  |5 ++-
 tcg/sparc/tcg-target.c |4 +-
 tcg/tcg.h  |7 ++--
 12 files changed, 54 insertions(+), 81 deletions(-)

diff --git a/Makefile.target b/Makefile.target
index 602d50d..b5fda44 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -94,7 +94,7 @@ tcg/tcg.o: cpu.h
 
 # HELPER_CFLAGS is used for all the code compiled with static register
 # variables
-%_helper.o cpu-exec.o user-exec.o: QEMU_CFLAGS += $(HELPER_CFLAGS)
+%_helper.o user-exec.o: QEMU_CFLAGS += $(HELPER_CFLAGS)
 
 # Note: this is a workaround. The real fix is

Re: [Qemu-devel] [PATCH 21/26] target-xtensa: implement unaligned exception option

[Richard Henderson]

On 05/22/2011 05:10 AM, Max Filippov wrote:
> There are three alignment possibilities for xtensa: no unaligned
> exception, unaligned exception and hardware alignment. In the first
> case unaligned access silently goes to aligned address. It looks like
> it cannot be done via do_unaligned_access, can it? In the third case
> most unaligned accesses are handled transparently by the hardware.
> But e.g. unaligned access by multiprocessor synchronization
> instructions still cause alignment exception. Do I need to implement
> a different alignment checking mechanism for those unhandled cases?

Case (1), silently going to an aligned address, should be handled
inside the translator by masking the address before the load.  See
the ARM and Alpha targets for examples.

Case (2) and (3) are both handled by using ALIGNED_ONLY.  All you
need to do to handle (3) is *not* throw an exception from the
do_unaligned_access function.  See how the code is structured
inside softmmu_template.h.

As for sync insns... You may need to handle it all out-of-line
and check for alignment there.


r~

Re: [Qemu-devel] [PATCH 9/9] cpu-exec.c: avoid AREG0 use

[Peter Maydell]

On 22 May 2011 17:55, Blue Swirl  wrote:
> For ARM, the handcrafted instructions below need to be changed to save also 
> r7:
>    /* stmdb sp!, { r4 - r6, r8 - r11, lr } */
>    tcg_out32(s, (COND_AL << 28) | 0x092d4f70);
>
>    /* ldmia sp!, { r4 - r6, r8 - r11, pc } */
>    tcg_out32(s, (COND_AL << 28) | 0x08bd8f70);

That would be ...ff0 rather than ...f70 in both cases
(bottom 16 bits are a bit map of registers being saved/loaded):

/* stmdb sp!, { r4 - r11, lr } */
tcg_out32(s, (COND_AL << 28) | 0x092d4ff0);

/* ldmia sp!, { r4 - r11, pc } */
tcg_out32(s, (COND_AL << 28) | 0x08bd8ff0);

-- PMM

Re: [Qemu-devel] [PATCH 9/9] cpu-exec.c: avoid AREG0 use

[Laurent Desnogues]

On Sun, May 22, 2011 at 7:10 PM, Peter Maydell  wrote:
> On 22 May 2011 17:55, Blue Swirl  wrote:
>> For ARM, the handcrafted instructions below need to be changed to save also 
>> r7:
>>    /* stmdb sp!, { r4 - r6, r8 - r11, lr } */
>>    tcg_out32(s, (COND_AL << 28) | 0x092d4f70);
>>
>>    /* ldmia sp!, { r4 - r6, r8 - r11, pc } */
>>    tcg_out32(s, (COND_AL << 28) | 0x08bd8f70);
>
> That would be ...ff0 rather than ...f70 in both cases
> (bottom 16 bits are a bit map of registers being saved/loaded):
>
>    /* stmdb sp!, { r4 - r11, lr } */
>    tcg_out32(s, (COND_AL << 28) | 0x092d4ff0);
>
>    /* ldmia sp!, { r4 - r11, pc } */
>    tcg_out32(s, (COND_AL << 28) | 0x08bd8ff0);

Shouldn't you extend the range to include r12, due to
the 8-byte alignment restriction of the stack?


Laurent

Re: [Qemu-devel] [PATCH 9/9] cpu-exec.c: avoid AREG0 use

[Peter Maydell]

On 22 May 2011 18:33, Laurent Desnogues  wrote:
> On Sun, May 22, 2011 at 7:10 PM, Peter Maydell  
> wrote:
>> On 22 May 2011 17:55, Blue Swirl  wrote:
>>> For ARM, the handcrafted instructions below need to be changed to save also 
>>> r7:

>> That would be ...ff0 rather than ...f70 in both cases
>> (bottom 16 bits are a bit map of registers being saved/loaded):

> Shouldn't you extend the range to include r12, due to
> the 8-byte alignment restriction of the stack?

Oops, good point.

/* Calling convention requires us to save r4-r11 and lr;
 * save also r12 to maintain stack 8-alignment.
 */

/* stmdb sp!, { r4 - r12, lr } */
tcg_out32(s, (COND_AL << 28) | 0x092d5ff0);

/* ldmia sp!, { r4 - r12, pc } */
tcg_out32(s, (COND_AL << 28) | 0x08bd9ff0);

-- PMM

Re: [Qemu-devel] [PATCH] piix_pci: fix piix3_set_irq_pic()

[Michael S. Tsirkin]

On Mon, May 16, 2011 at 07:50:55PM +0800, TeLeMan wrote:
> If pic_irq is greater than 7, the irq level is always 0 on 32bits.
> 
> Signed-off-by: TeLeMan 


Good catch

Acked-by: Michael S. Tsirkin 

> ---
>  hw/piix_pci.c |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/hw/piix_pci.c b/hw/piix_pci.c
> index 7f1c4cc..85a320e 100644
> --- a/hw/piix_pci.c
> +++ b/hw/piix_pci.c
> @@ -312,7 +312,7 @@ static void piix3_set_irq_pic(PIIX3State *piix3,
> int pic_irq)
>  {
>  qemu_set_irq(piix3->pic[pic_irq],
>   !!(piix3->pic_levels &
> -(((1UL << PIIX_NUM_PIRQS) - 1) <<
> +(((1ULL << PIIX_NUM_PIRQS) - 1) <<
>   (pic_irq * PIIX_NUM_PIRQS;
>  }
> 
> -- 
> 1.7.3.1.msysgit.0
> --
> SUN OF A BEACH

Re: [Qemu-devel] [PATCH] piix_pci: fix piix3_set_irq_pic()

[Stefan Weil]

Am 22.05.2011 21:06, schrieb Michael S. Tsirkin:

On Mon, May 16, 2011 at 07:50:55PM +0800, TeLeMan wrote:

If pic_irq is greater than 7, the irq level is always 0 on 32bits.

Signed-off-by: TeLeMan 



Good catch

Acked-by: Michael S. Tsirkin 



The patch fixes the regression in Linux networking with 32 bit hosts.
Tested with w32 and linux 32 bit hosts.

Tested-by: Stefan Weil 

[Qemu-devel] [Bug 723871] Re: qemu-kvm-0.14.0 Aborts with -vga qxl

[Boris Derzhavets]

** Attachment added: "spice-gtk-0.6-1ubuntu1_0.6-1_amd64.deb"
   
https://bugs.launchpad.net/qemu/+bug/723871/+attachment/2138919/+files/spice-gtk-0.6-1ubuntu1_0.6-1_amd64.deb

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

Title:
  qemu-kvm-0.14.0 Aborts with -vga qxl

Status in QEMU:
  Invalid
Status in “libvirt” package in Ubuntu:
  Triaged
Status in “qemu-kvm” package in Ubuntu:
  Fix Released

Bug description:
  Host CPU is Core i7 Q820.  KVM is from 2.6.35-gentoo-r5 kernel (x86_64).
  Host has spice-0.7.2 and spice-protocol-0.7.0.
  Guest is Windows XP SP3 with qxl driver 0.6.1, virtio-serial 1.1.6 and 
vdagent 0.6.3.

  qemu-kvm is started like so:
  qemu-system-x86_64 -cpu host -enable-kvm -pidfile /home/rick/qemu/hds/wxp.pid 
-drive 
file=/home/rick/qemu/hds/wxp.raw,if=virtio,media=disk,aio=native,snapshot=on -m 
768 -name WinXP -net nic,model=virtio -net user -localtime -usb -vga qxl 
-device virtio-serial -chardev spicevmc,name=vdagent,id=vdagent -device 
virtserialport,chardev=vdagent,name=com.redhat.spice.0 -spice 
port=1234,disable-ticketing -monitor stdio
  and crashes with:
  qemu-system-x86_64: /home/rick/qemu/src/qemu-kvm-0.14.0/qemu-kvm.c:1724: 
kvm_mutex_unlock: Assertion `!cpu_single_env' failed.
  Aborted

  If I use -no-kvm, it works fine.  If I use -vga std, it works fine.
  -enable-kvm and -vga qxl crashes.

Re: [Qemu-devel] [PATCH 21/26] target-xtensa: implement unaligned exception option

[Max Filippov]

> > There are three alignment possibilities for xtensa: no unaligned
> > exception, unaligned exception and hardware alignment. In the first
> > case unaligned access silently goes to aligned address. It looks like
> > it cannot be done via do_unaligned_access, can it? In the third case
> > most unaligned accesses are handled transparently by the hardware.
> > But e.g. unaligned access by multiprocessor synchronization
> > instructions still cause alignment exception. Do I need to implement
> > a different alignment checking mechanism for those unhandled cases?
> 
> Case (1), silently going to an aligned address, should be handled
> inside the translator by masking the address before the load.  See
> the ARM and Alpha targets for examples.

This is what gen_load_store_alignment does.

> Case (2) and (3) are both handled by using ALIGNED_ONLY.  All you
> need to do to handle (3) is *not* throw an exception from the
> do_unaligned_access function.  See how the code is structured
> inside softmmu_template.h.
> 
> As for sync insns... You may need to handle it all out-of-line
> and check for alignment there.

This is also done by gen_load_store_alignment.
Does it really worth copying part of this logic to do_unaligned_access just to 
use ALIGNED_ONLY framework?

Thanks.
-- Max

[Qemu-devel] [PATCH 2/4] hw/realview.c: Remove duplicate #include line

[Stefan Hajnoczi]

From: Peter Maydell 

Remove a duplicate #include of sysbus.h.

Signed-off-by: Peter Maydell 
Signed-off-by: Stefan Hajnoczi 
---
 hw/realview.c |1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/hw/realview.c b/hw/realview.c
index 96fb9da..82f3d82 100644
--- a/hw/realview.c
+++ b/hw/realview.c
@@ -17,7 +17,6 @@
 #include "sysemu.h"
 #include "boards.h"
 #include "bitbang_i2c.h"
-#include "sysbus.h"
 #include "blockdev.h"
 
 #define SMP_BOOT_ADDR 0xe000
-- 
1.7.4.4

[Qemu-devel] [PATCH 4/4] Fix typos in comments (chek -> check)

[Stefan Hajnoczi]

From: Stefan Weil 

Signed-off-by: Stefan Weil 
Signed-off-by: Stefan Hajnoczi 
---
 exec.c|2 +-
 target-ppc/STATUS |2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/exec.c b/exec.c
index a6df2d6..563e974 100644
--- a/exec.c
+++ b/exec.c
@@ -2061,7 +2061,7 @@ void cpu_physical_memory_reset_dirty(ram_addr_t start, 
ram_addr_t end,
 /* we modify the TLB cache so that the dirty bit will be set again
when accessing the range */
 start1 = (unsigned long)qemu_safe_ram_ptr(start);
-/* Chek that we don't span multiple blocks - this breaks the
+/* Check that we don't span multiple blocks - this breaks the
address comparisons below.  */
 if ((unsigned long)qemu_safe_ram_ptr(end - 1) - start1
 != (end - 1) - start) {
diff --git a/target-ppc/STATUS b/target-ppc/STATUS
index 32e7ffa..c8e9018 100644
--- a/target-ppc/STATUS
+++ b/target-ppc/STATUS
@@ -11,7 +11,7 @@ INSN: instruction set.
 SPR:  special purpose registers set
   OK => all SPR registered (but some may be fake)
   KO => some SPR are missing or should be removed
-  ?  => uncheked
+  ?  => unchecked
 MSR:  MSR bits definitions
   OK => all MSR bits properly defined
   KO => MSR definition is incorrect
-- 
1.7.4.4

[Qemu-devel] [PATCH 3/4] hw/sd.c: Don't complain about SDIO commands CMD52/CMD53

[Stefan Hajnoczi]

From: Peter Maydell 

The SDIO specification introduces new commands 52 and 53.
Handle as illegal command but do not complain on stderr,
as SDIO-aware OSes (including Linux) may legitimately use
these in their probing for presence of an SDIO card.

Signed-off-by: Peter Maydell 
Signed-off-by: Stefan Hajnoczi 
---
 hw/sd.c |   11 +++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/hw/sd.c b/hw/sd.c
index f44a970..cedfb20 100644
--- a/hw/sd.c
+++ b/hw/sd.c
@@ -1104,6 +1104,17 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
 }
 break;
 
+case 52:
+case 53:
+/* CMD52, CMD53: reserved for SDIO cards
+ * (see the SDIO Simplified Specification V2.0)
+ * Handle as illegal command but do not complain
+ * on stderr, as some OSes may use these in their
+ * probing for presence of an SDIO card.
+ */
+sd->card_status |= ILLEGAL_COMMAND;
+return sd_r0;
+
 /* Application specific commands (Class 8) */
 case 55:   /* CMD55:  APP_CMD */
 if (sd->rca != rca)
-- 
1.7.4.4

[Qemu-devel] [PATCH 1/4] piix_pci: fix piix3_set_irq_pic()

[Stefan Hajnoczi]

From: TeLeMan 

If pic_irq is greater than 7, the irq level is always 0 on 32bits.

Signed-off-by: TeLeMan 
Signed-off-by: Stefan Hajnoczi 
---
 hw/piix_pci.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/piix_pci.c b/hw/piix_pci.c
index 7f1c4cc..85a320e 100644
--- a/hw/piix_pci.c
+++ b/hw/piix_pci.c
@@ -312,7 +312,7 @@ static void piix3_set_irq_pic(PIIX3State *piix3, int 
pic_irq)
 {
 qemu_set_irq(piix3->pic[pic_irq],
  !!(piix3->pic_levels &
-(((1UL << PIIX_NUM_PIRQS) - 1) <<
+(((1ULL << PIIX_NUM_PIRQS) - 1) <<
  (pic_irq * PIIX_NUM_PIRQS;
 }
 
-- 
1.7.4.4

[Qemu-devel] [PULL 0/4] Trivial patches for May 16 to May 20

[Stefan Hajnoczi]

The following changes since commit dcfd14b3741983c466ad92fa2ae91eeafce3e5d5:

  Delete unused tb_invalidate_page_range (2011-05-22 10:47:28 +)

are available in the git repository at:
  git://repo.or.cz/qemu/stefanha.git trivial-patches

I must have lost Stefan Weil's patch last pull request.  He noticed and it has
been added.

Peter Maydell (2):
  hw/realview.c: Remove duplicate #include line
  hw/sd.c: Don't complain about SDIO commands CMD52/CMD53

Stefan Weil (1):
  Fix typos in comments (chek -> check)

TeLeMan (1):
  piix_pci: fix piix3_set_irq_pic()

 exec.c|2 +-
 hw/piix_pci.c |2 +-
 hw/realview.c |1 -
 hw/sd.c   |   11 +++
 target-ppc/STATUS |2 +-
 5 files changed, 14 insertions(+), 4 deletions(-)

-- 
1.7.4.4

[Qemu-devel] Reminder about your invitation from Paulo Silva

[Paulo Silva (LinkedIn Invitations)]

LinkedIn

This invitation is awaiting your response:
 From Paulo Silva 

 
-- 
(c) 2011, LinkedIn Corporation

Re: [Qemu-devel] [PATCH] piix_pci: fix piix3_set_irq_pic()

[Isaku Yamahata]

Thank you.
Acked-by: Isaku Yamahata 

On Mon, May 16, 2011 at 07:50:55PM +0800, TeLeMan wrote:
> If pic_irq is greater than 7, the irq level is always 0 on 32bits.
> 
> Signed-off-by: TeLeMan 
> ---
>  hw/piix_pci.c |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/hw/piix_pci.c b/hw/piix_pci.c
> index 7f1c4cc..85a320e 100644
> --- a/hw/piix_pci.c
> +++ b/hw/piix_pci.c
> @@ -312,7 +312,7 @@ static void piix3_set_irq_pic(PIIX3State *piix3,
> int pic_irq)
>  {
>  qemu_set_irq(piix3->pic[pic_irq],
>   !!(piix3->pic_levels &
> -(((1UL << PIIX_NUM_PIRQS) - 1) <<
> +(((1ULL << PIIX_NUM_PIRQS) - 1) <<
>   (pic_irq * PIIX_NUM_PIRQS;
>  }
> 
> -- 
> 1.7.3.1.msysgit.0
> --
> SUN OF A BEACH
> 

-- 
yamahata

Re: [Qemu-devel] [PATCH v4 2/3] coroutine: add check-coroutine automated tests

[Stefan Hajnoczi]

On Fri, May 20, 2011 at 11:14 PM, Anthony Liguori  wrote:
> On 05/20/2011 05:59 AM, Stefan Hajnoczi wrote:
>>
>> To run automated tests for coroutines:
>>
>>   make check-coroutine
>>   ./check-coroutine
>>
>> On success the program terminates with exit status 0.  On failure an
>> error message is written to stderr and the program exits with exit
>> status 1.
>>
>> Signed-off-by: Stefan Hajnoczi
>
> Can you include the glib patch and switch this to a gtest based test?  I
> don't want to introduce more libcheck tests.

This isn't a libcheck test.  I'm happy to user gtester though.

Stefan

Re: [Qemu-devel] [RFC] live snapshot, live merge, live block migration

[Jagane Sundar]

Hello Stefan,

I have been thinking about this since you sent out this message.
A quick look at the libvirt API indicates that their notion of a
snapshot often refers to a "disk+memory snapshot". It would
be good to provide feedback to the libvirt developers to make
sure that proper support for a 'disk only snapshot' capability is
included.

You might have already seen this, but here's a email chain from
the libvirt mailing list that's relevant:

http://www.redhat.com/archives/libvir-list/2010-March/msg01389.html

I am very interested in enhancing libvirt to support
the Livebackup semantics, for the following reason:
If libvirt can be enhanced to support all the constructs
required for full Livebackup functionality, then I would like to
remove the built-in livebackup network protocol, and rewrite
the client such that it is a native program on the VM host linked
with libvirt, and can perform a full or incremental backup using
libvirt. If a remote backup needs to be performed, then I would
require the remote client to ssh into the VM host, and then
run the local backup and pipe back to the remote backup host.
This way I would not need to deal with authentication of
livebackup client and server, and encryption of the network
connection.

Please see my feedback regarding the specific operations below:

On 5/20/2011 5:19 AM, Stefan Hajnoczi wrote:

I'm interested in what the API for snapshots would look like.
Specifically how does user software do the following:
1. Create a snapshot

For livebackup, one parameter that is required is the 'full' or
'incremental' backup parameter. If the param is 'incremental'
then only the blocks that were modified since the last snapshot
command was issued are part of the snapshot. If the param
is 'full', the the snapshot includes all the blocks of all the disks
in the VM.

2. Delete a snapshot

Simple for livebackup, since no more than one snapshot is
allowed. Hence naming is a non-issue. As is deleting.

3. List snapshots

Again, simple for livebackup, on account of the one
active snapshot restriction.

4. Access data from a snapshot

In traditional terms, access could mean many
things. Some examples:
1. Access lists a set of files on the local
file system of the VM Host. A small VM
   may be started up, and mount these
   snapshot files as a set of secondary drives
2. Publish the snapshot drives as iSCSI LUNs.
3. If the origin drives are on a Netapp filer,
perhaps a filer snapshot is created, and
a URL describing that snapshot is printed
out.

Access, in Livebackup terms, is merely copying
dirty blocks over from qemu. Livebackup does
not provide a random access mode - i.e. one
where a VM could be started using the snapshot.

Currently, Livebackup uses 4K clusters of 512 byte
blocks. 'Dirty clusters' are transferred over by the
client supplying a 'cluster number' param, and qemu
returning the next 'n' number of contiguous dirty
clusters. At the end, qemu returns a 'no-more-dirty'
error.

5. Restore a VM from a snapshot


Additional info for re-creating the VM needs to be
saved when a snapshot is saved. The origin VM's
libvirt XML desciptor should probably be saved
along with the snapshot.


6. Get the dirty blocks list (for incremental backup)

Either a complete dump of the dirty blocks, or a way
to iterate through the dirty blocks and fetch them
needs to be provided. My preference is to use the
iterate through the dirty blocks approach, since
that will enable the client to pace the backup
process and provide guarantees such as 'no more
than 10% of the network b/w will be utilized for
backup'.

We've discussed image format-level approaches but I think the scope of
the API should cover several levels at which snapshots are
implemented:
1. Image format - image file snapshot (Jes, Jagane)

Livebackup uses qcow2 to save the Copy-On-Write blocks
that are dirtied by the VM when the snapshot is active.

2. Host file system - ext4 and btrfs snapshots

I have tested with ext4 and raw LVM volumes for the origin
virtual disk files. The qcow2 COW files have only resided on
ext4.

3. Storage system - LVM or SAN volume snapshots




It will be hard to take advantage of more efficient host file system
or storage system snapshots if they are not designed in now.


I agree. A snapshot and restore from backup should not result in
the virtual disk file getting inflated (going from sparse to fully
allocated, for example).

Is anyone familiar enough with the libvirt storage APIs to draft an
extension that adds snapshot support?  I will take a stab at it if no
one else want to try it.


I have only looked at it briefly, after getting your email message.
If you can take a deeper look at it, I would be willing to work with
you to iron out details.

Thanks,
Jagane

Re: [Qemu-devel] [PATCH] virtio: correctly initialize vm_running

[Jason Wang]

On 05/18/2011 04:54 PM, Michael S. Tsirkin wrote:

On Wed, May 18, 2011 at 01:57:37PM +0800, Jason Wang wrote:

Current vm_running was not explicitly initialized and its value was changed by
vm state notifier, this may confuse the virtio device being hotplugged such as
virtio-net with vhost backend as it may think the vm was not running. Solve this
by initialize this value explicitly in virtio_common_init().

Signed-off-by: Jason Wang

I think a slightly cleaner way would be to invoke
virtio_vmstate_change after it is registered,
have that update the vm_running field.



Do you mean call it in virtio_common_init() directly? But the bindings 
are not initialized then. Or is there anything I missed?



---
  0 files changed, 0 insertions(+), 0 deletions(-)

diff --git a/hw/virtio.c b/hw/virtio.c
index 6e8814c..27d7e50 100644
--- a/hw/virtio.c
+++ b/hw/virtio.c
@@ -789,6 +789,7 @@ VirtIODevice *virtio_common_init(const char *name, uint16_t 
device_id,
  vdev->queue_sel = 0;
  vdev->config_vector = VIRTIO_NO_VECTOR;
  vdev->vq = qemu_mallocz(sizeof(VirtQueue) * VIRTIO_PCI_QUEUE_MAX);
+vdev->vm_running = vm_running;
  for(i = 0; i<  VIRTIO_PCI_QUEUE_MAX; i++) {
  vdev->vq[i].vector = VIRTIO_NO_VECTOR;
  vdev->vq[i].vdev = vdev;

Re: [Qemu-devel] [V2 2/2]Qemu: Add commands "hostcache_set" and "hostcache_get"

[Supriya Kannery]

On 05/20/2011 01:50 PM, Stefan Hajnoczi wrote:

On Thu, May 19, 2011 at 10:38:03PM +0530, Supriya Kannery wrote:

Monitor commands "hostcache_set" and "hostcache_get" added for dynamic
host cache change and display of host cache setting respectively.


A generic command for changing block device options would be nice,
althought I don't see other options where it makes sense to change them
at runtime.

The alternative would be:

block_set hostcache on

"block_set", {"device": "ide1-cd0", "name": "hostcache", "enable": true}

The hostcache_get information would be part of query-block output:
  {
 "device":"ide0-hd0",
 "locked":false,
 "removable":false,
 "inserted":{
"ro":false,
"drv":"qcow2",
"encrypted":false,
"file":"disks/test.img"
   "hostcache":true,
 },
 "type":"hd"
  },

This approach is extensible if more options need to be exposed.


Sure, I will resubmit this patchset, after making this feature more 
generic. Can you pls help finding atleast one or two options (other than 
hostcache) which can be changed dynamically. This will help me evaluate 
the generic approach.



+.params = "device",
+.help   = "retrieve host cache settings for device",


Please make it clear these operations affect block devices:
"for block device"


ok




+/*
+ * A failed attempt to reopen the image file must lead to 'abort()'
+ */
+if (ret != 0) {
+qerror_report(QERR_REOPEN_FILE_FAILED, bs->filename);
+abort();


The error is never reported on a QMP monitor because qerror_report()
simply stashes away the qerror.  The QMP client doesn't have a chance to
read the error before QEMU terminates.



Verified this from qemu command line and the error got displayed before 
aborting (when the image file was renamed while VM was running).



+
+int bdrv_change_hostcache(BlockDriverState *bs, bool enable_host_cache)


Consistently using "hostcache" or "host_cache" would be nice.



ok




+return -1;


This shouldn't be a failure and please don't use -1 when a negative
errno indicates failure.  -1 == -EPERM.  The return value should be 0
here.


+}


Anyway, this whole check is unnecessary since bdrv_reopen() already
performs it.



will take this off


Please run scripts/checkpatch.pl before submitting patches.



ok..will do


+
+Arguments:
+
+- "device": the device's ID, must be unique (json-string)
+- "cache": host cache value "off" or "on" (json-string)


There is a boolean value that can be used instead of string on|off.  See
the set_link command.



ok


Stefan