[Qemu-devel] [Bug 1014681] Re: BSOD with newer host kernels (x64) and W2k8S guest (x64)

[Arndt Kritzner]

Most (by far) crashes end with the same bug check code:

Loading Dump File [\\Lw\arndt\Kanzlei\Mini061612-01.dmp]
BugCheck 3B, {8003, f800016a6700, fa6002c89e60, 0}
Probably caused by : afd.sys ( afd!AfdIssueDeviceControl+18f )
-
Loading Dump File [\\Lw\arndt\Kanzlei\Mini061612-02.dmp]
BugCheck 3B, {8003, f800016b5700, fa6002715fa0, 0}
-
Loading Dump File [\\Lw\arndt\Kanzlei\Mini061612-03.dmp]
BugCheck 3B, {8003, f8000165e700, fa60032a3fa0, 0}
-
Loading Dump File [\\Lw\arndt\Kanzlei\Mini061612-04.dmp]
BugCheck 50, {fa60, 1, f80001615261, 0}
Could not read faulting driver name
Probably caused by : ntkrnlmp.exe ( nt! ?? ::FNODOBFM::`string'+5d2c )
-
Loading Dump File [\\Lw\arndt\Kanzlei\Mini061612-05.dmp]
BugCheck 3B, {8003, f800016b4700, fa6001fc5f00, 0}
-
Loading Dump File [\\Lw\arndt\Kanzlei\Mini061612-06.dmp]
BugCheck 3B, {8003, f8000165f700, fa6001d95fa0, 0}
-
Loading Dump File [\\Lw\arndt\Kanzlei\Mini061612-07.dmp]
BugCheck 3B, {8003, f800016b8700, fa600316ffa0, 0}
-
Loading Dump File [\\Lw\arndt\Kanzlei\Mini061612-08.dmp]
BugCheck 3B, {8003, f800016b5700, fa600292ca10, 0}
-
Loading Dump File [\\Lw\arndt\Kanzlei\Mini061612-09.dmp]
BugCheck 3B, {8003, f8000166b700, fa6001c1afa0, 0}
-
Loading Dump File [\\Lw\arndt\Kanzlei\Mini061612-10.dmp]
BugCheck 107E, {c005, f800016f2e7d, fa6001970858, 
fa6001970230}
Probably caused by : ntkrnlmp.exe ( nt!KiUnwaitThread+19 )
-
Loading Dump File [\\Lw\arndt\Kanzlei\Mini061612-11.dmp]
BugCheck 3B, {8003, f80001656700, fa60032befa0, 0}

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

Title:
  BSOD with newer host kernels (x64) and W2k8S guest (x64)

Status in QEMU:
  New

Bug description:
  Hallo, I attempted to move virtual machines from one host to another
  but got stuck with Windows-BSODs on the target host. The host-side
  console message is "virtio_ioport_write: unexpected address 0x13 value
  0x1". Eventually there are overlaps to bug #990364, but I'm not sure.

  Host machine: 2x Opteron 4238 a 6 cores, 32GB RAM, Linux x86_64
  Guest machine(s): Windows 2008 Server R2 x64

  I tried different combinations of component versions, but only kernel
  2.6.34 could run the guests (but has other difficulties). See testet
  variants in comment.

  Run arguments are attached. Minidump follows immediately.

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

[Qemu-devel] [PATCH] msi/msix: added public API to set/get MSI message address, and data

[Alexey Kardashevskiy]

agrhhh. sha1 of the patch changed after rebasing :)



Added (msi|msix)_(set|get)_message() function for whoever might
want to use them.

Currently msi_notify()/msix_notify() write to these vectors to
signal the guest about an interrupt so the correct values have to
written there by the guest or QEMU.

For example, POWER guest never initializes MSI/MSIX vectors, instead
it uses RTAS hypercalls. So in order to support MSIX for virtio-pci on
POWER we have to initialize MSI/MSIX message from QEMU.

As only set* function are required by now, the "get" functions were added
or made public for a symmetry.

Signed-off-by: Alexey Kardashevskiy 
---
 hw/msi.c  |   29 +
 hw/msi.h  |2 ++
 hw/msix.c |   11 ++-
 hw/msix.h |3 +++
 4 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/hw/msi.c b/hw/msi.c
index 5233204..9ad84a4 100644
--- a/hw/msi.c
+++ b/hw/msi.c
@@ -105,6 +105,35 @@ static inline uint8_t msi_pending_off(const PCIDevice* 
dev, bool msi64bit)
 return dev->msi_cap + (msi64bit ? PCI_MSI_PENDING_64 : PCI_MSI_PENDING_32);
 }
 
+MSIMessage msi_get_message(PCIDevice *dev)
+{
+uint16_t flags = pci_get_word(dev->config + msi_flags_off(dev));
+bool msi64bit = flags & PCI_MSI_FLAGS_64BIT;
+MSIMessage msg;
+
+if (msi64bit) {
+msg.address = pci_get_quad(dev->config + msi_address_lo_off(dev));
+} else {
+msg.address = pci_get_long(dev->config + msi_address_lo_off(dev));
+}
+msg.data = pci_get_word(dev->config + msi_data_off(dev, msi64bit));
+
+return msg;
+}
+
+void msi_set_message(PCIDevice *dev, MSIMessage msg)
+{
+uint16_t flags = pci_get_word(dev->config + msi_flags_off(dev));
+bool msi64bit = flags & PCI_MSI_FLAGS_64BIT;
+
+if (msi64bit) {
+pci_set_quad(dev->config + msi_address_lo_off(dev), msg.address);
+} else {
+pci_set_long(dev->config + msi_address_lo_off(dev), msg.address);
+}
+pci_set_word(dev->config + msi_data_off(dev, msi64bit), msg.data);
+}
+
 bool msi_enabled(const PCIDevice *dev)
 {
 return msi_present(dev) &&
diff --git a/hw/msi.h b/hw/msi.h
index 75747ab..4b0f4f8 100644
--- a/hw/msi.h
+++ b/hw/msi.h
@@ -31,6 +31,8 @@ struct MSIMessage {
 
 extern bool msi_supported;
 
+MSIMessage msi_get_message(PCIDevice *dev);
+void msi_set_message(PCIDevice *dev, MSIMessage msg);
 bool msi_enabled(const PCIDevice *dev);
 int msi_init(struct PCIDevice *dev, uint8_t offset,
  unsigned int nr_vectors, bool msi64bit, bool msi_per_vector_mask);
diff --git a/hw/msix.c b/hw/msix.c
index ded3c55..9e8d8bb 100644
--- a/hw/msix.c
+++ b/hw/msix.c
@@ -35,7 +35,7 @@
 #define MSIX_PAGE_PENDING (MSIX_PAGE_SIZE / 2)
 #define MSIX_MAX_ENTRIES 32
 
-static MSIMessage msix_get_message(PCIDevice *dev, unsigned vector)
+MSIMessage msix_get_message(PCIDevice *dev, unsigned vector)
 {
 uint8_t *table_entry = dev->msix_table_page + vector * PCI_MSIX_ENTRY_SIZE;
 MSIMessage msg;
@@ -45,6 +45,15 @@ static MSIMessage msix_get_message(PCIDevice *dev, unsigned 
vector)
 return msg;
 }
 
+void msix_set_message(PCIDevice *dev, int vector, struct MSIMessage msg)
+{
+uint8_t *table_entry = dev->msix_table_page + vector * PCI_MSIX_ENTRY_SIZE;
+
+pci_set_quad(table_entry + PCI_MSIX_ENTRY_LOWER_ADDR, msg.address);
+pci_set_long(table_entry + PCI_MSIX_ENTRY_DATA, msg.data);
+table_entry[PCI_MSIX_ENTRY_VECTOR_CTRL] &= ~PCI_MSIX_ENTRY_CTRL_MASKBIT;
+}
+
 /* Add MSI-X capability to the config space for the device. */
 /* Given a bar and its size, add MSI-X table on top of it
  * and fill MSI-X capability in the config space.
diff --git a/hw/msix.h b/hw/msix.h
index 50aee82..3374cf8 100644
--- a/hw/msix.h
+++ b/hw/msix.h
@@ -4,6 +4,9 @@
 #include "qemu-common.h"
 #include "pci.h"
 
+MSIMessage msix_get_message(PCIDevice *dev, unsigned vector);
+void msix_set_message(PCIDevice *dev, int vector, MSIMessage msg);
+
 int msix_init(PCIDevice *pdev, unsigned short nentries,
   MemoryRegion *bar,
   unsigned bar_nr, unsigned bar_size);
-- 
1.7.10



On 21/06/12 16:53, Jan Kiszka wrote:
> On 2012-06-21 08:46, Alexey Kardashevskiy wrote:
>>
>> Ok, another try. Is it any better now? :)
> 
> No - posted the old version accidentally?
> 
> Jan
> 
>>
>>
>> Normally QEMU expects the guest to initialize MSI/MSIX vectors.
>> However on POWER the guest uses RTAS subsystem to configure MSI/MSIX and
>> does not write these vectors to device's config space or MSIX BAR.
>>
>> On the other hand, msi_notify()/msix_notify() write to these vectors to
>> signal the guest about an interrupt so we have to write correct vectors
>> to the devices in order not to change every user of MSI/MSIX.
>>
>> The first aim is to support MSIX for virtio-pci on POWER. There is
>> another patch for POWER coming which introduces a special memory region
>> where MSI/MSIX vectors point to.
>>
>> Signed-off-by: Alexey Kardashevskiy 
>> ---
>>  hw/msi.c  |   14 ++
>>  hw/msi.h  |

Re: [Qemu-devel] [PATCH 04/13] usb-ohci: Use universal DMA helper functions

[Michael S. Tsirkin]

On Thu, Jun 21, 2012 at 08:02:06AM +1000, Benjamin Herrenschmidt wrote:
> On Wed, 2012-06-20 at 16:40 -0500, Anthony Liguori wrote:
> 
> > Well let's return void in the DMA methods and let the IOMMUs assert on 
> > error. 
> > At least that will avoid surprises until someone decides they care enough 
> > about 
> > errors to touch all callers.
> > 
> > I think silently failing a memcpy() can potentially lead to a vulnerability 
> > so 
> > I'd rather avoid that.
> 
> No I'd rather keep the error returns, really, even if that means fixing
> a few devices. I can look at making sure we don't pass random qemu data,
> on error that's reasonably easy.
> 
> assert on error means guest code can assert qemu ... not a great idea
> but maybe we can add a warning.

Why not?  Guest can always just halt if it wants to anyway.
On the other hand, warnings can fill up host logs so
represent a security problem.

> > >> Why leave pci accessors and not implement usb_memory_rw() wrappers?
> > >
> > > Well, "usb" is a bit too generic, ehci and ohci would each need to have
> > > their own sets of wrappers. But yes, that's possible... is it really
> > > worth it ? There's nothing fundamentally wrong with using the dma_*
> > > accessors.
> > 
> > So is using the pci accessors wrong?
> 
> Not really either, I don't think it matters :-)
> 
> > I'm not saying you should go and convert every caller of the pci_ 
> > functions, I 
> > just want a clear policy on what interface devices should use.
> 
> Ideally the bus interface for the bus they sit on so they don't have to
> bother digging the DMAContext and are immune to change we would do in
> that area.
> 
> Devices that mix multiple bus types however are a bit more tricky, but
> so far are few, and those can use dma_* and know where to get the
> DMAContext from.
> 
> If we ever replace DMAContext with something else we can probably just
> change the field to that "something else" with a very simple
> search/replace on those devices (at least that's the best case :-)
> 
> I think anything else is just no worth bothering.
> 
> Cheers,
> Ben.

Re: [Qemu-devel] [PATCH] msi/msix: added public API to set/get MSI message address, and data

[Jan Kiszka]

On 2012-06-21 09:18, Alexey Kardashevskiy wrote:
> 
> agrhhh. sha1 of the patch changed after rebasing :)
> 
> 
> 
> Added (msi|msix)_(set|get)_message() function for whoever might
> want to use them.
> 
> Currently msi_notify()/msix_notify() write to these vectors to
> signal the guest about an interrupt so the correct values have to
> written there by the guest or QEMU.
> 
> For example, POWER guest never initializes MSI/MSIX vectors, instead
> it uses RTAS hypercalls. So in order to support MSIX for virtio-pci on
> POWER we have to initialize MSI/MSIX message from QEMU.
> 
> As only set* function are required by now, the "get" functions were added
> or made public for a symmetry.
> 
> Signed-off-by: Alexey Kardashevskiy 
> ---
>  hw/msi.c  |   29 +
>  hw/msi.h  |2 ++
>  hw/msix.c |   11 ++-
>  hw/msix.h |3 +++
>  4 files changed, 44 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/msi.c b/hw/msi.c
> index 5233204..9ad84a4 100644
> --- a/hw/msi.c
> +++ b/hw/msi.c
> @@ -105,6 +105,35 @@ static inline uint8_t msi_pending_off(const PCIDevice* 
> dev, bool msi64bit)
>  return dev->msi_cap + (msi64bit ? PCI_MSI_PENDING_64 : 
> PCI_MSI_PENDING_32);
>  }
>  
> +MSIMessage msi_get_message(PCIDevice *dev)

MSIMessage msi_get_message(PCIDevice *dev, unsigned vector)

> +{
> +uint16_t flags = pci_get_word(dev->config + msi_flags_off(dev));
> +bool msi64bit = flags & PCI_MSI_FLAGS_64BIT;
> +MSIMessage msg;
> +
> +if (msi64bit) {
> +msg.address = pci_get_quad(dev->config + msi_address_lo_off(dev));
> +} else {
> +msg.address = pci_get_long(dev->config + msi_address_lo_off(dev));
> +}
> +msg.data = pci_get_word(dev->config + msi_data_off(dev, msi64bit));

And I have this here in addition:

unsigned int nr_vectors = msi_nr_vectors(flags);
...

if (nr_vectors > 1) {
msg.data &= ~(nr_vectors - 1);
msg.data |= vector;
}

See PCI spec and existing code.

> +
> +return msg;
> +}
> +
> +void msi_set_message(PCIDevice *dev, MSIMessage msg)
> +{
> +uint16_t flags = pci_get_word(dev->config + msi_flags_off(dev));
> +bool msi64bit = flags & PCI_MSI_FLAGS_64BIT;
> +
> +if (msi64bit) {
> +pci_set_quad(dev->config + msi_address_lo_off(dev), msg.address);
> +} else {
> +pci_set_long(dev->config + msi_address_lo_off(dev), msg.address);
> +}
> +pci_set_word(dev->config + msi_data_off(dev, msi64bit), msg.data);
> +}
> +
>  bool msi_enabled(const PCIDevice *dev)
>  {
>  return msi_present(dev) &&
> diff --git a/hw/msi.h b/hw/msi.h
> index 75747ab..4b0f4f8 100644
> --- a/hw/msi.h
> +++ b/hw/msi.h
> @@ -31,6 +31,8 @@ struct MSIMessage {
>  
>  extern bool msi_supported;
>  
> +MSIMessage msi_get_message(PCIDevice *dev);
> +void msi_set_message(PCIDevice *dev, MSIMessage msg);
>  bool msi_enabled(const PCIDevice *dev);
>  int msi_init(struct PCIDevice *dev, uint8_t offset,
>   unsigned int nr_vectors, bool msi64bit, bool 
> msi_per_vector_mask);
> diff --git a/hw/msix.c b/hw/msix.c
> index ded3c55..9e8d8bb 100644
> --- a/hw/msix.c
> +++ b/hw/msix.c
> @@ -35,7 +35,7 @@
>  #define MSIX_PAGE_PENDING (MSIX_PAGE_SIZE / 2)
>  #define MSIX_MAX_ENTRIES 32
>  
> -static MSIMessage msix_get_message(PCIDevice *dev, unsigned vector)
> +MSIMessage msix_get_message(PCIDevice *dev, unsigned vector)
>  {
>  uint8_t *table_entry = dev->msix_table_page + vector * 
> PCI_MSIX_ENTRY_SIZE;
>  MSIMessage msg;
> @@ -45,6 +45,15 @@ static MSIMessage msix_get_message(PCIDevice *dev, 
> unsigned vector)
>  return msg;
>  }
>  
> +void msix_set_message(PCIDevice *dev, int vector, struct MSIMessage msg)
> +{
> +uint8_t *table_entry = dev->msix_table_page + vector * 
> PCI_MSIX_ENTRY_SIZE;
> +
> +pci_set_quad(table_entry + PCI_MSIX_ENTRY_LOWER_ADDR, msg.address);
> +pci_set_long(table_entry + PCI_MSIX_ENTRY_DATA, msg.data);
> +table_entry[PCI_MSIX_ENTRY_VECTOR_CTRL] &= ~PCI_MSIX_ENTRY_CTRL_MASKBIT;
> +}
> +
>  /* Add MSI-X capability to the config space for the device. */
>  /* Given a bar and its size, add MSI-X table on top of it
>   * and fill MSI-X capability in the config space.
> diff --git a/hw/msix.h b/hw/msix.h
> index 50aee82..3374cf8 100644
> --- a/hw/msix.h
> +++ b/hw/msix.h
> @@ -4,6 +4,9 @@
>  #include "qemu-common.h"
>  #include "pci.h"
>  
> +MSIMessage msix_get_message(PCIDevice *dev, unsigned vector);
> +void msix_set_message(PCIDevice *dev, int vector, MSIMessage msg);
> +
>  int msix_init(PCIDevice *pdev, unsigned short nentries,
>MemoryRegion *bar,
>unsigned bar_nr, unsigned bar_size);
> 

General remark: You will make the life of the maintainers easier by
formatting your patch in a way that a clean merge via git works without
hand-editing. E.g. this patch was no scissor line (---8<--- etc.)
between introductory text and the patch description. And the subject is
not "[PATCH] ...".

Jan

-- 
Siemens AG, Corp

Re: [Qemu-devel] [PATCH 01/13] Better support for dma_addr_t variables

[Peter Maydell]

On 20 June 2012 23:59, Anthony Liguori  wrote:
> On 06/20/2012 05:26 PM, Peter Maydell wrote:
>> On 20 June 2012 22:14, Anthony Liguori  wrote:
>> ...for that matter weren't we tossing around the idea of just
>> making target_phys_addr_t 64 bits for everything? (I actually
>> want to do this for target-arm anyway; last time I did some
>> quick smoke-tests of performance it didn't seem to hurt really
>> even on a 32 bit host, and it avoids having to put the A15 in
>> a different qemu-system-* binary to the other cores.)

> Didn't you whine and moan about the impact to printf()s last time I did
> this? ;-)

IIRC somebody in the review thread came up with a nice
solution to that, though I forget what it was exactly.

-- PMM

[Qemu-devel] [PATCH 1/2 v2] scsi bus: introduce hotplug() and hot_unplug() interfaces for SCSI bus

[Cong Meng]

Add two interfaces hotplug() and hot_unplug() to scsi bus info.
The embody scsi bus can implement these two interfaces to signal the HBA driver
of guest kernel to add/remove the scsi device in question.

Signed-off-by: Cong Meng 
Signed-off-by: Sen Wang 
---
 hw/scsi-bus.c |   16 +++-
 hw/scsi.h |2 ++
 2 files changed, 17 insertions(+), 1 deletions(-)

diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index dbdb99c..f08900e 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -177,6 +177,10 @@ static int scsi_qdev_init(DeviceState *qdev)
  dev);
 }
 
+if (bus->info->hotplug) {
+bus->info->hotplug(bus, dev);
+}
+
 err:
 return rc;
 }
@@ -1539,6 +1543,16 @@ static int get_scsi_requests(QEMUFile *f, void *pv, 
size_t size)
 return 0;
 }
 
+static int scsi_qdev_unplug(DeviceState *qdev)
+{
+SCSIDevice *dev = SCSI_DEVICE(qdev);
+SCSIBus *bus = scsi_bus_from_device(dev);
+
+if (bus->info->hot_unplug)
+bus->info->hot_unplug(bus, dev);
+return qdev_simple_unplug_cb(qdev);
+}
+
 const VMStateInfo vmstate_info_scsi_requests = {
 .name = "scsi-requests",
 .get  = get_scsi_requests,
@@ -1575,7 +1589,7 @@ static void scsi_device_class_init(ObjectClass *klass, 
void *data)
 DeviceClass *k = DEVICE_CLASS(klass);
 k->bus_info = &scsi_bus_info;
 k->init = scsi_qdev_init;
-k->unplug   = qdev_simple_unplug_cb;
+k->unplug   = scsi_qdev_unplug;
 k->exit = scsi_qdev_exit;
 }
 
diff --git a/hw/scsi.h b/hw/scsi.h
index 2eb66f7..5768071 100644
--- a/hw/scsi.h
+++ b/hw/scsi.h
@@ -130,6 +130,8 @@ struct SCSIBusInfo {
 void (*transfer_data)(SCSIRequest *req, uint32_t arg);
 void (*complete)(SCSIRequest *req, uint32_t arg, size_t resid);
 void (*cancel)(SCSIRequest *req);
+void (*hotplug)(SCSIBus *bus, SCSIDevice *dev);
+void (*hot_unplug)(SCSIBus *bus, SCSIDevice *dev);
 QEMUSGList *(*get_sg_list)(SCSIRequest *req);
 
 void (*save_request)(QEMUFile *f, SCSIRequest *req);
-- 
1.7.7.6

Re: [Qemu-devel] [RFC] [PATCHv2 2/2] Adding basic calls to libseccomp in vl.c

[Avi Kivity]

On 06/19/2012 09:58 PM, Blue Swirl wrote:
>>> At least qemu-ifup/down scripts, migration exec and smbd have been
>>> mentioned. Only the system calls made by smbd (for some version of it)
>>> can be known. The user could specify arbitrary commands for the
>>> others, those could be assumed to use some common (large) subset of
>>> system calls but I think the security value would be close to zero
>>> then.
>>
>> We're not trying to protect against the user, but against the guest.  If
>> we assume the user wrote those scripts with care so they cannot be
>> exploited by the guest, then we are okay.
> 
> My concern was that first we could accidentally filter a system call
> that changes the script or executable behavior, much like sendmail +
> capabilities bug, and then a guest could trigger running this
> script/executable and exploit the changed behavior.

Ah, I see.  I agree this is dangerous.  We should probably disable exec
if we seccomp.

>>
>> We have decomposed qemu to some extent, in that privileged operations
>> happen in libvirt.  So the modes make sense - qemu has no idea whether a
>> privileged management system is controlling it or not.
> 
> So with -seccomp, libvirt could tell QEMU that for example open(),
> execve(), bind() and connect() will never be needed?

Yes.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH v6 11/16] target-or32: Add a IIS dummy board

[Wei-Ren Chen]

> + *  OpenRISC simulator for use as an ISS.
^^^
  Shoudld be IIS?

Regards,
chenwj

-- 
Wei-Ren Chen (陳韋任)
Computer Systems Lab, Institute of Information Science,
Academia Sinica, Taiwan (R.O.C.)
Tel:886-2-2788-3799 #1667
Homepage: http://people.cs.nctu.edu.tw/~chenwj

Re: [Qemu-devel] [PATCH v6 11/16] target-or32: Add a IIS dummy board

[Peter Crosthwaite]

On Thu, Jun 21, 2012 at 12:58 PM, Jia Liu  wrote:
> Add a IIS dummy board.
>
> Signed-off-by: Jia Liu 
> ---
>  hw/openrisc/Makefile.objs |    2 +-
>  hw/openrisc_sim.c         |  160 
> +
>  2 files changed, 161 insertions(+), 1 deletion(-)
>  create mode 100644 hw/openrisc_sim.c
>
> diff --git a/hw/openrisc/Makefile.objs b/hw/openrisc/Makefile.objs
> index 1c541a5..38ff8f5 100644
> --- a/hw/openrisc/Makefile.objs
> +++ b/hw/openrisc/Makefile.objs
> @@ -1,3 +1,3 @@
> -obj-y = openrisc_pic.o openrisc_timer.o
> +obj-y = openrisc_pic.o openrisc_sim.o openrisc_timer.o
>
>  obj-y := $(addprefix ../,$(obj-y))
> diff --git a/hw/openrisc_sim.c b/hw/openrisc_sim.c
> new file mode 100644
> index 000..892c67f
> --- /dev/null
> +++ b/hw/openrisc_sim.c
> @@ -0,0 +1,160 @@
> +/*
> + *  OpenRISC simulator for use as an ISS.
> + *
> + *  Copyright (c) 2011-2012 Jia Liu 
> + *                          Feng Gao 
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see 
> .
> + */
> +
> +#include "hw.h"
> +#include "openrisc_cpudev.h"
> +#include "boards.h"
> +#include "elf.h"
> +#include "pc.h"
> +#include "loader.h"
> +#include "exec-memory.h"
> +#include "sysemu.h"
> +#include "sysbus.h"
> +#include "qtest.h"
> +
> +#define KERNEL_LOAD_ADDR 0x100
> +
> +static struct _loaderparams {
> +    uint64_t ram_size;
> +    const char *kernel_filename;
> +    const char *kernel_cmdline;
> +    const char *initrd_filename;
> +} loaderparams;
> +
> +static void main_cpu_reset(void *opaque)
> +{
> +    CPUOpenRISCState *env = opaque;
> +    cpu_reset(ENV_GET_CPU(env));
> +}
> +
> +static void openrisc_sim_net_init(MemoryRegion *address_space,
> +                                  target_phys_addr_t base,
> +                                  target_phys_addr_t descriptors,
> +                                  qemu_irq irq, NICInfo *nd)
> +{
> +    DeviceState *dev;
> +    SysBusDevice *s;
> +
> +    dev = qdev_create(NULL, "open_eth");
> +    qdev_set_nic_properties(dev, nd);
> +    qdev_init_nofail(dev);
> +
> +    s = sysbus_from_qdev(dev);
> +    sysbus_connect_irq(s, 0, irq);
> +    memory_region_add_subregion(address_space, base,
> +                                sysbus_mmio_get_region(s, 0));
> +    memory_region_add_subregion(address_space, descriptors,
> +                                sysbus_mmio_get_region(s, 1));
> +}
> +
> +static uint64_t openrisc_load_kernel(void)
> +{
> +    long kernel_size;
> +    uint64_t elf_entry;
> +    target_phys_addr_t entry;
> +
> +    if (loaderparams.kernel_filename && !qtest_enabled()) {
> +        kernel_size = load_elf(loaderparams.kernel_filename, NULL, NULL,
> +                               &elf_entry, NULL, NULL, 1, ELF_MACHINE, 1);
> +        entry = elf_entry;
> +        if (kernel_size < 0) {
> +            kernel_size = load_uimage(loaderparams.kernel_filename,
> +                                      &entry, NULL, NULL);
> +        }
> +        if (kernel_size < 0) {
> +            kernel_size = load_image_targphys(loaderparams.kernel_filename,
> +                                              KERNEL_LOAD_ADDR,
> +                                              ram_size - KERNEL_LOAD_ADDR);
> +            entry = KERNEL_LOAD_ADDR;
> +        }
> +        if (kernel_size < 0) {
> +            fprintf(stderr, "qemu: could not load kernel '%s'\n",
> +                    loaderparams.kernel_filename);
> +            exit(1);
> +        }
> +
> +        if (kernel_size > 0) {
> +            return elf_entry;
> +        }

Hi Jia,

This seems a little weird. What happens here when it successfully
loads a uimage or raw image? It returns the elf_entry (probably == 0)
as the load address?

Regards,
Peter

> +    } else {
> +        entry = 0;
> +    }
> +
> +    return entry;
> +}
> +
> +static void openrisc_sim_init(ram_addr_t ram_size,
> +                              const char *boot_device,
> +                              const char *kernel_filename,
> +                              const char *kernel_cmdline,
> +                              const char *initrd_filename,
> +                              const char *cpu_model)
> +{
> +    CPUOpenRISCState *env;
> +    MemoryRegion *ram = g_new(MemoryRegion, 1);
> +
> +    if (!cpu_model) {
> +        cpu_model = "or1200";
> +    }
> +    env = cpu_init

Re: [Qemu-devel] [PATCH v6 11/16] target-or32: Add a IIS dummy board

[Max Filippov]

On Thu, Jun 21, 2012 at 12:19 PM, 陳韋任 (Wei-Ren Chen)
 wrote:
>> + *  OpenRISC simulator for use as an ISS.
>                                        ^^^
>  Shoudld be IIS?

I guess it stands for Instruction Set Simulator, so rather the subject
should be changed.

-- 
Thanks.
-- Max

Re: [Qemu-devel] [PATCH v6 11/16] target-or32: Add a IIS dummy board

[Jia Liu]

Hello Wei-Ren,

On Thu, Jun 21, 2012 at 4:19 PM, 陳韋任 (Wei-Ren Chen)
 wrote:
>> + *  OpenRISC simulator for use as an ISS.
>^^^
>  Shoudld be IIS?
>

Instruction Set Sim
Instruction Level Sim
What ever, I'll make it more clear :)

> Regards,
> chenwj
>
> --
> Wei-Ren Chen (陳韋任)
> Computer Systems Lab, Institute of Information Science,
> Academia Sinica, Taiwan (R.O.C.)
> Tel:886-2-2788-3799 #1667
> Homepage: http://people.cs.nctu.edu.tw/~chenwj

Regards,
Jia.

Re: [Qemu-devel] [PATCH 1/2 v2] scsi bus: introduce hotplug() and hot_unplug() interfaces for SCSI bus

[Stefan Hajnoczi]

On Thu, Jun 21, 2012 at 8:54 AM, Cong Meng  wrote:
> +static int scsi_qdev_unplug(DeviceState *qdev)
> +{
> +    SCSIDevice *dev = SCSI_DEVICE(qdev);
> +    SCSIBus *bus = scsi_bus_from_device(dev);
> +
> +    if (bus->info->hot_unplug)
> +        bus->info->hot_unplug(bus, dev);

Please use scripts/checkpatch.pl to ensure that your patch follows
QEMU coding style.  if statements must use {}.

Stefan

Re: [Qemu-devel] [PATCH 4/4] [wip] ehci: don't flush cache on dorbell rings.

[Gerd Hoffmann]

On 06/20/12 14:41, Gerd Hoffmann wrote:
> Commit 4be23939ab0d7019c7e59a37485b416fbbf0f073 makes ehci instantly
> zap any unlinked queue heads when the guest rings the dorbell.

[ ... ]

> Simply not zapping queue heads on doorbell rings fixes the issue, but of 
> course
> re-introduces the risk of using cached but stale information.

Improved version attached.

cheers,
  Gerd

>From 27c3356deb29f7cd5189aee8fd84058129812e28 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann 
Date: Wed, 20 Jun 2012 13:14:08 +0200
Subject: [PATCH v2] ehci: don't flush cache on dorbell rings.

Commit 4be23939ab0d7019c7e59a37485b416fbbf0f073 makes ehci instantly
zap any unlinked queue heads when the guest rings the dorbell.

While hacking up uas support this turned out to be a problem.  The linux
kernel can unlink and instantly relink the very same queue head, thereby
killing any async packets in flight.  That alone isn't an issue yet, the
packet will canceled and resubmitted and everything is fine.  We'll run
into trouble though in case the async packet is completed already, so we
can't cancel it any more.  The transaction is simply lost then.

usb_ehci_qh_ptrs q (nil) - QH @ 39c4f000: next 39c4f122 qtds 
,0001,39c5
usb_ehci_qh_fields QH @ 39c4f000 - rl 0, mplen 0, eps 0, ep 0, dev 0
usb_ehci_qh_ptrs q 0x7f95feba90a0 - QH @ 39c4f000: next 39c4f122 qtds 
,0001,39c5
usb_ehci_qh_fields QH @ 39c4f000 - rl 0, mplen 0, eps 0, ep 0, dev 0
usb_ehci_qh_ptrs q 0x7f95fe515210 - QH @ 39c4f120: next 39c4f0c2 qtds 
29dbce40,29dbc4e0,0009
usb_ehci_qh_fields QH @ 39c4f120 - rl 4, mplen 512, eps 2, ep 1, dev 2
usb_ehci_packet_action q 0x7f95fe515210 p 0x7f95fdec32a0: alloc
usb_packet_state_change bus 0, port 2, ep 1, packet 0x7f95fdec32e0, state undef 
-> setup
usb_ehci_packet_action q 0x7f95fe515210 p 0x7f95fdec32a0: process
usb_uas_command dev 2, tag 0x2, lun 0, lun64 -
scsi_req_parsed target 0 lun 0 tag 2 command 42 dir 2 length 16384
scsi_req_parsed_lba target 0 lun 0 tag 2 command 42 lba 5933312
scsi_req_alloc target 0 lun 0 tag 2
scsi_req_continue target 0 lun 0 tag 2
scsi_req_data target 0 lun 0 tag 2 len 16384
usb_uas_scsi_data dev 2, tag 0x2, bytes 16384
usb_uas_write_ready dev 2, tag 0x2
usb_packet_state_change bus 0, port 2, ep 1, packet 0x7f95fdec32e0, state setup 
-> complete
usb_ehci_packet_action q 0x7f95fe515210 p 0x7f95fdec32a0: free
usb_ehci_qh_ptrs q 0x7f95fdec3210 - QH @ 39c4f0c0: next 39c4f002 qtds 
29dbce40,0001,0009
usb_ehci_qh_fields QH @ 39c4f0c0 - rl 4, mplen 512, eps 2, ep 2, dev 2
usb_ehci_queue_action q 0x7f95fe5152a0: free
usb_packet_state_change bus 0, port 2, ep 2, packet 0x7f95feba9170, state async 
-> complete
^^^ async packets completes.
usb_ehci_packet_action q 0x7f95fdec3210 p 0x7f95feba9130: wakeup

usb_ehci_qh_ptrs q (nil) - QH @ 39c4f000: next 39c4f122 qtds 
,0001,39c5
usb_ehci_qh_fields QH @ 39c4f000 - rl 0, mplen 0, eps 0, ep 0, dev 0
usb_ehci_qh_ptrs q 0x7f95feba90a0 - QH @ 39c4f000: next 39c4f122 qtds 
,0001,39c5
usb_ehci_qh_fields QH @ 39c4f000 - rl 0, mplen 0, eps 0, ep 0, dev 0
usb_ehci_qh_ptrs q 0x7f95fe515210 - QH @ 39c4f120: next 39c4f002 qtds 
29dbc4e0,29dbc8a0,0009
usb_ehci_qh_fields QH @ 39c4f120 - rl 4, mplen 512, eps 2, ep 1, dev 2
usb_ehci_queue_action q 0x7f95fdec3210: free
usb_ehci_packet_action q 0x7f95fdec3210 p 0x7f95feba9130: free
^^^ endpoint #2 queue head removed from schedule, doorbell makes ehci zap the 
queue,
the (completed) usb packet is freed too and gets lost.

usb_ehci_qh_ptrs q (nil) - QH @ 39c4f000: next 39c4f0c2 qtds 
,0001,39c5
usb_ehci_qh_fields QH @ 39c4f000 - rl 0, mplen 0, eps 0, ep 0, dev 0
usb_ehci_qh_ptrs q 0x7f95feba90a0 - QH @ 39c4f000: next 39c4f0c2 qtds 
,0001,39c5
usb_ehci_qh_fields QH @ 39c4f000 - rl 0, mplen 0, eps 0, ep 0, dev 0
usb_ehci_queue_action q 0x7f9600dff570: alloc
usb_ehci_qh_ptrs q 0x7f9600dff570 - QH @ 39c4f0c0: next 39c4f122 qtds 
29dbce40,0001,0009
usb_ehci_qh_fields QH @ 39c4f0c0 - rl 4, mplen 512, eps 2, ep 2, dev 2
usb_ehci_packet_action q 0x7f9600dff570 p 0x7f95feba9130: alloc
usb_packet_state_change bus 0, port 2, ep 2, packet 0x7f95feba9170, state undef 
-> setup
usb_ehci_packet_action q 0x7f9600dff570 p 0x7f95feba9130: process
usb_packet_state_change bus 0, port 2, ep 2, packet 0x7f95feba9170, state setup 
-> async
usb_ehci_packet_action q 0x7f9600dff570 p 0x7f95feba9130: async
^^^ linux kernel relinked the queue head, ehci creates a new usb packet,
but we should have delivered the completed one instead.
usb_ehci_qh_ptrs q 0x7f95fe515210 - QH @ 39c4f120: next 39c4f002 qtds 
29dbc4e0,29dbc8a0,0009
usb_ehci_qh_fields QH @ 39c4f120 - rl 4, mplen 512, eps 2, ep 1, dev 2

So instead of instantly zapping the queue we'll set a flag that the
queue needs revalidation in case we'll see it again in the schedule.
ehci then checks that the queue head fields addressing / describing the
endpoint and the qtd p

Re: [Qemu-devel] [PATCH 1/5] target-i386: drop usage of prev_debug_excp_handler

[Igor Mammedov]

On 06/20/2012 03:28 PM, Jan Kiszka wrote:

On 2012-06-20 14:59, Igor Mammedov wrote:

Chain of exception handlers are currently unused feature, drop it
for now to avoid moving prev_debug_excp_handler variable at global
scope when moving tcg initialization into target-i386/cpu.c

Later we probably could re-invent better interface for this.

Signed-off-by: Igor Mammedov 
---
  target-i386/helper.c |7 +--
  1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/target-i386/helper.c b/target-i386/helper.c
index 2cc8097..b9384f6 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -941,8 +941,6 @@ int check_hw_breakpoints(CPUX86State *env, int 
force_dr6_update)
  return hit_enabled;
  }

-static CPUDebugExcpHandler *prev_debug_excp_handler;
-
  static void breakpoint_handler(CPUX86State *env)
  {
  CPUBreakpoint *bp;
@@ -965,8 +963,6 @@ static void breakpoint_handler(CPUX86State *env)
  break;
  }
  }
-if (prev_debug_excp_handler)
-prev_debug_excp_handler(env);
  }

  typedef struct MCEInjectionParams {
@@ -1166,8 +1162,7 @@ X86CPU *cpu_x86_init(const char *cpu_model)
  inited = 1;
  optimize_flags_init();
  #ifndef CONFIG_USER_ONLY
-prev_debug_excp_handler =
-cpu_set_debug_excp_handler(breakpoint_handler);
+cpu_set_debug_excp_handler(breakpoint_handler);
  #endif
  }
  if (cpu_x86_register(cpu, cpu_model) < 0) {



That's inconsistent. Let's remove this for all targets and drop the
return value of cpu_set_debug_excp_handler.

Jan


Thanks, I'll fix it.

--
-
 Igor

Re: [Qemu-devel] [PATCH 3/5] target-i386: call x86_cpu_realize() after APIC is initialized.

[Igor Mammedov]

On 06/20/2012 03:35 PM, Andreas Färber wrote:

Am 20.06.2012 14:59, schrieb Igor Mammedov:

It's not correct to make CPU runnable (i.e. calling x86_cpu_realize())
when not all properties are set (APIC in this case).

Fix it by calling x86_cpu_realize() at board level after APIC is
initialized, right before cpu_reset().

Signed-off-by: Igor Mammedov 
---
  hw/pc.c  |1 +
  target-i386/helper.c |2 --
  2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/pc.c b/hw/pc.c
index 8368701..8a662cf 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -948,6 +948,7 @@ static X86CPU *pc_new_cpu(const char *cpu_model)
  env->apic_state = apic_init(env, env->cpuid_apic_id);
  }
  qemu_register_reset(pc_cpu_reset, cpu);
+x86_cpu_realize(OBJECT(cpu), NULL);
  pc_cpu_reset(cpu);
  return cpu;
  }
diff --git a/target-i386/helper.c b/target-i386/helper.c
index c52ec13..b38ea7f 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -1161,8 +1161,6 @@ X86CPU *cpu_x86_init(const char *cpu_model)
  return NULL;
  }

-x86_cpu_realize(OBJECT(cpu), NULL);
-
  return cpu;
  }



This will require changes in linux-user and possibly bsd-user. Having a

Why it would require changes in linux-user? So far x86_cpu_realize() does
nothing useful in linux-user,  compiled and tested. It should be harmless
for linux-user not to execute it.
But I haven't compiled/tested bsd-user, do I need BSD for this?


cpu_realize() would probably help with avoiding #ifdef'ery.
Unfortunately deriving CPUState from DeviceState proves a bit difficult
in the meantime (it worked at one point, now there's lots of circular
header dependencies), and realize support for Object got stopped.

I'm in process of untangling this header mayhem (at least to a point
that allows compilation complete when CPU is derived from Device)



Andreas



--
-
 Igor

Re: [Qemu-devel] [PATCH] Remove support for non-threaded VNC server

[Michael Tokarev]

20.06.2012 16:24, Daniel P. Berrange wrote:

  delete mode 100644 ui/vnc-jobs-async.c
  delete mode 100644 ui/vnc-jobs-sync.c
  create mode 100644 ui/vnc-jobs.c


Is there a reason to rename vnc-jobs-foo.c to vnc-jobs.c ?
I'd leave it alone at this stage, omiting just the rename...

/mjt

Re: [Qemu-devel] [PATCH] Remove support for non-threaded VNC server

[Daniel P. Berrange]

On Thu, Jun 21, 2012 at 12:57:44PM +0300, Michael Tokarev wrote:
> 20.06.2012 16:24, Daniel P. Berrange wrote:
> >  delete mode 100644 ui/vnc-jobs-async.c
> >  delete mode 100644 ui/vnc-jobs-sync.c
> >  create mode 100644 ui/vnc-jobs.c
> 
> Is there a reason to rename vnc-jobs-foo.c to vnc-jobs.c ?

The corresponding header file is vnc-jobs.h, and since we only
have one impl of it now, using the same name is normal practice.

> I'd leave it alone at this stage, omiting just the rename...

I disagree, GIT handles renames like this fine, so there's no
reason not to do this.

Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH 4/4] [wip] ehci: don't flush cache on dorbell rings.

[Peter Maydell]

On 20 June 2012 13:41, Gerd Hoffmann  wrote:
> Commit 4be23939ab0d7019c7e59a37485b416fbbf0f073 makes ehci instantly
> zap any unlinked queue heads when the guest rings the dorbell.

Should be "doorbell" here and in the Subject, worth fixing up when
you do a next version / pullreq.

-- PMM

Re: [Qemu-devel] [PATCH 3/5] target-i386: call x86_cpu_realize() after APIC is initialized.

[Andreas Färber]

Am 21.06.2012 11:43, schrieb Igor Mammedov:
> On 06/20/2012 03:35 PM, Andreas Färber wrote:
>> Am 20.06.2012 14:59, schrieb Igor Mammedov:
>>> It's not correct to make CPU runnable (i.e. calling x86_cpu_realize())
>>> when not all properties are set (APIC in this case).
>>>
>>> Fix it by calling x86_cpu_realize() at board level after APIC is
>>> initialized, right before cpu_reset().
>>>
>>> Signed-off-by: Igor Mammedov 
>>> ---
>>>   hw/pc.c  |1 +
>>>   target-i386/helper.c |2 --
>>>   2 files changed, 1 insertion(+), 2 deletions(-)
>>>
>>> diff --git a/hw/pc.c b/hw/pc.c
>>> index 8368701..8a662cf 100644
>>> --- a/hw/pc.c
>>> +++ b/hw/pc.c
>>> @@ -948,6 +948,7 @@ static X86CPU *pc_new_cpu(const char *cpu_model)
>>>   env->apic_state = apic_init(env, env->cpuid_apic_id);
>>>   }
>>>   qemu_register_reset(pc_cpu_reset, cpu);
>>> +x86_cpu_realize(OBJECT(cpu), NULL);
>>>   pc_cpu_reset(cpu);
>>>   return cpu;
>>>   }
>>> diff --git a/target-i386/helper.c b/target-i386/helper.c
>>> index c52ec13..b38ea7f 100644
>>> --- a/target-i386/helper.c
>>> +++ b/target-i386/helper.c
>>> @@ -1161,8 +1161,6 @@ X86CPU *cpu_x86_init(const char *cpu_model)
>>>   return NULL;
>>>   }
>>>
>>> -x86_cpu_realize(OBJECT(cpu), NULL);
>>> -
>>>   return cpu;
>>>   }
>>>
>>
>> This will require changes in linux-user and possibly bsd-user. Having a
> Why it would require changes in linux-user? So far x86_cpu_realize() does
> nothing useful in linux-user,  compiled and tested. It should be harmless
> for linux-user not to execute it.

Hm, I'd need to recheck...

> But I haven't compiled/tested bsd-user, do I need BSD for this?

Yes, you do. But if it's not needed for linux-user then it shouldn't be
needed for bsd-user either.

>> cpu_realize() would probably help with avoiding #ifdef'ery.
>> Unfortunately deriving CPUState from DeviceState proves a bit difficult
>> in the meantime (it worked at one point, now there's lots of circular
>> header dependencies), and realize support for Object got stopped.

> I'm in process of untangling this header mayhem (at least to a point
> that allows compilation complete when CPU is derived from Device)

So am I... A few weeks ago my qom-cpu-dev branch on GitHub used to
compile, now something has changed and I needed to take cpu.h out of
qemu-common.h and move lots of, e.g., ARM devices into libhw to avoid
cpu.h dependencies and add cpu.h includes elsewhere. Would be good to
coordinate that, are you on IRC later today?

Andreas

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

Re: [Qemu-devel] [PATCH] Remove support for non-threaded VNC server

[Gerd Hoffmann]

On 06/21/12 11:59, Daniel P. Berrange wrote:
> On Thu, Jun 21, 2012 at 12:57:44PM +0300, Michael Tokarev wrote:
>> 20.06.2012 16:24, Daniel P. Berrange wrote:
>>>  delete mode 100644 ui/vnc-jobs-async.c
>>>  delete mode 100644 ui/vnc-jobs-sync.c
>>>  create mode 100644 ui/vnc-jobs.c
>>
>> Is there a reason to rename vnc-jobs-foo.c to vnc-jobs.c ?
> 
> The corresponding header file is vnc-jobs.h, and since we only
> have one impl of it now, using the same name is normal practice.
> 
>> I'd leave it alone at this stage, omiting just the rename...
> 
> I disagree, GIT handles renames like this fine, so there's no
> reason not to do this.

/me suggests "git format-patch -M" so the patch shows the rename.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH v6 08/16] target-or32: Add instruction tanslation

[Max Filippov]

On Thu, Jun 21, 2012 at 6:58 AM, Jia Liu  wrote:
> Add OpenRISC instruction tanslation routines.
>
> Signed-off-by: Jia Liu 

[...]

> +    case 0x0009:
> +        switch (op1) {
> +        case 0x03:   /*l.div*/
> +            LOG_DIS("l.div r%d, r%d, r%d\n", rd, ra, rb);
> +            {
> +                int lab0 = gen_new_label();
> +                int lab1 = gen_new_label();
> +                int lab2 = gen_new_label();
> +                TCGv_i32 sr_ove = tcg_temp_local_new_i32();
> +                if (rb == 0) {
> +                    tcg_gen_ori_tl(cpu_sr, cpu_sr, (SR_OV | SR_CY));
> +                    tcg_gen_brcondi_tl(TCG_COND_NE, sr_ove, SR_OVE, lab0);
> +                    tcg_gen_andi_tl(sr_ove, cpu_sr, SR_OVE);
> +                    gen_exception(dc, EXCP_RANGE);
> +                    gen_set_label(lab0);
> +                } else {
> +                    tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_R[rb],
> +                                       0x, lab1);
> +                    tcg_gen_brcondi_tl(TCG_COND_NE, cpu_R[ra],
> +                                       0x, lab2);
> +                    tcg_gen_brcondi_tl(TCG_COND_NE, cpu_R[rb],
> +                                       0x8000, lab2);
> +                    gen_set_label(lab1);
> +                    tcg_gen_ori_tl(cpu_sr, cpu_sr, (SR_OV | SR_CY));
> +                    tcg_gen_andi_tl(sr_ove, cpu_sr, SR_OVE);
> +                    tcg_gen_brcondi_tl(TCG_COND_NE, sr_ove, SR_OVE, lab2);

Causes host division by zero/overflow. I'd suggest to brcond to lab3 set after
the final tcg_gen_div.

> +                    gen_exception(dc, EXCP_RANGE);
> +                    gen_set_label(lab2);
> +                    tcg_gen_div_tl(cpu_R[rd], cpu_R[ra], cpu_R[rb]);
> +                }
> +                tcg_temp_free_i32(sr_ove);
> +            }
> +            break;
> +
> +        default:
> +            gen_illegal_exception(dc);
> +            break;
> +        }
> +        break;
> +
> +    case 0x000a:
> +        switch (op1) {
> +        case 0x03:   /*l.divu*/
> +            LOG_DIS("l.divu r%d, r%d, r%d\n", rd, ra, rb);
> +            {
> +                int lab0 = gen_new_label();
> +                int lab1 = gen_new_label();
> +                TCGv_i32 sr_ove = tcg_temp_local_new_i32();
> +                if (rb == 0) {
> +                    tcg_gen_ori_tl(cpu_sr, cpu_sr, (SR_OV | SR_CY));
> +                    tcg_gen_andi_tl(sr_ove, cpu_sr, SR_OVE);
> +                    tcg_gen_brcondi_tl(TCG_COND_NE, sr_ove, SR_OVE, lab0);
> +                    gen_exception(dc, EXCP_RANGE);
> +                    gen_set_label(lab0);
> +                } else {
> +                    tcg_gen_brcondi_tl(TCG_COND_NE, cpu_R[rb],
> +                                       0x, lab1);
> +                    tcg_gen_ori_tl(cpu_sr, cpu_sr, (SR_OV | SR_CY));
> +                    tcg_gen_andi_tl(sr_ove, cpu_sr, SR_OVE);
> +                    tcg_gen_brcondi_tl(TCG_COND_NE, sr_ove, SR_OVE, lab1);

Ditto.

> +                    gen_exception(dc, EXCP_RANGE);
> +                    gen_set_label(lab1);
> +                    tcg_gen_divu_tl(cpu_R[rd], cpu_R[ra], cpu_R[rb]);
> +                }
> +                tcg_temp_free_i32(sr_ove);
> +            }
> +            break;

[...]

> +    case 0x000e:
> +        switch (op1) {
> +        case 0x00:   /*l.cmov*/
> +            LOG_DIS("l.cmov r%d, r%d, r%d\n", rd, ra, rb);
> +            {
> +                int lab = gen_new_label();
> +                TCGv res = tcg_temp_new();

Need to be temp_local to survive brcond.

> +                TCGv sr_f = tcg_temp_new();
> +                tcg_gen_andi_tl(sr_f, cpu_sr, SR_F);
> +                tcg_gen_mov_tl(res, cpu_R[rb]);
> +                tcg_gen_brcondi_tl(TCG_COND_NE, sr_f, SR_F, lab);
> +                tcg_gen_mov_tl(res, cpu_R[ra]);
> +                gen_set_label(lab);
> +                tcg_gen_mov_tl(cpu_R[rd], res);
> +                tcg_temp_free(sr_f);
> +                tcg_temp_free(res);
> +            }
> +            break;

[...]

-- 
Thanks.
-- Max

Re: [Qemu-devel] [PATCH 4/4] [wip] ehci: don't flush cache on dorbell rings.

[Andreas Färber]

Am 21.06.2012 12:04, schrieb Peter Maydell:
> On 20 June 2012 13:41, Gerd Hoffmann  wrote:
>> Commit 4be23939ab0d7019c7e59a37485b416fbbf0f073 makes ehci instantly
>> zap any unlinked queue heads when the guest rings the dorbell.
> 
> Should be "doorbell" here and in the Subject, worth fixing up when
> you do a next version / pullreq.

Maybe Dor has some special bell as ringtone. :)

Andreas

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

Re: [Qemu-devel] [PATCH] msi/msix: added public API to set/get MSI message address, and data

[Alexey Kardashevskiy]

On 21/06/12 17:39, Jan Kiszka wrote:
> On 2012-06-21 09:18, Alexey Kardashevskiy wrote:
>>
>> agrhhh. sha1 of the patch changed after rebasing :)
>>
>>
>>
>> Added (msi|msix)_(set|get)_message() function for whoever might
>> want to use them.
>>
>> Currently msi_notify()/msix_notify() write to these vectors to
>> signal the guest about an interrupt so the correct values have to
>> written there by the guest or QEMU.
>>
>> For example, POWER guest never initializes MSI/MSIX vectors, instead
>> it uses RTAS hypercalls. So in order to support MSIX for virtio-pci on
>> POWER we have to initialize MSI/MSIX message from QEMU.
>>
>> As only set* function are required by now, the "get" functions were added
>> or made public for a symmetry.
>>
>> Signed-off-by: Alexey Kardashevskiy 
>> ---
>>  hw/msi.c  |   29 +
>>  hw/msi.h  |2 ++
>>  hw/msix.c |   11 ++-
>>  hw/msix.h |3 +++
>>  4 files changed, 44 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/msi.c b/hw/msi.c
>> index 5233204..9ad84a4 100644
>> --- a/hw/msi.c
>> +++ b/hw/msi.c
>> @@ -105,6 +105,35 @@ static inline uint8_t msi_pending_off(const PCIDevice* 
>> dev, bool msi64bit)
>>  return dev->msi_cap + (msi64bit ? PCI_MSI_PENDING_64 : 
>> PCI_MSI_PENDING_32);
>>  }
>>  
>> +MSIMessage msi_get_message(PCIDevice *dev)
> 
> MSIMessage msi_get_message(PCIDevice *dev, unsigned vector)


Who/how/why is going to calculate the vector here?

> 
>> +{
>> +uint16_t flags = pci_get_word(dev->config + msi_flags_off(dev));
>> +bool msi64bit = flags & PCI_MSI_FLAGS_64BIT;
>> +MSIMessage msg;
>> +
>> +if (msi64bit) {
>> +msg.address = pci_get_quad(dev->config + msi_address_lo_off(dev));
>> +} else {
>> +msg.address = pci_get_long(dev->config + msi_address_lo_off(dev));
>> +}
>> +msg.data = pci_get_word(dev->config + msi_data_off(dev, msi64bit));
> 
> And I have this here in addition:
> 
> unsigned int nr_vectors = msi_nr_vectors(flags);
> ...
> 
> if (nr_vectors > 1) {
> msg.data &= ~(nr_vectors - 1);
> msg.data |= vector;
> }
> 
> See PCI spec and existing code.


What for? I really do not get it why someone might want to read something but 
not real value.
What PCI code should I look?


> 
>> +
>> +return msg;
>> +}
>> +
>> +void msi_set_message(PCIDevice *dev, MSIMessage msg)
>> +{
>> +uint16_t flags = pci_get_word(dev->config + msi_flags_off(dev));
>> +bool msi64bit = flags & PCI_MSI_FLAGS_64BIT;
>> +
>> +if (msi64bit) {
>> +pci_set_quad(dev->config + msi_address_lo_off(dev), msg.address);
>> +} else {
>> +pci_set_long(dev->config + msi_address_lo_off(dev), msg.address);
>> +}
>> +pci_set_word(dev->config + msi_data_off(dev, msi64bit), msg.data);
>> +}
>> +
>>  bool msi_enabled(const PCIDevice *dev)
>>  {
>>  return msi_present(dev) &&
>> diff --git a/hw/msi.h b/hw/msi.h
>> index 75747ab..4b0f4f8 100644
>> --- a/hw/msi.h
>> +++ b/hw/msi.h
>> @@ -31,6 +31,8 @@ struct MSIMessage {
>>  
>>  extern bool msi_supported;
>>  
>> +MSIMessage msi_get_message(PCIDevice *dev);
>> +void msi_set_message(PCIDevice *dev, MSIMessage msg);
>>  bool msi_enabled(const PCIDevice *dev);
>>  int msi_init(struct PCIDevice *dev, uint8_t offset,
>>   unsigned int nr_vectors, bool msi64bit, bool 
>> msi_per_vector_mask);
>> diff --git a/hw/msix.c b/hw/msix.c
>> index ded3c55..9e8d8bb 100644
>> --- a/hw/msix.c
>> +++ b/hw/msix.c
>> @@ -35,7 +35,7 @@
>>  #define MSIX_PAGE_PENDING (MSIX_PAGE_SIZE / 2)
>>  #define MSIX_MAX_ENTRIES 32
>>  
>> -static MSIMessage msix_get_message(PCIDevice *dev, unsigned vector)
>> +MSIMessage msix_get_message(PCIDevice *dev, unsigned vector)
>>  {
>>  uint8_t *table_entry = dev->msix_table_page + vector * 
>> PCI_MSIX_ENTRY_SIZE;
>>  MSIMessage msg;
>> @@ -45,6 +45,15 @@ static MSIMessage msix_get_message(PCIDevice *dev, 
>> unsigned vector)
>>  return msg;
>>  }
>>  
>> +void msix_set_message(PCIDevice *dev, int vector, struct MSIMessage msg)
>> +{
>> +uint8_t *table_entry = dev->msix_table_page + vector * 
>> PCI_MSIX_ENTRY_SIZE;
>> +
>> +pci_set_quad(table_entry + PCI_MSIX_ENTRY_LOWER_ADDR, msg.address);
>> +pci_set_long(table_entry + PCI_MSIX_ENTRY_DATA, msg.data);
>> +table_entry[PCI_MSIX_ENTRY_VECTOR_CTRL] &= ~PCI_MSIX_ENTRY_CTRL_MASKBIT;
>> +}
>> +
>>  /* Add MSI-X capability to the config space for the device. */
>>  /* Given a bar and its size, add MSI-X table on top of it
>>   * and fill MSI-X capability in the config space.
>> diff --git a/hw/msix.h b/hw/msix.h
>> index 50aee82..3374cf8 100644
>> --- a/hw/msix.h
>> +++ b/hw/msix.h
>> @@ -4,6 +4,9 @@
>>  #include "qemu-common.h"
>>  #include "pci.h"
>>  
>> +MSIMessage msix_get_message(PCIDevice *dev, unsigned vector);
>> +void msix_set_message(PCIDevice *dev, int vector, MSIMessage msg);
>> +
>>  int msix_init(PCIDevice *pdev, unsigned short nentries,
>>MemoryRegion *bar,
>>   

Re: [Qemu-devel] hw/Makefile.objs question

[Andreas Färber]

Am 21.06.2012 05:22, schrieb Alexey Kardashevskiy:
> I am trying to compile the very last qemu with vfio_pci enabled. VFIO_PCI is 
> added as below:
> 
> ./configure:
> 
>  case "$target_arch2" in
>   i386|x86_64|ppc64)
>  if test "$vfio_pci" = "yes" -a "$target_softmmu" = "yes" ; then
>echo "CONFIG_VFIO_PCI=y" >> $config_target_mak
>  fi
>  esac
> 
> 
> ./Makefile.target:
> 
>  # VFIO PCI device assignment
> obj-$(CONFIG_VFIO_PCI) += vfio_pci.o
> 
> 
> And it worked before. However it does not anymore as it seems that everything 
> in hw/ (and vfio_pci.c
> as well as is in hw/ and it is a device) can be only compiled via 
> hw/Makefile.objs and
> hw/ppc/Makefile.objs (my platform is POWER), it is ignored if to keep it as 
> is.
> 
> So I have to move "obj-$(CONFIG_VFIO_PCI) += vfio_pci.o" to hw/Makefile.objs 
> (and change obj- to
> hw-obj-) but the hw/Makefile.objs does not include (directly or indirectly) 
> generated
> ppc64-softmmu/config-target.mak with CONFIG_VFIO_PCI=y.
> 
> What is the correct solution?

If the file compiles the same for all three, put CONFIG_VFIO_PCI=y into
default-configs/{i386,x86_64,ppc64}-softmmu.mak and do
hw-obj-$(CONFIG_VFIO_PCI) += in hw/Makefile.objs.

Otherwise, add to hw/{i386,ppc}/Makefile.objs - or with Anthony's
proposal from yesterday hw/Makefile.objs becomes possible, too.

Andreas

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

Re: [Qemu-devel] [PATCH] msi/msix: added public API to set/get MSI message address, and data

[Jan Kiszka]

On 2012-06-21 12:28, Alexey Kardashevskiy wrote:
> On 21/06/12 17:39, Jan Kiszka wrote:
>> On 2012-06-21 09:18, Alexey Kardashevskiy wrote:
>>>
>>> agrhhh. sha1 of the patch changed after rebasing :)
>>>
>>>
>>>
>>> Added (msi|msix)_(set|get)_message() function for whoever might
>>> want to use them.
>>>
>>> Currently msi_notify()/msix_notify() write to these vectors to
>>> signal the guest about an interrupt so the correct values have to
>>> written there by the guest or QEMU.
>>>
>>> For example, POWER guest never initializes MSI/MSIX vectors, instead
>>> it uses RTAS hypercalls. So in order to support MSIX for virtio-pci on
>>> POWER we have to initialize MSI/MSIX message from QEMU.
>>>
>>> As only set* function are required by now, the "get" functions were added
>>> or made public for a symmetry.
>>>
>>> Signed-off-by: Alexey Kardashevskiy 
>>> ---
>>>  hw/msi.c  |   29 +
>>>  hw/msi.h  |2 ++
>>>  hw/msix.c |   11 ++-
>>>  hw/msix.h |3 +++
>>>  4 files changed, 44 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/hw/msi.c b/hw/msi.c
>>> index 5233204..9ad84a4 100644
>>> --- a/hw/msi.c
>>> +++ b/hw/msi.c
>>> @@ -105,6 +105,35 @@ static inline uint8_t msi_pending_off(const PCIDevice* 
>>> dev, bool msi64bit)
>>>  return dev->msi_cap + (msi64bit ? PCI_MSI_PENDING_64 : 
>>> PCI_MSI_PENDING_32);
>>>  }
>>>  
>>> +MSIMessage msi_get_message(PCIDevice *dev)
>>
>> MSIMessage msi_get_message(PCIDevice *dev, unsigned vector)
> 
> 
> Who/how/why is going to calculate the vector here?
> 
>>
>>> +{
>>> +uint16_t flags = pci_get_word(dev->config + msi_flags_off(dev));
>>> +bool msi64bit = flags & PCI_MSI_FLAGS_64BIT;
>>> +MSIMessage msg;
>>> +
>>> +if (msi64bit) {
>>> +msg.address = pci_get_quad(dev->config + msi_address_lo_off(dev));
>>> +} else {
>>> +msg.address = pci_get_long(dev->config + msi_address_lo_off(dev));
>>> +}
>>> +msg.data = pci_get_word(dev->config + msi_data_off(dev, msi64bit));
>>
>> And I have this here in addition:
>>
>> unsigned int nr_vectors = msi_nr_vectors(flags);
>> ...
>>
>> if (nr_vectors > 1) {
>> msg.data &= ~(nr_vectors - 1);
>> msg.data |= vector;
>> }
>>
>> See PCI spec and existing code.
> 
> 
> What for? I really do not get it why someone might want to read something but 
> not real value.
> What PCI code should I look?

I'm not sure what your use case for reading the message is. For KVM
device assignment it is preparing an alternative message delivery path
for MSI vectors. And for this we will need vector notifier support for
MSI as well. You can check the MSI-X code for corresponding use cases of
msix_get_message.

And when we already have msi_get_message, another logical use case is
msi_notify. See msix.c again.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH] msi/msix: added public API to set/get MSI message address, and data

[Alexey Kardashevskiy]

On 21/06/12 20:38, Jan Kiszka wrote:
> On 2012-06-21 12:28, Alexey Kardashevskiy wrote:
>> On 21/06/12 17:39, Jan Kiszka wrote:
>>> On 2012-06-21 09:18, Alexey Kardashevskiy wrote:

 agrhhh. sha1 of the patch changed after rebasing :)



 Added (msi|msix)_(set|get)_message() function for whoever might
 want to use them.

 Currently msi_notify()/msix_notify() write to these vectors to
 signal the guest about an interrupt so the correct values have to
 written there by the guest or QEMU.

 For example, POWER guest never initializes MSI/MSIX vectors, instead
 it uses RTAS hypercalls. So in order to support MSIX for virtio-pci on
 POWER we have to initialize MSI/MSIX message from QEMU.

 As only set* function are required by now, the "get" functions were added
 or made public for a symmetry.

 Signed-off-by: Alexey Kardashevskiy 
 ---
  hw/msi.c  |   29 +
  hw/msi.h  |2 ++
  hw/msix.c |   11 ++-
  hw/msix.h |3 +++
  4 files changed, 44 insertions(+), 1 deletion(-)

 diff --git a/hw/msi.c b/hw/msi.c
 index 5233204..9ad84a4 100644
 --- a/hw/msi.c
 +++ b/hw/msi.c
 @@ -105,6 +105,35 @@ static inline uint8_t msi_pending_off(const 
 PCIDevice* dev, bool msi64bit)
  return dev->msi_cap + (msi64bit ? PCI_MSI_PENDING_64 : 
 PCI_MSI_PENDING_32);
  }
  
 +MSIMessage msi_get_message(PCIDevice *dev)
>>>
>>> MSIMessage msi_get_message(PCIDevice *dev, unsigned vector)
>>
>>
>> Who/how/why is going to calculate the vector here?
>>
>>>
 +{
 +uint16_t flags = pci_get_word(dev->config + msi_flags_off(dev));
 +bool msi64bit = flags & PCI_MSI_FLAGS_64BIT;
 +MSIMessage msg;
 +
 +if (msi64bit) {
 +msg.address = pci_get_quad(dev->config + msi_address_lo_off(dev));
 +} else {
 +msg.address = pci_get_long(dev->config + msi_address_lo_off(dev));
 +}
 +msg.data = pci_get_word(dev->config + msi_data_off(dev, msi64bit));
>>>
>>> And I have this here in addition:
>>>
>>> unsigned int nr_vectors = msi_nr_vectors(flags);
>>> ...
>>>
>>> if (nr_vectors > 1) {
>>> msg.data &= ~(nr_vectors - 1);
>>> msg.data |= vector;
>>> }
>>>
>>> See PCI spec and existing code.
>>
>>
>> What for? I really do not get it why someone might want to read something 
>> but not real value.
>> What PCI code should I look?
> 
> I'm not sure what your use case for reading the message is. For KVM
> device assignment it is preparing an alternative message delivery path
> for MSI vectors. And for this we will need vector notifier support for
> MSI as well. You can check the MSI-X code for corresponding use cases of
> msix_get_message.

> And when we already have msi_get_message, another logical use case is
> msi_notify. See msix.c again.

Aaaa.

I have no case for reading the message. All I need is writing. And I want it 
public as I want to use
it from hw/spapr_pci.c. You suggested to add reading, I added "get" to be 
_symmetric_ to "set"
("get" returns what "set" wrote). You want a different thing which I can do but 
it is not
msi_get_message(), it is something like msi_prepare_message(MSImessage msg) or
msi_set_vector(uint16_t data) or simply internal kitchen of msi_notify().

Still can do what you suggested, it just does not seem right.


-- 
Alexey

Re: [Qemu-devel] [PATCH 2/2] virtio-scsi: Implement hotplug support for virtio-scsi

[Stefan Hajnoczi]

On Wed, Jun 20, 2012 at 7:47 AM, Cong Meng  wrote:
> Implement the hotplug() and hot_unplug() interfaces in virtio-scsi, by signal
> the virtio_scsi.ko in guest kernel via event virtual queue.
>
> The counterpart patch of virtio_scsi.ko will be sent soon in another thread.
>
> Signed-off-by: Cong Meng 
> Signed-off-by: Sen Wang 
> ---
>  hw/virtio-scsi.c |   72 +++--
>  1 files changed, 69 insertions(+), 3 deletions(-)

I compared against the virtio-scsi specification and this looks good:
http://ozlabs.org/~rusty/virtio-spec/virtio-0.9.5.pdf

Dropped events and event throttling are not implemented by this patch.
 This means that the guest can miss events if it runs out of event
queue elements.  A scenario that might be able to trigger this is if
multiple LUNs are hotplugged in a single QEMU monitor callback.

Implementing dropped events is easy in hw/virtio-scsi.c.  Keep a bool
or counter of dropped events and report them when the guest kicks us
with a free event element (virtio_scsi_handle_event).

Stefan

Re: [Qemu-devel] [PATCH] msi/msix: added public API to set/get MSI message address, and data

[Jan Kiszka]

On 2012-06-21 12:50, Alexey Kardashevskiy wrote:
> On 21/06/12 20:38, Jan Kiszka wrote:
>> On 2012-06-21 12:28, Alexey Kardashevskiy wrote:
>>> On 21/06/12 17:39, Jan Kiszka wrote:
 On 2012-06-21 09:18, Alexey Kardashevskiy wrote:
>
> agrhhh. sha1 of the patch changed after rebasing :)
>
>
>
> Added (msi|msix)_(set|get)_message() function for whoever might
> want to use them.
>
> Currently msi_notify()/msix_notify() write to these vectors to
> signal the guest about an interrupt so the correct values have to
> written there by the guest or QEMU.
>
> For example, POWER guest never initializes MSI/MSIX vectors, instead
> it uses RTAS hypercalls. So in order to support MSIX for virtio-pci on
> POWER we have to initialize MSI/MSIX message from QEMU.
>
> As only set* function are required by now, the "get" functions were added
> or made public for a symmetry.
>
> Signed-off-by: Alexey Kardashevskiy 
> ---
>  hw/msi.c  |   29 +
>  hw/msi.h  |2 ++
>  hw/msix.c |   11 ++-
>  hw/msix.h |3 +++
>  4 files changed, 44 insertions(+), 1 deletion(-)
>
> diff --git a/hw/msi.c b/hw/msi.c
> index 5233204..9ad84a4 100644
> --- a/hw/msi.c
> +++ b/hw/msi.c
> @@ -105,6 +105,35 @@ static inline uint8_t msi_pending_off(const 
> PCIDevice* dev, bool msi64bit)
>  return dev->msi_cap + (msi64bit ? PCI_MSI_PENDING_64 : 
> PCI_MSI_PENDING_32);
>  }
>  
> +MSIMessage msi_get_message(PCIDevice *dev)

 MSIMessage msi_get_message(PCIDevice *dev, unsigned vector)
>>>
>>>
>>> Who/how/why is going to calculate the vector here?
>>>

> +{
> +uint16_t flags = pci_get_word(dev->config + msi_flags_off(dev));
> +bool msi64bit = flags & PCI_MSI_FLAGS_64BIT;
> +MSIMessage msg;
> +
> +if (msi64bit) {
> +msg.address = pci_get_quad(dev->config + 
> msi_address_lo_off(dev));
> +} else {
> +msg.address = pci_get_long(dev->config + 
> msi_address_lo_off(dev));
> +}
> +msg.data = pci_get_word(dev->config + msi_data_off(dev, msi64bit));

 And I have this here in addition:

 unsigned int nr_vectors = msi_nr_vectors(flags);
 ...

 if (nr_vectors > 1) {
 msg.data &= ~(nr_vectors - 1);
 msg.data |= vector;
 }

 See PCI spec and existing code.
>>>
>>>
>>> What for? I really do not get it why someone might want to read something 
>>> but not real value.
>>> What PCI code should I look?
>>
>> I'm not sure what your use case for reading the message is. For KVM
>> device assignment it is preparing an alternative message delivery path
>> for MSI vectors. And for this we will need vector notifier support for
>> MSI as well. You can check the MSI-X code for corresponding use cases of
>> msix_get_message.
> 
>> And when we already have msi_get_message, another logical use case is
>> msi_notify. See msix.c again.
> 
> Aaaa.
> 
> I have no case for reading the message. All I need is writing. And I want it 
> public as I want to use
> it from hw/spapr_pci.c. You suggested to add reading, I added "get" to be 
> _symmetric_ to "set"
> ("get" returns what "set" wrote). You want a different thing which I can do 
> but it is not
> msi_get_message(), it is something like msi_prepare_message(MSImessage msg) or
> msi_set_vector(uint16_t data) or simply internal kitchen of msi_notify().
> 
> Still can do what you suggested, it just does not seem right.

It is right - when looking at it from a different angle. ;)

I don't mind if you add msi_get_message now or leave this to me. Likely
the latter is better as you have no use case for msi_get_message (and
also msix_get_message!) outside of their modules, thus we should not
export those functions anyway.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] hw/Makefile.objs question

[Alexey Kardashevskiy]

On 21/06/12 20:36, Andreas Färber wrote:
> Am 21.06.2012 05:22, schrieb Alexey Kardashevskiy:
>> I am trying to compile the very last qemu with vfio_pci enabled. VFIO_PCI is 
>> added as below:
>>
>> ./configure:
>>
>>  case "$target_arch2" in
>>   i386|x86_64|ppc64)
>>  if test "$vfio_pci" = "yes" -a "$target_softmmu" = "yes" ; then
>>echo "CONFIG_VFIO_PCI=y" >> $config_target_mak
>>  fi
>>  esac
>>
>>
>> ./Makefile.target:
>>
>>  # VFIO PCI device assignment
>> obj-$(CONFIG_VFIO_PCI) += vfio_pci.o
>>
>>
>> And it worked before. However it does not anymore as it seems that 
>> everything in hw/ (and vfio_pci.c
>> as well as is in hw/ and it is a device) can be only compiled via 
>> hw/Makefile.objs and
>> hw/ppc/Makefile.objs (my platform is POWER), it is ignored if to keep it as 
>> is.
>>
>> So I have to move "obj-$(CONFIG_VFIO_PCI) += vfio_pci.o" to hw/Makefile.objs 
>> (and change obj- to
>> hw-obj-) but the hw/Makefile.objs does not include (directly or indirectly) 
>> generated
>> ppc64-softmmu/config-target.mak with CONFIG_VFIO_PCI=y.
>>
>> What is the correct solution?
> 
> If the file compiles the same for all three, put CONFIG_VFIO_PCI=y into
> default-configs/{i386,x86_64,ppc64}-softmmu.mak and do
> hw-obj-$(CONFIG_VFIO_PCI) += in hw/Makefile.objs.


It only compiles with ./configure --enable-vfio-pci which may or may not set 
CONFIG_VFIO_PCI to "y".
Your proposal makes it always "y" (for selected platforms).


> Otherwise, add to hw/{i386,ppc}/Makefile.objs - or with Anthony's
> proposal from yesterday hw/Makefile.objs becomes possible, too.

Again, it will be unconditional "y".



-- 
Alexey

Re: [Qemu-devel] [PATCH 4/4] [wip] ehci: don't flush cache on dorbell rings.

[Dor Laor]

On 06/21/2012 01:27 PM, Andreas Färber wrote:

Am 21.06.2012 12:04, schrieb Peter Maydell:

On 20 June 2012 13:41, Gerd Hoffmann  wrote:

Commit 4be23939ab0d7019c7e59a37485b416fbbf0f073 makes ehci instantly
zap any unlinked queue heads when the guest rings the dorbell.


Should be "doorbell" here and in the Subject, worth fixing up when
you do a next version / pullreq.


Maybe Dor has some special bell as ringtone. :)


Luckily Bell is not my last name :)


Andreas

[Qemu-devel] [PATCH] msi/msix: added API to set MSI message address and data

[Alexey Kardashevskiy]

Added (msi|msix)_set_message() functions.

Currently msi_notify()/msix_notify() write to these vectors to
signal the guest about an interrupt so the correct values have to
written there by the guest or QEMU.

For example, POWER guest never initializes MSI/MSIX vectors, instead
it uses RTAS hypercalls. So in order to support MSIX for virtio-pci on
POWER we have to initialize MSI/MSIX message from QEMU.

Signed-off-by: Alexey Kardashevskiy 
---
 hw/msi.c  |   13 +
 hw/msi.h  |1 +
 hw/msix.c |9 +
 hw/msix.h |2 ++
 4 files changed, 25 insertions(+)

diff --git a/hw/msi.c b/hw/msi.c
index 5233204..cc6102f 100644
--- a/hw/msi.c
+++ b/hw/msi.c
@@ -105,6 +105,19 @@ static inline uint8_t msi_pending_off(const PCIDevice* 
dev, bool msi64bit)
 return dev->msi_cap + (msi64bit ? PCI_MSI_PENDING_64 : PCI_MSI_PENDING_32);
 }
 
+void msi_set_message(PCIDevice *dev, MSIMessage msg)
+{
+uint16_t flags = pci_get_word(dev->config + msi_flags_off(dev));
+bool msi64bit = flags & PCI_MSI_FLAGS_64BIT;
+
+if (msi64bit) {
+pci_set_quad(dev->config + msi_address_lo_off(dev), msg.address);
+} else {
+pci_set_long(dev->config + msi_address_lo_off(dev), msg.address);
+}
+pci_set_word(dev->config + msi_data_off(dev, msi64bit), msg.data);
+}
+
 bool msi_enabled(const PCIDevice *dev)
 {
 return msi_present(dev) &&
diff --git a/hw/msi.h b/hw/msi.h
index 75747ab..6ec1f99 100644
--- a/hw/msi.h
+++ b/hw/msi.h
@@ -31,6 +31,7 @@ struct MSIMessage {
 
 extern bool msi_supported;
 
+void msi_set_message(PCIDevice *dev, MSIMessage msg);
 bool msi_enabled(const PCIDevice *dev);
 int msi_init(struct PCIDevice *dev, uint8_t offset,
  unsigned int nr_vectors, bool msi64bit, bool msi_per_vector_mask);
diff --git a/hw/msix.c b/hw/msix.c
index ded3c55..5f7d6d3 100644
--- a/hw/msix.c
+++ b/hw/msix.c
@@ -45,6 +45,15 @@ static MSIMessage msix_get_message(PCIDevice *dev, unsigned 
vector)
 return msg;
 }
 
+void msix_set_message(PCIDevice *dev, int vector, struct MSIMessage msg)
+{
+uint8_t *table_entry = dev->msix_table_page + vector * PCI_MSIX_ENTRY_SIZE;
+
+pci_set_quad(table_entry + PCI_MSIX_ENTRY_LOWER_ADDR, msg.address);
+pci_set_long(table_entry + PCI_MSIX_ENTRY_DATA, msg.data);
+table_entry[PCI_MSIX_ENTRY_VECTOR_CTRL] &= ~PCI_MSIX_ENTRY_CTRL_MASKBIT;
+}
+
 /* Add MSI-X capability to the config space for the device. */
 /* Given a bar and its size, add MSI-X table on top of it
  * and fill MSI-X capability in the config space.
diff --git a/hw/msix.h b/hw/msix.h
index 50aee82..26a437e 100644
--- a/hw/msix.h
+++ b/hw/msix.h
@@ -4,6 +4,8 @@
 #include "qemu-common.h"
 #include "pci.h"
 
+void msix_set_message(PCIDevice *dev, int vector, MSIMessage msg);
+
 int msix_init(PCIDevice *pdev, unsigned short nentries,
   MemoryRegion *bar,
   unsigned bar_nr, unsigned bar_size);
-- 
1.7.10

ps. double '-' and git version is an end-of-patch scissor as I read somewhere, 
cannot recall where exactly :)






On 21/06/12 20:56, Jan Kiszka wrote:
> On 2012-06-21 12:50, Alexey Kardashevskiy wrote:
>> On 21/06/12 20:38, Jan Kiszka wrote:
>>> On 2012-06-21 12:28, Alexey Kardashevskiy wrote:
 On 21/06/12 17:39, Jan Kiszka wrote:
> On 2012-06-21 09:18, Alexey Kardashevskiy wrote:
>>
>> agrhhh. sha1 of the patch changed after rebasing :)
>>
>>
>>
>> Added (msi|msix)_(set|get)_message() function for whoever might
>> want to use them.
>>
>> Currently msi_notify()/msix_notify() write to these vectors to
>> signal the guest about an interrupt so the correct values have to
>> written there by the guest or QEMU.
>>
>> For example, POWER guest never initializes MSI/MSIX vectors, instead
>> it uses RTAS hypercalls. So in order to support MSIX for virtio-pci on
>> POWER we have to initialize MSI/MSIX message from QEMU.
>>
>> As only set* function are required by now, the "get" functions were added
>> or made public for a symmetry.
>>
>> Signed-off-by: Alexey Kardashevskiy 
>> ---
>>  hw/msi.c  |   29 +
>>  hw/msi.h  |2 ++
>>  hw/msix.c |   11 ++-
>>  hw/msix.h |3 +++
>>  4 files changed, 44 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/msi.c b/hw/msi.c
>> index 5233204..9ad84a4 100644
>> --- a/hw/msi.c
>> +++ b/hw/msi.c
>> @@ -105,6 +105,35 @@ static inline uint8_t msi_pending_off(const 
>> PCIDevice* dev, bool msi64bit)
>>  return dev->msi_cap + (msi64bit ? PCI_MSI_PENDING_64 : 
>> PCI_MSI_PENDING_32);
>>  }
>>  
>> +MSIMessage msi_get_message(PCIDevice *dev)
>
> MSIMessage msi_get_message(PCIDevice *dev, unsigned vector)


 Who/how/why is going to calculate the vector here?

>
>> +{
>> +uint16_t flags = pci_get_word(dev->config + msi_flags_off(dev));
>> +bool msi64bit = flags & PCI_MSI_FLAG

Re: [Qemu-devel] [PATCH] msi/msix: added API to set MSI message address and data

[Jan Kiszka]

On 2012-06-21 13:39, Alexey Kardashevskiy wrote:
> Added (msi|msix)_set_message() functions.
> 
> Currently msi_notify()/msix_notify() write to these vectors to
> signal the guest about an interrupt so the correct values have to
> written there by the guest or QEMU.
> 
> For example, POWER guest never initializes MSI/MSIX vectors, instead
> it uses RTAS hypercalls. So in order to support MSIX for virtio-pci on
> POWER we have to initialize MSI/MSIX message from QEMU.
> 
> Signed-off-by: Alexey Kardashevskiy 
> ---
>  hw/msi.c  |   13 +
>  hw/msi.h  |1 +
>  hw/msix.c |9 +
>  hw/msix.h |2 ++
>  4 files changed, 25 insertions(+)
> 
> diff --git a/hw/msi.c b/hw/msi.c
> index 5233204..cc6102f 100644
> --- a/hw/msi.c
> +++ b/hw/msi.c
> @@ -105,6 +105,19 @@ static inline uint8_t msi_pending_off(const PCIDevice* 
> dev, bool msi64bit)
>  return dev->msi_cap + (msi64bit ? PCI_MSI_PENDING_64 : 
> PCI_MSI_PENDING_32);
>  }
>  
> +void msi_set_message(PCIDevice *dev, MSIMessage msg)
> +{
> +uint16_t flags = pci_get_word(dev->config + msi_flags_off(dev));
> +bool msi64bit = flags & PCI_MSI_FLAGS_64BIT;
> +
> +if (msi64bit) {
> +pci_set_quad(dev->config + msi_address_lo_off(dev), msg.address);
> +} else {
> +pci_set_long(dev->config + msi_address_lo_off(dev), msg.address);
> +}
> +pci_set_word(dev->config + msi_data_off(dev, msi64bit), msg.data);
> +}
> +
>  bool msi_enabled(const PCIDevice *dev)
>  {
>  return msi_present(dev) &&
> diff --git a/hw/msi.h b/hw/msi.h
> index 75747ab..6ec1f99 100644
> --- a/hw/msi.h
> +++ b/hw/msi.h
> @@ -31,6 +31,7 @@ struct MSIMessage {
>  
>  extern bool msi_supported;
>  
> +void msi_set_message(PCIDevice *dev, MSIMessage msg);
>  bool msi_enabled(const PCIDevice *dev);
>  int msi_init(struct PCIDevice *dev, uint8_t offset,
>   unsigned int nr_vectors, bool msi64bit, bool 
> msi_per_vector_mask);
> diff --git a/hw/msix.c b/hw/msix.c
> index ded3c55..5f7d6d3 100644
> --- a/hw/msix.c
> +++ b/hw/msix.c
> @@ -45,6 +45,15 @@ static MSIMessage msix_get_message(PCIDevice *dev, 
> unsigned vector)
>  return msg;
>  }
>  
> +void msix_set_message(PCIDevice *dev, int vector, struct MSIMessage msg)
> +{
> +uint8_t *table_entry = dev->msix_table_page + vector * 
> PCI_MSIX_ENTRY_SIZE;
> +
> +pci_set_quad(table_entry + PCI_MSIX_ENTRY_LOWER_ADDR, msg.address);
> +pci_set_long(table_entry + PCI_MSIX_ENTRY_DATA, msg.data);
> +table_entry[PCI_MSIX_ENTRY_VECTOR_CTRL] &= ~PCI_MSIX_ENTRY_CTRL_MASKBIT;
> +}
> +
>  /* Add MSI-X capability to the config space for the device. */
>  /* Given a bar and its size, add MSI-X table on top of it
>   * and fill MSI-X capability in the config space.
> diff --git a/hw/msix.h b/hw/msix.h
> index 50aee82..26a437e 100644
> --- a/hw/msix.h
> +++ b/hw/msix.h
> @@ -4,6 +4,8 @@
>  #include "qemu-common.h"
>  #include "pci.h"
>  
> +void msix_set_message(PCIDevice *dev, int vector, MSIMessage msg);
> +
>  int msix_init(PCIDevice *pdev, unsigned short nentries,
>MemoryRegion *bar,
>unsigned bar_nr, unsigned bar_size);
> 

Interface looks good as fas as I can tell (can't asses the POWER need
for clearing the mask bit on msix_set_message).

> -- 
> 1.7.10
> 
> ps. double '-' and git version is an end-of-patch scissor as I read 
> somewhere, cannot recall where exactly 

Check man git-am.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH 5/5] target-i386: move cpu_reset and reset callback to cpu.c

[Igor Mammedov]

I've forgot to include hunk with hw/qdev.h in cpu.c.
I'll repost.

--
-
 Igor

Re: [Qemu-devel] [PATCH 3/5] target-i386: call x86_cpu_realize() after APIC is initialized.

[Igor Mammedov]

On 06/21/2012 12:14 PM, Andreas Färber wrote:

Am 21.06.2012 11:43, schrieb Igor Mammedov:

On 06/20/2012 03:35 PM, Andreas Färber wrote:

Am 20.06.2012 14:59, schrieb Igor Mammedov:

It's not correct to make CPU runnable (i.e. calling x86_cpu_realize())
when not all properties are set (APIC in this case).

Fix it by calling x86_cpu_realize() at board level after APIC is
initialized, right before cpu_reset().

Signed-off-by: Igor Mammedov 
---
   hw/pc.c  |1 +
   target-i386/helper.c |2 --
   2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/pc.c b/hw/pc.c
index 8368701..8a662cf 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -948,6 +948,7 @@ static X86CPU *pc_new_cpu(const char *cpu_model)
   env->apic_state = apic_init(env, env->cpuid_apic_id);
   }
   qemu_register_reset(pc_cpu_reset, cpu);
+x86_cpu_realize(OBJECT(cpu), NULL);
   pc_cpu_reset(cpu);
   return cpu;
   }
diff --git a/target-i386/helper.c b/target-i386/helper.c
index c52ec13..b38ea7f 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -1161,8 +1161,6 @@ X86CPU *cpu_x86_init(const char *cpu_model)
   return NULL;
   }

-x86_cpu_realize(OBJECT(cpu), NULL);
-
   return cpu;
   }



This will require changes in linux-user and possibly bsd-user. Having a

Why it would require changes in linux-user? So far x86_cpu_realize() does
nothing useful in linux-user,  compiled and tested. It should be harmless
for linux-user not to execute it.


Hm, I'd need to recheck...


But I haven't compiled/tested bsd-user, do I need BSD for this?


Yes, you do. But if it's not needed for linux-user then it shouldn't be
needed for bsd-user either.


cpu_realize() would probably help with avoiding #ifdef'ery.
Unfortunately deriving CPUState from DeviceState proves a bit difficult
in the meantime (it worked at one point, now there's lots of circular
header dependencies), and realize support for Object got stopped.



I'm in process of untangling this header mayhem (at least to a point
that allows compilation complete when CPU is derived from Device)


So am I... A few weeks ago my qom-cpu-dev branch on GitHub used to
compile, now something has changed and I needed to take cpu.h out of
qemu-common.h and move lots of, e.g., ARM devices into libhw to avoid
cpu.h dependencies and add cpu.h includes elsewhere. Would be good to
coordinate that, are you on IRC later today?


Sure, I'd like to.
Could you send me connection details for "IRC"? I do not know where it is.



Andreas



--
-
 Igor

Re: [Qemu-devel] [PATCH] build: introduce target CONFIG_ variables and use them for kvm

[Paolo Bonzini]

(Sorry for breaking the thread).

> This avoids the problem associated with having multiple target specific files
> in a single directory with the current build system.

What is exactly the problem?

I saw something about dependencies, I think that should be solved with
something like

$(foreach var, $(nested-vars), $(eval -include $(patsubst %.o, %.d, $($(var)

at the very end of unnest-vars.

> We can eventually get rid of the hw/$BASE_ARCH/Makefiles.obj files too

The goal should be to limit hw/$BASE_ARCH/Makefile.objs to hardware
that is CPU-dependent and to board descriptions.

I _think_ (but I don't have a checkout at hand) that hardware like
virtio can use obj-$(CONFIG_VIRTIO) while staying in hw/Makefile.objs,
but it should really be the only case of target-dependent file in hw/.
 Everything else in hw/$BASE_ARCH should move to target-$BASE_ARCH/hw.
 The steps should be as follows:

1) Identify more groups of hardware that can be moved from
hw/$BASE_ARCH to libhw. Move them.

2) At this point, hw/$BASE_ARCH/Makefile.objs should only refer to a)
boards b) hardware that is CPU dependent c) KVM device models with
host dependencies. Move the sources to hw/$BASE_ARCH, possibly
hw/$BASE_ARCH/kvm, and remove the addprefix invocations from
hw/$BASE_ARCH/Makefile.objs.

3) Move hw/$BASE_ARCH to target-$BASE_ARCH/hw.

I think CONFIG_$BASE_ARCH is a bad idea because it violates the
modularity that Juan introduced together with the config-devices.mak
files.

Paolo

Re: [Qemu-devel] hw/Makefile.objs question

[Andreas Färber]

Am 21.06.2012 13:21, schrieb Alexey Kardashevskiy:
> On 21/06/12 20:36, Andreas Färber wrote:
>> Am 21.06.2012 05:22, schrieb Alexey Kardashevskiy:
>>> I am trying to compile the very last qemu with vfio_pci enabled. VFIO_PCI 
>>> is added as below:
>>>
>>> ./configure:
>>>
>>>  case "$target_arch2" in
>>>   i386|x86_64|ppc64)
>>>  if test "$vfio_pci" = "yes" -a "$target_softmmu" = "yes" ; then
>>>echo "CONFIG_VFIO_PCI=y" >> $config_target_mak
>>>  fi
>>>  esac
>>>
>>>
>>> ./Makefile.target:
>>>
>>>  # VFIO PCI device assignment
>>> obj-$(CONFIG_VFIO_PCI) += vfio_pci.o
>>>
>>>
>>> And it worked before. However it does not anymore as it seems that 
>>> everything in hw/ (and vfio_pci.c
>>> as well as is in hw/ and it is a device) can be only compiled via 
>>> hw/Makefile.objs and
>>> hw/ppc/Makefile.objs (my platform is POWER), it is ignored if to keep it as 
>>> is.
>>>
>>> So I have to move "obj-$(CONFIG_VFIO_PCI) += vfio_pci.o" to 
>>> hw/Makefile.objs (and change obj- to
>>> hw-obj-) but the hw/Makefile.objs does not include (directly or indirectly) 
>>> generated
>>> ppc64-softmmu/config-target.mak with CONFIG_VFIO_PCI=y.
>>>
>>> What is the correct solution?
>>
>> If the file compiles the same for all three, put CONFIG_VFIO_PCI=y into
>> default-configs/{i386,x86_64,ppc64}-softmmu.mak and do
>> hw-obj-$(CONFIG_VFIO_PCI) += in hw/Makefile.objs.
> 
> 
> It only compiles with ./configure --enable-vfio-pci which may or may not set 
> CONFIG_VFIO_PCI to "y".
> Your proposal makes it always "y" (for selected platforms).

Apply some creativity, there's surely examples around. The question is
whether the contents of vfio_pci.o changes or not. If not, then you only
need to build it once in libhwX/, depending on $config_target_mak, and
link to the appropriate targets. If it accesses CPU internals then it
must be built per target.

>> Otherwise, add to hw/{i386,ppc}/Makefile.objs - or with Anthony's
>> proposal from yesterday hw/Makefile.objs becomes possible, too.
> 
> Again, it will be unconditional "y".

No, in this case the condition would be set from configure as before, it
only moves from Makefile.target to the appropriate Makefile.objs.
Note that to limit it to ppc64 (as opposed to ppc) some additional ifeq
check would be needed, as before.

Andreas

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

Re: [Qemu-devel] [RFC] Fixing the error failure

[Daniel P. Berrange]

On Wed, Jun 20, 2012 at 02:48:38PM -0300, Luiz Capitulino wrote:
> Yet another thread fork.
> 
> After talking with Daniel and Markus about QMP errors (which is not just about
> QMP, as this affects QEMU as whole), I've put together the proposal below.
> 
> I'll discuss three points. First, the error format and classes. Second, the
> internal API and third compatibility.
> 
> Don't be afraid about the length of this email, only the first section is long
> but it mostly contains error classes listings.
> 
> 1. Error format and classes
> 
> We can keep the same error format we have today, which is:
> 
>  { "error": { "class": json-string,
>   "data": json-object,
>   "desc": json-string }, "id": json-value }
> 
>   where 'data', 'desc' and 'id' are optional fields.

Yep, that is good.

> However, we'd change how we use 'desc' and our error classes. 'desc' would
> become a string which is filled by a printf-like function (see section 2) and

Good, using a printf-like string for desc is the big change
I wanted to see in QMP errors.

> we'd replace all error classes we have today by the following ones:

Nooo, that's going a bit to far in simplification

>   o ParameterError: any error which involves a bad parameter. Replaces
> InvalidParameter, InvalidParameterCombination, InvalidParameterType,
> InvalidParameterValue, MissingParameter
> 
>   o SystemError: syscall or library errors. Replaces BufferOverrun,
> IOError, OpenFileFailed, PermissionDenied, TooManyFiles,
> SockConnectInprogress, SockConnectFailed, SockListenFailed,
> SockBindFailed, SockCreateFailed.
> 
> This error can include an optional 'os-error' field in the 'data'
> member (see section 2).
> 
>   o QEMUError: errors that are internal to QEMU, like AmbiguousPath,
> BadBusForDevice, BusNoHotplug, BusNotFound, CommandDisabled,
> CommandNotFound, DuplicateId, FeatureDisabled, JSONParseError,
> JSONParsing, KVMMissingCap, MigrationActive, MigrationNotSupported,
> MigrationExpected, NoBusForDevice, NotSupported, PropertyValueBad,
> PropertyValueInUse, PropertyValueNotFound, PropertyValueNotPowerOf2,
> PropertyValueOutOfRange, ResetRequired, SetPasswdFailed, Unsupported,
> VirtFSFeatureBlocksMigration, VNCServerFailed
> 
>   o UndefinedError: the same it's today, undefined :)

There is a balance to be struck here - previously we were tending
to invent a new error class for every conceivable error condition.
This proposal meanwhile is swinging too far to the other extreme
having only 4/5 classes. There is a balance to be had here.

It is perfectly reasonable, and indeed useful, to have distinct
errors like CommandNotFound, CommandDisabled, and so on.  What
we shouldn't do, however, is do things like invent a new error
for every possible errno value. The examples of  PropertyValueNotFound,
PropertyValueNotPowerOf2, PropertyValueOutOfRange are cases where
we invented too many codes, and in the new world we would do
something like

   PropertyValueInvalid
 msg = "Value must be a power of 2"
 msg = "Value must be in range 1-30"
 msg = "Value not found"

We have to just use prudent judgement to decide whether to use
an existing error, or create a new one.

In libvirt we have always reserved the right to change the error
code reported for particular scenarios. So, for example, alot of
our errors started out as "InternalError" (equiv to UndefinedError)
but over time we have changed some to more specialized values
eg "InvalidOperation", "ConfigNotSupported" and so on.

We should probably explicitly note that any use of "UndefinedError"
is liable to be changed in a future QEMU release.

> Now, there are two important points to be observed:
> 
>  - We check for DeviceEncrypted in hmp.c and fetch its parameters. This
>probably indicates that we might want to create specialized classes
>when necessary
> 
>  - I don't know where to put all the DeviceFoo errors, but they probably fit
>in QEMUError or a new class like DeviceError
> 3. Compatibility
> 
> We have two options here:
> 
>  1. Maintain the current errors and implement the new way only for new
> commands (includes commands merged for 1.2).
> 
>   Pros: maintains compatibility and won't require any code churn
>   Cons: we'll have two different errors schemas in use at the same
> time and will be carrying garbage forward
> 
>  2. Do a full conversion to the new way.
> 
>   Pros: we drop bad code and avoid pollution (good for Rio+20)
>   Cons: possibly breaks compatibility and will require a lot of code
> churn up front

Just maintain existing usage, but apply appropriate judgement for new
conversions.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http:

Re: [Qemu-devel] [RFC] SSI QOMification

[Andreas Färber]

Am 21.06.2012 02:21, schrieb Peter Crosthwaite:
> Ping!
> 
> Id really appreciate some input on this issue (rather than going ahead
> and doing it to discover that someone disagrees with the approach).
> 
> On Mon, Jun 18, 2012 at 3:13 PM, Peter Crosthwaite
>  wrote:
>> So here are the nitty-gritty details around the pending QOM stuff:
>>
>> Anthony is currently overhauling QBus, and im guessing the SSI bus is
>> part of that? qom-next stable enough in this area to look at or not?

qom-next has been fully merged into qemu.git now, so please do not base
any work on the stale branch.

>> Also Anthony mentioned recently some GPIO refactoring stuff

Guess you're referring to the "Pin" series. That was waiting on the QBus
merge. Now it's probably been held up by the various Makefile fires. ;)

Andreas

>> - Are
>> GPIOs on multiple levels of abstraction supported yet? IE if I have a
>> SPI GPIO device, I need a GPIO on the SSISlave layer but also GPIOs on
>> my (concrete) device layer. I know this currently doesnt work cos of
>> qdev but thats going away right?
>>
>> Regards,
>> Peter

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

Re: [Qemu-devel] [PATCH 04/13] usb-ohci: Use universal DMA helper functions

[Anthony Liguori]

On 06/21/2012 02:33 AM, Michael S. Tsirkin wrote:

On Thu, Jun 21, 2012 at 08:02:06AM +1000, Benjamin Herrenschmidt wrote:

On Wed, 2012-06-20 at 16:40 -0500, Anthony Liguori wrote:


Well let's return void in the DMA methods and let the IOMMUs assert on error.
At least that will avoid surprises until someone decides they care enough about
errors to touch all callers.

I think silently failing a memcpy() can potentially lead to a vulnerability so
I'd rather avoid that.


No I'd rather keep the error returns, really, even if that means fixing
a few devices. I can look at making sure we don't pass random qemu data,
on error that's reasonably easy.

assert on error means guest code can assert qemu ... not a great idea
but maybe we can add a warning.


Why not?  Guest can always just halt if it wants to anyway.
On the other hand, warnings can fill up host logs so
represent a security problem.


As long as we scrub the buffers, returning an unhandled error seems okay to me.

I've long thought we should have some sort of generic way to throw an error and 
effectively pause a single device.  I'm not sure how it would work in practice 
though.


Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH] build: introduce target CONFIG_ variables and use them for kvm

[Anthony Liguori]

On 06/21/2012 07:31 AM, Paolo Bonzini wrote:

(Sorry for breaking the thread).


This avoids the problem associated with having multiple target specific files
in a single directory with the current build system.


What is exactly the problem?


Peter's got an ARM specific KVM device he wants to stick in hw/kvm.

Right now, kvm/ is all x86 specific and wants CONFIG_KVM && CONFIG_I386.  We 
gets this by doing:


hw/${TARGET}/Makefile.objs  <=  CONFIG_I386

And then within Makefile.objs:

obj-$(CONFIG_KVM) += kvm/

But this applies for the whole directory.  Previously, you did:

obj-$(CONFIG_KVM) += kvm/apic.o kvm/clock.o ...

The way you did it made it possible for hw/arm/Makefile.obj to have a different 
set of objects but also didn't use sub directory makefiles.


So this patch allows us to achieve CONFIG_KVM && CONFIG_I386 by doing:

hw/Makefiles.objs:

obj-$(CONFIG_KVM) += kvm/

hw/kvm/Makefiles.obj:

obj-$(CONFIG_I386) += apic.o clock.o

Which I think is actually more straight forward.  This gives us the logic we 
need and let's use us subdirectory makefiles too.




I saw something about dependencies, I think that should be solved with
something like

$(foreach var, $(nested-vars), $(eval -include $(patsubst %.o, %.d, $($(var)

at the very end of unnest-vars.


Already added that BTW :-)  That's a different thread.


We can eventually get rid of the hw/$BASE_ARCH/Makefiles.obj files too


The goal should be to limit hw/$BASE_ARCH/Makefile.objs to hardware
that is CPU-dependent and to board descriptions.

I _think_ (but I don't have a checkout at hand) that hardware like
virtio can use obj-$(CONFIG_VIRTIO) while staying in hw/Makefile.objs,
but it should really be the only case of target-dependent file in hw/.
  Everything else in hw/$BASE_ARCH should move to target-$BASE_ARCH/hw.
  The steps should be as follows:


Yup, I'm trying to refactor some of this.

Most of what's in hw/$BASE_ARCH today is really just hardware that doesn't apply 
to any other targets but is not truly target specific.


We could introduce per-device CONFIG variables and update all of the 
default-configs/ but that's a big pain for marginal benefit.  Instead, I think 
what we want in the long term is to have machine-specific CONFIG variables. 
Something like CONFIG_PC or CONFIG_VERSITALE.


But in the very short term, CONFIG_I386 makes a good stepping stone to CONFIG_PC 
and let's use refactor the Makefiles such that we can introduce more granular 
CONFIG_* down the road without changing object locations.


I think that's really the way to think of it.  We start with big guards 
(CONFIG_I386) and over time break them down into smaller guards (CONFIG_PC).



1) Identify more groups of hardware that can be moved from
hw/$BASE_ARCH to libhw. Move them.

2) At this point, hw/$BASE_ARCH/Makefile.objs should only refer to a)
boards b) hardware that is CPU dependent c) KVM device models with
host dependencies. Move the sources to hw/$BASE_ARCH, possibly
hw/$BASE_ARCH/kvm, and remove the addprefix invocations from
hw/$BASE_ARCH/Makefile.objs.

3) Move hw/$BASE_ARCH to target-$BASE_ARCH/hw.

I think CONFIG_$BASE_ARCH is a bad idea because it violates the
modularity that Juan introduced together with the config-devices.mak
files.


On the contrary, I think it's the easiest way to improve our modularity.  See 
above.

Regards,

Anthony Liguori



Paolo

Re: [Qemu-devel] hw/Makefile.objs question

[Alexey Kardashevskiy]

On 21/06/12 22:19, Andreas Färber wrote:
> Am 21.06.2012 13:21, schrieb Alexey Kardashevskiy:
>> On 21/06/12 20:36, Andreas Färber wrote:
>>> Am 21.06.2012 05:22, schrieb Alexey Kardashevskiy:
 I am trying to compile the very last qemu with vfio_pci enabled. VFIO_PCI 
 is added as below:

 ./configure:

  case "$target_arch2" in
   i386|x86_64|ppc64)
  if test "$vfio_pci" = "yes" -a "$target_softmmu" = "yes" ; then
echo "CONFIG_VFIO_PCI=y" >> $config_target_mak
  fi
  esac


 ./Makefile.target:

  # VFIO PCI device assignment
 obj-$(CONFIG_VFIO_PCI) += vfio_pci.o


 And it worked before. However it does not anymore as it seems that 
 everything in hw/ (and vfio_pci.c
 as well as is in hw/ and it is a device) can be only compiled via 
 hw/Makefile.objs and
 hw/ppc/Makefile.objs (my platform is POWER), it is ignored if to keep it 
 as is.

 So I have to move "obj-$(CONFIG_VFIO_PCI) += vfio_pci.o" to 
 hw/Makefile.objs (and change obj- to
 hw-obj-) but the hw/Makefile.objs does not include (directly or 
 indirectly) generated
 ppc64-softmmu/config-target.mak with CONFIG_VFIO_PCI=y.

 What is the correct solution?
>>>
>>> If the file compiles the same for all three, put CONFIG_VFIO_PCI=y into
>>> default-configs/{i386,x86_64,ppc64}-softmmu.mak and do
>>> hw-obj-$(CONFIG_VFIO_PCI) += in hw/Makefile.objs.
>>
>>
>> It only compiles with ./configure --enable-vfio-pci which may or may not set 
>> CONFIG_VFIO_PCI to "y".
>> Your proposal makes it always "y" (for selected platforms).
> 
> Apply some creativity, there's surely examples around. The question is
> whether the contents of vfio_pci.o changes or not.

Applied already and gave up, this is why I am writing here :)
What would be a good example of a device which is enabled by configure script?

> If not, then you only
> need to build it once in libhwX/, depending on $config_target_mak, and
> link to the appropriate targets. If it accesses CPU internals then it
> must be built per target.

$config_target_mak points to ppc64-softmmu/config-target.mak, not in the root 
folder.
When executed in libhw64, it is not visible to makefile.


>>> Otherwise, add to hw/{i386,ppc}/Makefile.objs - or with Anthony's
>>> proposal from yesterday hw/Makefile.objs becomes possible, too.
>>
>> Again, it will be unconditional "y".
> 
> No, in this case the condition would be set from configure as before, it
> only moves from Makefile.target to the appropriate Makefile.objs.
> Note that to limit it to ppc64 (as opposed to ppc) some additional ifeq
> check would be needed, as before.




-- 
Alexey

Re: [Qemu-devel] [PATCH v6 00/17] hub-based networking patchset

[Stefan Hajnoczi]

On Wed, Jun 20, 2012 at 10:42 AM,   wrote:
> From: Zhi Yong Wu 
>
> All comments have been addressed and stefan has completed one more reviewing.
>
> For this patchset, my git repo:
>
> g...@github.com:wuzhy/qemu.git for-anthony
>
> Changelog from v5:
>  1.) cleanup VLANState in other targets files [anthony]
>
> v5:
>  1.) roll back qdev_prop_vlan [stefanha]
>
> v4:
>  1.) refactor hub own flow control [paolo]
>  2.) refactor the output for monitor command "info network" [jan kiszka]
>
> v3:
>  1.) add the support for hub own flow control [paolo]
>  2.) make the monitor output more reasonable hub info [jan kiszka]
>
> v2:
>  1.) cleanup some obsolete vlan info
>  2.) cleanup deliver/deliver_iov func pointers [paolo]
>  3.) support more flexible flow control [paolo]
>
> Stefan Hajnoczi (12):
>  net: Add a hub net client
>  net: Use hubs for the vlan feature
>  net: Look up 'vlan' net clients using hubs
>  hub: Check that hubs are configured correctly
>  net: Drop vlan argument to qemu_new_net_client()
>  net: Remove vlan qdev property
>  net: Remove vlan code from net.c
>  net: Remove VLANState
>  net: Rename non_vlan_clients to net_clients
>  net: Rename VLANClientState to NetClientState
>  net: Rename vc local variables to nc
>  net: Rename qemu_del_vlan_client() to qemu_del_net_client()
>
> Zhi Yong Wu (5):
>  net: Make "info network" output more readable info
>  net: cleanup deliver/deliver_iov func pointers
>  net: determine if packets can be sent before net queue deliver
>    packets
>  hub: add the support for hub own flow control
>  net: roll back qdev_prop_vlan
>
>  hw/cadence_gem.c        |    8 +-
>  hw/dp8393x.c            |    7 +-
>  hw/e1000.c              |   10 +-
>  hw/eepro100.c           |    8 +-
>  hw/etraxfs_eth.c        |    8 +-
>  hw/exynos4_boards.c     |    2 +-
>  hw/highbank.c           |    2 +-
>  hw/integratorcp.c       |    2 +-
>  hw/lan9118.c            |    8 +-
>  hw/lance.c              |    2 +-
>  hw/mcf5208.c            |    2 +-
>  hw/mcf_fec.c            |    7 +-
>  hw/milkymist-minimac2.c |    6 +-
>  hw/mips_mipssim.c       |    2 +-
>  hw/mips_r4k.c           |    2 +-
>  hw/mipsnet.c            |    6 +-
>  hw/musicpal.c           |    6 +-
>  hw/ne2000-isa.c         |    2 +-
>  hw/ne2000.c             |    8 +-
>  hw/ne2000.h             |    4 +-
>  hw/opencores_eth.c      |    8 +-
>  hw/pcnet-pci.c          |    4 +-
>  hw/pcnet.c              |    6 +-
>  hw/pcnet.h              |    6 +-
>  hw/qdev-properties.c    |   53 +++--
>  hw/qdev.c               |    2 -
>  hw/qdev.h               |    7 +-
>  hw/rtl8139.c            |   10 +-
>  hw/smc91c111.c          |    6 +-
>  hw/spapr_llan.c         |    4 +-
>  hw/stellaris_enet.c     |    6 +-
>  hw/usb/dev-network.c    |    8 +-
>  hw/vexpress.c           |    2 +-
>  hw/vhost_net.c          |   24 +-
>  hw/vhost_net.h          |    2 +-
>  hw/virtio-net.c         |   12 +-
>  hw/xen_nic.c            |    7 +-
>  hw/xgmac.c              |    6 +-
>  hw/xilinx_axienet.c     |    6 +-
>  hw/xilinx_ethlite.c     |    6 +-
>  hw/xtensa_lx60.c        |    2 +-
>  net.c                   |  593 
> ++-
>  net.h                   |   87 
>  net/Makefile.objs       |    2 +-
>  net/dump.c              |   27 ++-
>  net/dump.h              |    2 +-
>  net/hub.c               |  335 ++
>  net/hub.h               |   29 +++
>  net/queue.c             |   37 ++--
>  net/queue.h             |   25 +--
>  net/slirp.c             |   32 +--
>  net/slirp.h             |    2 +-
>  net/socket.c            |   66 +++---
>  net/socket.h            |    2 +-
>  net/tap-win32.c         |   26 +-
>  net/tap.c               |   44 ++--
>  net/tap.h               |   20 +-
>  net/vde.c               |   16 +-
>  net/vde.h               |    2 +-
>  qemu-common.h           |    3 +-
>  slirp/if.c              |    5 -
>  slirp/libslirp.h        |    1 -
>  62 files changed, 868 insertions(+), 777 deletions(-)
>  create mode 100644 net/hub.c
>  create mode 100644 net/hub.h

It successfully builds all targets now.  Other than that the code
hasn't changed since the time I last reviewed.

Stefan

Re: [Qemu-devel] hw/Makefile.objs question

[Anthony Liguori]

On 06/21/2012 08:10 AM, Alexey Kardashevskiy wrote:

On 21/06/12 22:19, Andreas Färber wrote:

Am 21.06.2012 13:21, schrieb Alexey Kardashevskiy:

On 21/06/12 20:36, Andreas Färber wrote:

Am 21.06.2012 05:22, schrieb Alexey Kardashevskiy:

I am trying to compile the very last qemu with vfio_pci enabled. VFIO_PCI is 
added as below:

./configure:

  case "$target_arch2" in
   i386|x86_64|ppc64)
  if test "$vfio_pci" = "yes" -a "$target_softmmu" = "yes" ; then
echo "CONFIG_VFIO_PCI=y">>  $config_target_mak
  fi
  esac


./Makefile.target:

  # VFIO PCI device assignment
obj-$(CONFIG_VFIO_PCI) += vfio_pci.o


And it worked before. However it does not anymore as it seems that everything 
in hw/ (and vfio_pci.c
as well as is in hw/ and it is a device) can be only compiled via 
hw/Makefile.objs and
hw/ppc/Makefile.objs (my platform is POWER), it is ignored if to keep it as is.

So I have to move "obj-$(CONFIG_VFIO_PCI) += vfio_pci.o" to hw/Makefile.objs 
(and change obj- to
hw-obj-) but the hw/Makefile.objs does not include (directly or indirectly) 
generated
ppc64-softmmu/config-target.mak with CONFIG_VFIO_PCI=y.

What is the correct solution?


If the file compiles the same for all three, put CONFIG_VFIO_PCI=y into
default-configs/{i386,x86_64,ppc64}-softmmu.mak and do
hw-obj-$(CONFIG_VFIO_PCI) += in hw/Makefile.objs.



It only compiles with ./configure --enable-vfio-pci which may or may not set 
CONFIG_VFIO_PCI to "y".
Your proposal makes it always "y" (for selected platforms).


Apply some creativity, there's surely examples around. The question is
whether the contents of vfio_pci.o changes or not.


Applied already and gave up, this is why I am writing here :)
What would be a good example of a device which is enabled by configure script?


We don't have a great way to do this.  CONFIG_VIRTFS is the best example.

You need three things:

1) a CONFIG_ variable set by configure (whether the user wants VFIO)

2) a CONFIG_ variable set by the target default-config/ whether the board 
supports VFIO.  This could just be CONFIG_PCI btw.


3) a variable that you create in Makefile.objs that is only set if (1) && (2) 
are both set to y


You can then use the variable from (3) to do obj-$(CONFIG_...) in the 
appropriate Makefiles.obj.


Regards,

Anthony Liguori




If not, then you only
need to build it once in libhwX/, depending on $config_target_mak, and
link to the appropriate targets. If it accesses CPU internals then it
must be built per target.


$config_target_mak points to ppc64-softmmu/config-target.mak, not in the root 
folder.
When executed in libhw64, it is not visible to makefile.



Otherwise, add to hw/{i386,ppc}/Makefile.objs - or with Anthony's
proposal from yesterday hw/Makefile.objs becomes possible, too.


Again, it will be unconditional "y".


No, in this case the condition would be set from configure as before, it
only moves from Makefile.target to the appropriate Makefile.objs.
Note that to limit it to ppc64 (as opposed to ppc) some additional ifeq
check would be needed, as before.

Re: [Qemu-devel] [PATCH v1 0/2] Xilinx Timer updates

[Edgar E. Iglesias]

On Sat, Jun 16, 2012 at 03:20:57PM +1000, Peter A. G. Crosthwaite wrote:
> Patch 1 is trival, just deleted a redundant include that shouldn't be there.
> Patch 2 is a major bugfix for Microblaze platforms - the timer was deadlocking
> the system.
> 
> Peter A. G. Crosthwaite (2):
>   xilinx_timer: Removed include of qemu-timer
>   xilinx_timer: Fixed deadlock issue


Applied, thanks Peter


> 
>  hw/xilinx_timer.c |3 +--
>  1 files changed, 1 insertions(+), 2 deletions(-)
> 
> -- 
> 1.7.3.2
> 

Re: [Qemu-devel] Plans for the hard feature freeze

[Andreas Färber]

Am 11.05.2012 03:49, schrieb Andreas Färber:
> Am 29.04.2012 20:57, schrieb Anthony Liguori:
>> We'll run this release much like the last one.  All bug fixing will
>> happen in master until 1.1 is released.  If you plan on maintaining a
>> subsystem tree while we're in feature freeze, please respond to this
>> note with information about your tree.
> 
> On Paolo's suggestion and with Anthony's blessing I have set up a
> qom-next tree:
> 
> T: git git://repo.or.cz/qemu/afaerber.git qom-next
> T: git http://repo.or.cz/r/qemu/afaerber.git qom-next
> W: http://repo.or.cz/w/qemu/afaerber.git/shortlog/refs/heads/qom-next
> 
> This is intended as temporary solution to minimize conflicts between QOM
> refactorings or features during the Hard Freeze and to facilitate a
> quick and painless merge into master once the window for 1.2 opens.

Slightly later than anticipated (due to the early 1.1 release and some
fallout from the build system overhaul) everything that made it into
qom-next is now merged into qemu.git master.

The qom-next branch is therefore officially disbanded.
Any QOM patches should be based onto qemu.git master branch again.

Still open among the QOM-related series I had tracked are for example:
* Paolo's and my "realize" - pending semantics discussion
* Paolo's static properties generalization - depends on state in Object
* my QOM CPUState part 4 (field movements) - needs review apart from TLB
* Igor's x86 CPU APIC/reset refactoring - under review
* Juan's VMState series - how to attach VMSD to CPUClass/DeviceClass?
* Markus' floppy QOM'ification - initial RFC only

Thanks to everyone who supported this effort during the Freeze!

Regards,
Andreas

> Stefan, can you add this branch to the build bots please?
> 
> qom-next will track master as a patch queue, rebasing as necessary.
> So if you want to add a tag or find a patch is bogus, say so. :)
> 
> Regards,
> Andreas

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

[Qemu-devel] [PATCH v2 11/11] PPC: BookE206: Bump MAS2 to 64bit

[Alexander Graf]

On 64bit capable systems, MAS2 can actually hold a 64bit virtual page
address. So increase the mask for its EPN.

Signed-off-by: Alexander Graf 
---
 target-ppc/cpu.h |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 652a35a..ca2fc21 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -693,7 +693,7 @@ enum {
 #define MAS1_VALID 0x8000
 
 #define MAS2_EPN_SHIFT 12
-#define MAS2_EPN_MASK  (0xf << MAS2_EPN_SHIFT)
+#define MAS2_EPN_MASK  (~0ULL << MAS2_EPN_SHIFT)
 
 #define MAS2_ACM_SHIFT 6
 #define MAS2_ACM   (1 << MAS2_ACM_SHIFT)
-- 
1.6.0.2

[Qemu-devel] [PATCH v2 05/11] PPC: Add support for MSR_CM

[Alexander Graf]

The BookE variant of MSR_SF is MSR_CM. Implement everything it takes in TCG to
support running 64bit code with MSR_CM set.

Signed-off-by: Alexander Graf 
---
 target-ppc/cpu.h |9 +
 target-ppc/excp_helper.c |9 +
 target-ppc/mem_helper.c  |2 +-
 target-ppc/translate.c   |2 +-
 4 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 12200ab..7a77fff 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -2212,6 +2212,15 @@ static inline uint32_t booke206_tlbnps(CPUPPCState *env, 
const int tlbn)
 
 #endif
 
+static inline bool msr_is_64bit(CPUPPCState *env, target_ulong msr)
+{
+if (env->mmu_model == POWERPC_MMU_BOOKE206) {
+return msr & (1ULL << MSR_CM);
+}
+
+return msr & (1ULL << MSR_SF);
+}
+
 extern void (*cpu_ppc_hypercall)(CPUPPCState *);
 
 static inline bool cpu_has_work(CPUPPCState *env)
diff --git a/target-ppc/excp_helper.c b/target-ppc/excp_helper.c
index c7762b9..1a593f6 100644
--- a/target-ppc/excp_helper.c
+++ b/target-ppc/excp_helper.c
@@ -608,10 +608,11 @@ static inline void powerpc_excp(CPUPPCState *env, int 
excp_model, int excp)
 vector |= env->excp_prefix;
 #if defined(TARGET_PPC64)
 if (excp_model == POWERPC_EXCP_BOOKE) {
-if (!msr_icm) {
-vector = (uint32_t)vector;
-} else {
+if (env->spr[SPR_BOOKE_EPCR] & EPCR_ICM) {
+/* Cat.64-bit: EPCR.ICM is copied to MSR.CM */
 new_msr |= (target_ulong)1 << MSR_CM;
+} else {
+vector = (uint32_t)vector;
 }
 } else {
 if (!msr_isf && !(env->mmu_model & POWERPC_MMU_64)) {
@@ -803,7 +804,7 @@ static inline void do_rfi(CPUPPCState *env, target_ulong 
nip, target_ulong msr,
   target_ulong msrm, int keep_msrh)
 {
 #if defined(TARGET_PPC64)
-if (msr & (1ULL << MSR_SF)) {
+if (msr_is_64bit(env, msr)) {
 nip = (uint64_t)nip;
 msr &= (uint64_t)msrm;
 } else {
diff --git a/target-ppc/mem_helper.c b/target-ppc/mem_helper.c
index ebcd7b2..5b5f1bd 100644
--- a/target-ppc/mem_helper.c
+++ b/target-ppc/mem_helper.c
@@ -35,7 +35,7 @@ static inline target_ulong addr_add(CPUPPCState *env, 
target_ulong addr,
 target_long arg)
 {
 #if defined(TARGET_PPC64)
-if (!msr_sf) {
+if (!msr_is_64bit(env, env->msr)) {
 return (uint32_t)(addr + arg);
 } else
 #endif
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 9103fd5..73ee74b 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -9626,7 +9626,7 @@ static inline void 
gen_intermediate_code_internal(CPUPPCState *env,
 ctx.access_type = -1;
 ctx.le_mode = env->hflags & (1 << MSR_LE) ? 1 : 0;
 #if defined(TARGET_PPC64)
-ctx.sf_mode = msr_sf;
+ctx.sf_mode = msr_is_64bit(env, env->msr);
 ctx.has_cfar = !!(env->flags & POWERPC_FLAG_CFAR);
 #endif
 ctx.fpu_enabled = msr_fp;
-- 
1.6.0.2

[Qemu-devel] [PATCH v2 07/11] PPC: BookE: Make ivpr selectable by CPU type

[Alexander Graf]

IVPR can either hold 32 or 64 bit addresses, depending on the CPU type. Let
the CPU initialization function pass in its mask itself, so we can easily
extend it.

Signed-off-by: Alexander Graf 
---
 target-ppc/translate_init.c |9 +
 1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 57027a2..98695ab 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -2804,7 +2804,7 @@ static void init_excp_G2 (CPUPPCState *env)
 #endif
 }
 
-static void init_excp_e200 (CPUPPCState *env)
+static void init_excp_e200(CPUPPCState *env, target_ulong ivpr_mask)
 {
 #if !defined(CONFIG_USER_ONLY)
 env->excp_vectors[POWERPC_EXCP_RESET]= 0x0FFC;
@@ -2829,7 +2829,7 @@ static void init_excp_e200 (CPUPPCState *env)
 env->excp_vectors[POWERPC_EXCP_EFPRI]= 0x;
 env->hreset_excp_prefix = 0xUL;
 env->ivor_mask = 0xFFF7UL;
-env->ivpr_mask = 0xUL;
+env->ivpr_mask = ivpr_mask;
 /* Hardware reset vector */
 env->hreset_vector = 0xFFFCUL;
 #endif
@@ -4307,7 +4307,7 @@ static void init_proc_e200 (CPUPPCState *env)
 env->id_tlbs = 0;
 env->tlb_type = TLB_EMB;
 #endif
-init_excp_e200(env);
+init_excp_e200(env, 0xUL);
 env->dcache_line_size = 32;
 env->icache_line_size = 32;
 /* XXX: TODO: allocate internal IRQ controller */
@@ -4434,6 +4434,7 @@ static void init_proc_e500 (CPUPPCState *env, int version)
 {
 uint32_t tlbncfg[2];
 uint64_t ivor_mask = 0x000FULL;
+uint64_t ivpr_mask = 0xULL;
 uint32_t l1cfg0 = 0x3800  /* 8 ways */
 | 0x0020; /* 32 kb */
 #if !defined(CONFIG_USER_ONLY)
@@ -4575,7 +4576,7 @@ static void init_proc_e500 (CPUPPCState *env, int version)
 }
 #endif
 
-init_excp_e200(env);
+init_excp_e200(env, ivpr_mask);
 /* Allocate hardware IRQ controller */
 ppce500_irq_init(env);
 }
-- 
1.6.0.2

Re: [Qemu-devel] hw/Makefile.objs question

[Andreas Färber]

Am 21.06.2012 15:10, schrieb Alexey Kardashevskiy:
> On 21/06/12 22:19, Andreas Färber wrote:
>> Am 21.06.2012 13:21, schrieb Alexey Kardashevskiy:
>>> On 21/06/12 20:36, Andreas Färber wrote:
 Am 21.06.2012 05:22, schrieb Alexey Kardashevskiy:
> I am trying to compile the very last qemu with vfio_pci enabled. VFIO_PCI 
> is added as below:
>
> ./configure:
>
>  case "$target_arch2" in
>   i386|x86_64|ppc64)
>  if test "$vfio_pci" = "yes" -a "$target_softmmu" = "yes" ; then
>echo "CONFIG_VFIO_PCI=y" >> $config_target_mak
>  fi
>  esac
>
>
> ./Makefile.target:
>
>  # VFIO PCI device assignment
> obj-$(CONFIG_VFIO_PCI) += vfio_pci.o
>
>
> And it worked before. However it does not anymore as it seems that 
> everything in hw/ (and vfio_pci.c
> as well as is in hw/ and it is a device) can be only compiled via 
> hw/Makefile.objs and
> hw/ppc/Makefile.objs (my platform is POWER), it is ignored if to keep it 
> as is.
>
> So I have to move "obj-$(CONFIG_VFIO_PCI) += vfio_pci.o" to 
> hw/Makefile.objs (and change obj- to
> hw-obj-) but the hw/Makefile.objs does not include (directly or 
> indirectly) generated
> ppc64-softmmu/config-target.mak with CONFIG_VFIO_PCI=y.
>
> What is the correct solution?

 If the file compiles the same for all three, put CONFIG_VFIO_PCI=y into
 default-configs/{i386,x86_64,ppc64}-softmmu.mak and do
 hw-obj-$(CONFIG_VFIO_PCI) += in hw/Makefile.objs.
>>>
>>>
>>> It only compiles with ./configure --enable-vfio-pci which may or may not 
>>> set CONFIG_VFIO_PCI to "y".
>>> Your proposal makes it always "y" (for selected platforms).
>>
>> Apply some creativity, there's surely examples around. The question is
>> whether the contents of vfio_pci.o changes or not.
> 
> Applied already and gave up, this is why I am writing here :)
> What would be a good example of a device which is enabled by configure script?

You're missing my point: We need to know *where* the .o file needs to
go, then we can point you to appropriate examples. Making things depend
on configure options should be trivial from there.

My understanding is that VFIO depends only on Linux/KVM support so I
don't understand why you'd be excluding VFIO for ppc. s390 has no PCI
AFAIU so that'd be okay.

>> If not, then you only
>> need to build it once in libhwX/, depending on $config_target_mak, and
>> link to the appropriate targets. If it accesses CPU internals then it
>> must be built per target.
> 
> $config_target_mak points to ppc64-softmmu/config-target.mak, not in the root 
> folder.
> When executed in libhw64, it is not visible to makefile.

Correct. You keep mixing up both approaches I named...

*If* the file is built per target (hw/ppc64/Makefile.objs), then you can
use *-softmmu/config-target.mak and just need to use a different
Makefile than before.

*If* the file is built per libhw (hw/Makefile.objs), then you need one
option whether to compile it and another for whether to link it into a
particular target.

You still haven't answered the question of which of these two cases
applies here, so I cannot say more than I already have. Anthony's 3)
elaborates on my briefly mentioned ifeq.

Andreas

 Otherwise, add to hw/{i386,ppc}/Makefile.objs - or with Anthony's
 proposal from yesterday hw/Makefile.objs becomes possible, too.
>>>
>>> Again, it will be unconditional "y".
>>
>> No, in this case the condition would be set from configure as before, it
>> only moves from Makefile.target to the appropriate Makefile.objs.
>> Note that to limit it to ppc64 (as opposed to ppc) some additional ifeq
>> check would be needed, as before.

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

Re: [Qemu-devel] [PATCH 04/13] usb-ohci: Use universal DMA helper functions

[Michael S. Tsirkin]

On Thu, Jun 21, 2012 at 07:55:58AM -0500, Anthony Liguori wrote:
> On 06/21/2012 02:33 AM, Michael S. Tsirkin wrote:
> >On Thu, Jun 21, 2012 at 08:02:06AM +1000, Benjamin Herrenschmidt wrote:
> >>On Wed, 2012-06-20 at 16:40 -0500, Anthony Liguori wrote:
> >>
> >>>Well let's return void in the DMA methods and let the IOMMUs assert on 
> >>>error.
> >>>At least that will avoid surprises until someone decides they care enough 
> >>>about
> >>>errors to touch all callers.
> >>>
> >>>I think silently failing a memcpy() can potentially lead to a 
> >>>vulnerability so
> >>>I'd rather avoid that.
> >>
> >>No I'd rather keep the error returns, really, even if that means fixing
> >>a few devices. I can look at making sure we don't pass random qemu data,
> >>on error that's reasonably easy.
> >>
> >>assert on error means guest code can assert qemu ... not a great idea
> >>but maybe we can add a warning.
> >
> >Why not?  Guest can always just halt if it wants to anyway.
> >On the other hand, warnings can fill up host logs so
> >represent a security problem.
> 
> As long as we scrub the buffers, returning an unhandled error seems okay to 
> me.
> 
> I've long thought we should have some sort of generic way to throw
> an error and effectively pause a single device.  I'm not sure how it
> would work in practice though.
> 
> Regards,
> 
> Anthony Liguori

I think we should add an API to log a message and pause the VM.
Later admin can resume the VM, save it to file for debugging etc.

-- 
MST

[Qemu-devel] [PATCH v2 09/11] PPC: Extract SPR dump generation into its own function

[Alexander Graf]

This patch moves the debug #ifdef'ed SPR trace generation into its
own function, so we can call it from multiple places.

Signed-off-by: Alexander Graf 
---
 target-ppc/translate_init.c |   30 ++
 1 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index d185aaa..8ff47ae 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -55,28 +55,34 @@ PPC_IRQ_INIT_FN(e500);
 /* Generic callbacks:
  * do nothing but store/retrieve spr value
  */
+static void spr_load_dump_spr(int sprn)
+{
+#ifdef PPC_DUMP_SPR_ACCESSES
+TCGv_i32 t0 = tcg_const_i32(sprn);
+gen_helper_load_dump_spr(t0);
+tcg_temp_free_i32(t0);
+#endif
+}
+
 static void spr_read_generic (void *opaque, int gprn, int sprn)
 {
 gen_load_spr(cpu_gpr[gprn], sprn);
+spr_load_dump_spr(sprn);
+}
+
+static void spr_store_dump_spr(int sprn)
+{
 #ifdef PPC_DUMP_SPR_ACCESSES
-{
-TCGv_i32 t0 = tcg_const_i32(sprn);
-gen_helper_load_dump_spr(t0);
-tcg_temp_free_i32(t0);
-}
+TCGv_i32 t0 = tcg_const_i32(sprn);
+gen_helper_store_dump_spr(t0);
+tcg_temp_free_i32(t0);
 #endif
 }
 
 static void spr_write_generic (void *opaque, int sprn, int gprn)
 {
 gen_store_spr(sprn, cpu_gpr[gprn]);
-#ifdef PPC_DUMP_SPR_ACCESSES
-{
-TCGv_i32 t0 = tcg_const_i32(sprn);
-gen_helper_store_dump_spr(t0);
-tcg_temp_free_i32(t0);
-}
-#endif
+spr_store_dump_spr(sprn);
 }
 
 #if !defined(CONFIG_USER_ONLY)
-- 
1.6.0.2

[Qemu-devel] [PATCH v2 04/11] PPC: Add some booke SPR defines

[Alexander Graf]

The number of SPRs avaiable in different PowerPC chip is still increasing. Add
definitions for the MAS7_MAS3 SPR and all currently known bits in EPCR.

Signed-off-by: Alexander Graf 
---
 target-ppc/cpu.h |   22 ++
 1 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 67e699c..12200ab 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -1395,6 +1395,7 @@ static inline void cpu_clone_regs(CPUPPCState *env, 
target_ulong newsp)
 #define SPR_BOOKE_TLB1PS  (0x159)
 #define SPR_BOOKE_TLB2PS  (0x15A)
 #define SPR_BOOKE_TLB3PS  (0x15B)
+#define SPR_BOOKE_MAS7_MAS3   (0x174)
 #define SPR_BOOKE_IVOR0   (0x190)
 #define SPR_BOOKE_IVOR1   (0x191)
 #define SPR_BOOKE_IVOR2   (0x192)
@@ -1762,6 +1763,27 @@ static inline void cpu_clone_regs(CPUPPCState *env, 
target_ulong newsp)
 #define SPR_604_HID15 (0x3FF)
 #define SPR_E500_SVR  (0x3FF)
 
+/* Disable MAS Interrupt Updates for Hypervisor */
+#define EPCR_DMIUH(1 << 22)
+/* Disable Guest TLB Management Instructions */
+#define EPCR_DGTMI(1 << 23)
+/* Guest Interrupt Computation Mode */
+#define EPCR_GICM (1 << 24)
+/* Interrupt Computation Mode */
+#define EPCR_ICM  (1 << 25)
+/* Disable Embedded Hypervisor Debug */
+#define EPCR_DUVD (1 << 26)
+/* Instruction Storage Interrupt Directed to Guest State */
+#define EPCR_ISIGS(1 << 27)
+/* Data Storage Interrupt Directed to Guest State */
+#define EPCR_DSIGS(1 << 28)
+/* Instruction TLB Error Interrupt Directed to Guest State */
+#define EPCR_ITLBGS   (1 << 29)
+/* Data TLB Error Interrupt Directed to Guest State */
+#define EPCR_DTLBGS   (1 << 30)
+/* External Input Interrupt Directed to Guest State */
+#define EPCR_EXTGS(1 << 31)
+
 /*/
 /* PowerPC Instructions types definitions*/
 enum {
-- 
1.6.0.2

[Qemu-devel] [PATCH v2 00/11] PPC: e5500 emulation

[Alexander Graf]

This patch set adds support to emulate an e5500 based virtual machine. We don't
have a machine model for that one yet, but with this patch set applied we can
fake the compatibility property of the MPC8544DS model into P5020DS, which
gets guest kernels working for me.

The patch set is based on my recent dynamic device tree work. For a ready to use
git tree, please check here:

  git://repo.or.cz/qemu/agraf.git ppc-e5500

To use the code, grab yourself an e5500 kernel and run:

  $ qemu-system-ppc64 -M mpc8544ds -cpu e5500 -nographic -kernel uImage \
-machine dt_compatible=fsl,,P5020DS

This should get you a working kernel. Everything after that works just the same
as with e500v2 or e500mc.

v1 -> v2:

  - remove reset msr vector
  - clean up ivpr_mask code
  - make MAS2 64bit aware

Alex

Alexander Graf (11):
  dt: make setprop argument static
  PPC: e500: allow users to set the /compatible property via -machine
  uImage: increase the gzip load size
  PPC: Add some booke SPR defines
  PPC: Add support for MSR_CM
  PPC: BookE: Implement EPR SPR
  PPC: BookE: Make ivpr selectable by CPU type
  PPC: Add e5500 CPU target
  PPC: Extract SPR dump generation into its own function
  PPC: BookE: Support 32 and 64 bit wide MAS2
  PPC: BookE206: Bump MAS2 to 64bit

 device_tree.c   |2 +-
 device_tree.h   |2 +-
 hw/loader.c |4 +-
 hw/ppce500_mpc8544ds.c  |   13 +++-
 qemu-config.c   |4 +
 target-ppc/Makefile.objs|1 +
 target-ppc/cpu.h|   34 +-
 target-ppc/excp_helper.c|9 ++-
 target-ppc/helper.h |1 +
 target-ppc/mem_helper.c |2 +-
 target-ppc/mpic_helper.c|   35 ++
 target-ppc/translate.c  |2 +-
 target-ppc/translate_init.c |  154 +--
 13 files changed, 229 insertions(+), 34 deletions(-)
 create mode 100644 target-ppc/mpic_helper.c

[Qemu-devel] [PATCH v2 02/11] PPC: e500: allow users to set the /compatible property via -machine

[Alexander Graf]

Device trees usually have a node /compatible, which indicate which machine
type we're looking at. For quick prototyping, it can be very useful to change
the contents of that node via the command line.

Thus, introduce a new option to -machine called dt_compatible, which when
set changes the /compatible contents to its value.

Signed-off-by: Alexander Graf 
---
 hw/ppce500_mpc8544ds.c |   12 +---
 qemu-config.c  |4 
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/hw/ppce500_mpc8544ds.c b/hw/ppce500_mpc8544ds.c
index f6da25b..d38ad99 100644
--- a/hw/ppce500_mpc8544ds.c
+++ b/hw/ppce500_mpc8544ds.c
@@ -119,7 +119,8 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
 uint32_t clock_freq = 4;
 uint32_t tb_freq = 4;
 int i;
-char compatible[] = "MPC8544DS\0MPC85xxDS";
+const char *compatible = "MPC8544DS\0MPC85xxDS";
+int compatible_len = sizeof("MPC8544DS\0MPC85xxDS");
 char compatible_sb[] = "fsl,mpc8544-immr\0simple-bus";
 char model[] = "MPC8544DS";
 char soc[128];
@@ -144,8 +145,14 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
 
 machine_opts = qemu_opts_find(qemu_find_opts("machine"), 0);
 if (machine_opts) {
+const char *tmp;
 dumpdtb = qemu_opt_get(machine_opts, "dumpdtb");
 dtb_file = qemu_opt_get(machine_opts, "dtb");
+tmp = qemu_opt_get(machine_opts, "dt_compatible");
+if (tmp) {
+compatible = tmp;
+compatible_len = strlen(compatible) + 1;
+}
 }
 
 if (dtb_file) {
@@ -169,8 +176,7 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
 
 /* Manipulate device tree in memory. */
 qemu_devtree_setprop_string(fdt, "/", "model", model);
-qemu_devtree_setprop(fdt, "/", "compatible", compatible,
- sizeof(compatible));
+qemu_devtree_setprop(fdt, "/", "compatible", compatible, compatible_len);
 qemu_devtree_setprop_cell(fdt, "/", "#address-cells", 2);
 qemu_devtree_setprop_cell(fdt, "/", "#size-cells", 2);
 
diff --git a/qemu-config.c b/qemu-config.c
index 2cd2726..5c3296b 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -591,6 +591,10 @@ static QemuOptsList qemu_machine_opts = {
 .name = "phandle_start",
 .type = QEMU_OPT_STRING,
 .help = "The first phandle ID we may generate dynamically",
+}, {
+.name = "dt_compatible",
+.type = QEMU_OPT_STRING,
+.help = "Overrides the \"compatible\" property of the dt root 
node",
 },
 { /* End of list */ }
 },
-- 
1.6.0.2

[Qemu-devel] [PATCH v2 10/11] PPC: BookE: Support 32 and 64 bit wide MAS2

[Alexander Graf]

The MAS registers on BookE are all 32 bit wide, except for MAS2, which
can hold up to 64 bit on 64 bit capable CPUs. Reflect this in the SPR
setting code, so that the guest can never write invalid values in them.

Signed-off-by: Alexander Graf 
---
 target-ppc/translate_init.c |   19 ++-
 1 files changed, 18 insertions(+), 1 deletions(-)

diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 8ff47ae..e6580ff 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -86,6 +86,19 @@ static void spr_write_generic (void *opaque, int sprn, int 
gprn)
 }
 
 #if !defined(CONFIG_USER_ONLY)
+static void spr_write_generic32(void *opaque, int sprn, int gprn)
+{
+#ifdef TARGET_PPC64
+TCGv t0 = tcg_temp_new();
+tcg_gen_ext32u_tl(t0, cpu_gpr[gprn]);
+gen_store_spr(sprn, t0);
+tcg_temp_free(t0);
+spr_store_dump_spr(sprn);
+#else
+spr_write_generic(opaque, sprn, gprn);
+#endif
+}
+
 static void spr_write_clear (void *opaque, int sprn, int gprn)
 {
 TCGv t0 = tcg_temp_new();
@@ -1597,10 +1610,14 @@ static void gen_spr_BookE206(CPUPPCState *env, uint32_t 
mas_mask,
 /* TLB assist registers */
 /* XXX : not implemented */
 for (i = 0; i < 8; i++) {
+void (*uea_write)(void *o, int sprn, int gprn) = &spr_write_generic32;
+if (i == 2 && (mas_mask & (1 << i)) && (env->insns_flags & PPC_64B)) {
+uea_write = &spr_write_generic;
+}
 if (mas_mask & (1 << i)) {
 spr_register(env, mas_sprn[i], mas_names[i],
  SPR_NOACCESS, SPR_NOACCESS,
- &spr_read_generic, &spr_write_generic,
+ &spr_read_generic, uea_write,
  0x);
 }
 }
-- 
1.6.0.2

[Qemu-devel] [PATCH v2 03/11] uImage: increase the gzip load size

[Alexander Graf]

Recent u-boot has different defines for its gzip extract buffer, but the
common ground seems to be 64MB. So let's bump it up to that, enabling me
to load my test image again ;).

Signed-off-by: Alexander Graf 
---
 hw/loader.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/loader.c b/hw/loader.c
index 7d64113..33acc2f 100644
--- a/hw/loader.c
+++ b/hw/loader.c
@@ -377,9 +377,9 @@ static void zfree(void *x, void *addr)
 
 #define DEFLATED   8
 
-/* This is the maximum in uboot, so if a uImage overflows this, it would
+/* This is the usual maximum in uboot, so if a uImage overflows this, it would
  * overflow on real hardware too. */
-#define UBOOT_MAX_GUNZIP_BYTES 0x80
+#define UBOOT_MAX_GUNZIP_BYTES (64 << 20)
 
 static ssize_t gunzip(void *dst, size_t dstlen, uint8_t *src,
   size_t srclen)
-- 
1.6.0.2

[Qemu-devel] [PATCH v2 08/11] PPC: Add e5500 CPU target

[Alexander Graf]

This patch adds e5500's CPU initialization to the TCG CPU initialization
code.

Signed-off-by: Alexander Graf 

---

v1 -> v2:

  - remove reset msr vector
  - clean up ivpr_mask code
---
 target-ppc/translate_init.c |   96 +-
 1 files changed, 93 insertions(+), 3 deletions(-)

diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 98695ab..d185aaa 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -4424,16 +4424,69 @@ static void init_proc_e300 (CPUPPCState *env)
 #define check_pow_e500mc   check_pow_none
 #define init_proc_e500mc   init_proc_e500mc
 
+/* e5500 core 
*/
+#define POWERPC_INSNS_e5500(PPC_INSNS_BASE | PPC_ISEL |
\
+PPC_WRTEE | PPC_RFDI | PPC_RFMCI | 
\
+PPC_CACHE | PPC_CACHE_LOCK | PPC_CACHE_ICBI |  
\
+PPC_CACHE_DCBZ | PPC_CACHE_DCBA |  
\
+PPC_FLOAT | PPC_FLOAT_FRES |   
\
+PPC_FLOAT_FRSQRTE | PPC_FLOAT_FSEL |   
\
+PPC_FLOAT_STFIWX | PPC_WAIT |  
\
+PPC_MEM_TLBSYNC | PPC_TLBIVAX | PPC_MEM_SYNC | 
\
+PPC_64B | PPC_POPCNTB | PPC_POPCNTWD)
+#define POWERPC_INSNS2_e5500   (PPC2_BOOKE206 | PPC2_PRCNTL)
+#define POWERPC_MSRM_e5500 (0x9402FB36ULL)
+#define POWERPC_MMU_e5500  (POWERPC_MMU_BOOKE206)
+#define POWERPC_EXCP_e5500 (POWERPC_EXCP_BOOKE)
+#define POWERPC_INPUT_e5500(PPC_FLAGS_INPUT_BookE)
+/* Fixme: figure out the correct flag for e5500 */
+#define POWERPC_BFDM_e5500 (bfd_mach_ppc_e500)
+#define POWERPC_FLAG_e5500 (POWERPC_FLAG_CE | POWERPC_FLAG_DE | \
+POWERPC_FLAG_PMM | POWERPC_FLAG_BUS_CLK)
+#define check_pow_e5500check_pow_none
+#define init_proc_e5500init_proc_e5500
+
+#if !defined(CONFIG_USER_ONLY)
+static void spr_write_mas73(void *opaque, int sprn, int gprn)
+{
+TCGv val = tcg_temp_new();
+tcg_gen_ext32u_tl(val, cpu_gpr[gprn]);
+gen_store_spr(SPR_BOOKE_MAS3, val);
+tcg_gen_shri_tl(val, gprn, 32);
+gen_store_spr(SPR_BOOKE_MAS7, val);
+tcg_temp_free(val);
+}
+
+static void spr_read_mas73(void *opaque, int gprn, int sprn)
+{
+TCGv mas7 = tcg_temp_new();
+TCGv mas3 = tcg_temp_new();
+gen_load_spr(mas7, SPR_BOOKE_MAS7);
+tcg_gen_shli_tl(mas7, mas7, 32);
+gen_load_spr(mas3, SPR_BOOKE_MAS3);
+tcg_gen_or_tl(cpu_gpr[gprn], mas3, mas7);
+tcg_temp_free(mas3);
+tcg_temp_free(mas7);
+}
+
+static void spr_load_epr(void *opaque, int gprn, int sprn)
+{
+gen_helper_load_epr(cpu_gpr[gprn], cpu_env);
+}
+
+#endif
+
 enum fsl_e500_version {
 fsl_e500v1,
 fsl_e500v2,
 fsl_e500mc,
+fsl_e5500,
 };
 
 static void init_proc_e500 (CPUPPCState *env, int version)
 {
 uint32_t tlbncfg[2];
-uint64_t ivor_mask = 0x000FULL;
+uint64_t ivor_mask;
 uint64_t ivpr_mask = 0xULL;
 uint32_t l1cfg0 = 0x3800  /* 8 ways */
 | 0x0020; /* 32 kb */
@@ -4448,8 +4501,16 @@ static void init_proc_e500 (CPUPPCState *env, int 
version)
  * complain when accessing them.
  * gen_spr_BookE(env, 0x000FFD7FULL);
  */
-if (version == fsl_e500mc) {
-ivor_mask = 0x03FEULL;
+switch (version) {
+case fsl_e500v1:
+case fsl_e500v2:
+default:
+ivor_mask = 0x000FULL;
+break;
+case fsl_e500mc:
+case fsl_e5500:
+ivor_mask = 0x03FEULL;
+break;
 }
 gen_spr_BookE(env, ivor_mask);
 /* Processor identification */
@@ -4477,6 +4538,7 @@ static void init_proc_e500 (CPUPPCState *env, int version)
 tlbncfg[1] = gen_tlbncfg(16, 1, 12, TLBnCFG_AVAIL | TLBnCFG_IPROT, 16);
 break;
 case fsl_e500mc:
+case fsl_e5500:
 tlbncfg[0] = gen_tlbncfg(4, 1, 1, 0, 512);
 tlbncfg[1] = gen_tlbncfg(64, 1, 12, TLBnCFG_AVAIL | TLBnCFG_IPROT, 64);
 break;
@@ -4492,6 +4554,7 @@ static void init_proc_e500 (CPUPPCState *env, int version)
 env->icache_line_size = 32;
 break;
 case fsl_e500mc:
+case fsl_e5500:
 env->dcache_line_size = 64;
 env->icache_line_size = 64;
 l1cfg0 |= 0x100; /* 64 byte cache block size */
@@ -4567,6 +4630,22 @@ static void init_proc_e500 (CPUPPCState *env, int 
version)
  SPR_NOACCESS, SPR_NOACCESS,
  &spr_read_generic, &spr_write_booke206_mmucsr0,
  0x);
+spr_register(env, SPR_BOOKE_EPR, "EPR",
+ SPR_NOACCESS, SPR_NOACCESS,
+ &spr_load_epr, SPR_NOACCESS,
+ 0x);
+ 

[Qemu-devel] [PATCH v2 06/11] PPC: BookE: Implement EPR SPR

[Alexander Graf]

On the e500 series, accessing SPR_EPR magically turns into an access at
that CPU's IACK register on the MPIC. Implement that logic to get kernels
that make use of that feature work.

Signed-off-by: Alexander Graf 
---
 hw/ppce500_mpc8544ds.c   |1 +
 target-ppc/Makefile.objs |1 +
 target-ppc/cpu.h |1 +
 target-ppc/helper.h  |1 +
 target-ppc/mpic_helper.c |   35 +++
 5 files changed, 39 insertions(+), 0 deletions(-)
 create mode 100644 target-ppc/mpic_helper.c

diff --git a/hw/ppce500_mpc8544ds.c b/hw/ppce500_mpc8544ds.c
index d38ad99..8b9fd83 100644
--- a/hw/ppce500_mpc8544ds.c
+++ b/hw/ppce500_mpc8544ds.c
@@ -469,6 +469,7 @@ static void mpc8544ds_init(ram_addr_t ram_size,
 irqs[i][OPENPIC_OUTPUT_INT] = input[PPCE500_INPUT_INT];
 irqs[i][OPENPIC_OUTPUT_CINT] = input[PPCE500_INPUT_CINT];
 env->spr[SPR_BOOKE_PIR] = env->cpu_index = i;
+env->mpic_cpu_base = MPC8544_MPIC_REGS_BASE + 0x2;
 
 ppc_booke_timers_init(env, 4, PPC_TIMER_E500);
 
diff --git a/target-ppc/Makefile.objs b/target-ppc/Makefile.objs
index 6c11ef8..237a0ed 100644
--- a/target-ppc/Makefile.objs
+++ b/target-ppc/Makefile.objs
@@ -9,3 +9,4 @@ obj-y += mmu_helper.o
 obj-y += timebase_helper.o
 obj-y += misc_helper.o
 obj-y += mem_helper.o
+obj-y += mpic_helper.o
diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 7a77fff..652a35a 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -1066,6 +1066,7 @@ struct CPUPPCState {
 target_ulong ivor_mask;
 target_ulong ivpr_mask;
 target_ulong hreset_vector;
+target_phys_addr_t mpic_cpu_base;
 #endif
 
 /* Those resources are used only during code translation */
diff --git a/target-ppc/helper.h b/target-ppc/helper.h
index ddab97b..fd04c06 100644
--- a/target-ppc/helper.h
+++ b/target-ppc/helper.h
@@ -405,6 +405,7 @@ DEF_HELPER_2(store_40x_dbcr0, void, env, tl)
 DEF_HELPER_2(store_40x_sler, void, env, tl)
 DEF_HELPER_2(store_booke_tcr, void, env, tl)
 DEF_HELPER_2(store_booke_tsr, void, env, tl)
+DEF_HELPER_1(load_epr, tl, env)
 DEF_HELPER_3(store_ibatl, void, env, i32, tl)
 DEF_HELPER_3(store_ibatu, void, env, i32, tl)
 DEF_HELPER_3(store_dbatl, void, env, i32, tl)
diff --git a/target-ppc/mpic_helper.c b/target-ppc/mpic_helper.c
new file mode 100644
index 000..2c6a4d3
--- /dev/null
+++ b/target-ppc/mpic_helper.c
@@ -0,0 +1,35 @@
+/*
+ *  PowerPC emulation helpers for QEMU.
+ *
+ *  Copyright (c) 2003-2007 Jocelyn Mayer
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see .
+ */
+#include "cpu.h"
+#include "helper.h"
+
+/*/
+/* SPR accesses */
+
+#if !defined(CONFIG_USER_ONLY)
+/*
+ * This is an ugly helper for EPR, which is basically the same as accessing
+ * the IACK (PIAC) register on the MPIC. Because we model the MPIC as a device
+ * that can only talk to the CPU through MMIO, let's access it that way!
+ */
+target_ulong helper_load_epr(CPUPPCState *env)
+{
+return ldl_phys(env->mpic_cpu_base + 0xA0);
+}
+#endif
-- 
1.6.0.2

[Qemu-devel] [PATCH v2 01/11] dt: make setprop argument static

[Alexander Graf]

Whatever we pass in to qemu_devtree_setprop to put into the device tree
will not get modified by that function, so it can easily be declared const.

Signed-off-by: Alexander Graf 
---
 device_tree.c |2 +-
 device_tree.h |2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/device_tree.c b/device_tree.c
index acae53e..b366fdd 100644
--- a/device_tree.c
+++ b/device_tree.c
@@ -127,7 +127,7 @@ static int findnode_nofail(void *fdt, const char *node_path)
 }
 
 int qemu_devtree_setprop(void *fdt, const char *node_path,
- const char *property, void *val_array, int size)
+ const char *property, const void *val_array, int size)
 {
 int r;
 
diff --git a/device_tree.h b/device_tree.h
index 4898d95..2244270 100644
--- a/device_tree.h
+++ b/device_tree.h
@@ -18,7 +18,7 @@ void *create_device_tree(int *sizep);
 void *load_device_tree(const char *filename_path, int *sizep);
 
 int qemu_devtree_setprop(void *fdt, const char *node_path,
- const char *property, void *val_array, int size);
+ const char *property, const void *val_array, int 
size);
 int qemu_devtree_setprop_cell(void *fdt, const char *node_path,
   const char *property, uint32_t val);
 int qemu_devtree_setprop_u64(void *fdt, const char *node_path,
-- 
1.6.0.2

Re: [Qemu-devel] [PATCH 2/2] kvm: use per-cpu lock to free vcpu thread out of the big lock

[Jan Kiszka]

On 2012-06-21 16:49, Liu Ping Fan wrote:
> In order to break the big lock, using per-cpu_lock in kvm_cpu_exec()
> to protect the race from other cpu's access to env->apic_state & related
> field in env.
> Also, we need to protect agaist run_on_cpu().
> 
> Race condition can be like this:
> 1.  vcpu-1 IPI vcpu-2
> vcpu-3 IPI vcpu-2
> Open window exists for accessing to vcpu-2's apic_state & env
> 
> 2. run_on_cpu() write env->queued_work_last, while flush_queued_work()
>read
> 

How much of this is still relevant with the (nowadays default-on)
in-kernel irqchips?

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

[Qemu-devel] [RFC] use little granularity lock to substitue qemu_mutex_lock_iothread

[Liu Ping Fan]

Nowadays, we use qemu_mutex_lock_iothread()/qemu_mutex_unlock_iothread() to
protect the race to access the emulated dev launched by vcpu threads & iothread.

But this lock is too big. We can break it down.
These patches separate the CPUArchState's protection from the other devices, so 
we
can have a per-cpu lock for each CPUArchState, not the big lock any longer.

[Qemu-devel] [PATCH 1/2] CPUArchState: introduce per-cpu lock

[Liu Ping Fan]

introduce a lock for per-cpu to protect agaist accesing from
other vcpu thread.

Signed-off-by: Liu Ping Fan 
---
 cpu-defs.h  |2 ++
 cpus.c  |   17 +
 main-loop.h |3 +++
 3 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/cpu-defs.h b/cpu-defs.h
index f49e950..7305822 100644
--- a/cpu-defs.h
+++ b/cpu-defs.h
@@ -30,6 +30,7 @@
 #include "osdep.h"
 #include "qemu-queue.h"
 #include "targphys.h"
+#include "qemu-thread-posix.h"
 
 #ifndef TARGET_LONG_BITS
 #error TARGET_LONG_BITS must be defined before including this header
@@ -220,6 +221,7 @@ typedef struct CPUWatchpoint {
 CPU_COMMON_THREAD   \
 struct QemuCond *halt_cond; \
 int thread_kicked;  \
+struct QemuMutex *cpu_lock; \
 struct qemu_work_item *queued_work_first, *queued_work_last;\
 const char *cpu_model_str;  \
 struct KVMState *kvm_state; \
diff --git a/cpus.c b/cpus.c
index b182b3d..554f7bc 100644
--- a/cpus.c
+++ b/cpus.c
@@ -735,6 +735,7 @@ static void *qemu_kvm_cpu_thread_fn(void *arg)
 env->thread_id = qemu_get_thread_id();
 cpu_single_env = env;
 
+
 r = kvm_init_vcpu(env);
 if (r < 0) {
 fprintf(stderr, "kvm_init_vcpu failed: %s\n", strerror(-r));
@@ -891,6 +892,20 @@ int qemu_cpu_is_self(void *_env)
 return qemu_thread_is_self(env->thread);
 }
 
+void qemu_mutex_lock_cpu(void *_env)
+{
+CPUArchState *env = _env;
+
+qemu_mutex_lock(env->cpu_lock);
+}
+
+void qemu_mutex_unlock_cpu(void *_env)
+{
+CPUArchState *env = _env;
+
+qemu_mutex_unlock(env->cpu_lock);
+}
+
 void qemu_mutex_lock_iothread(void)
 {
 if (!tcg_enabled()) {
@@ -1027,6 +1042,8 @@ void qemu_init_vcpu(void *_env)
 env->nr_cores = smp_cores;
 env->nr_threads = smp_threads;
 env->stopped = 1;
+env->cpu_lock = g_malloc0(sizeof(QemuMutex));
+qemu_mutex_init(env->cpu_lock);
 if (kvm_enabled()) {
 qemu_kvm_start_vcpu(env);
 } else if (tcg_enabled()) {
diff --git a/main-loop.h b/main-loop.h
index dce1cd9..d8d44a4 100644
--- a/main-loop.h
+++ b/main-loop.h
@@ -323,6 +323,9 @@ void qemu_bh_delete(QEMUBH *bh);
 int qemu_add_child_watch(pid_t pid);
 #endif
 
+void qemu_mutex_lock_cpu(void *_env);
+void qemu_mutex_unlock_cpu(void *_env);
+
 /**
  * qemu_mutex_lock_iothread: Lock the main loop mutex.
  *
-- 
1.7.4.4

[Qemu-devel] [PATCH 2/2] kvm: use per-cpu lock to free vcpu thread out of the big lock

[Liu Ping Fan]

In order to break the big lock, using per-cpu_lock in kvm_cpu_exec()
to protect the race from other cpu's access to env->apic_state & related
field in env.
Also, we need to protect agaist run_on_cpu().

Race condition can be like this:
1.  vcpu-1 IPI vcpu-2
vcpu-3 IPI vcpu-2
Open window exists for accessing to vcpu-2's apic_state & env

2. run_on_cpu() write env->queued_work_last, while flush_queued_work()
   read

Signed-off-by: Liu Ping Fan 
---
 cpus.c|6 --
 hw/apic.c |   58 ++
 hw/pc.c   |8 +++-
 kvm-all.c |   13 +++--
 4 files changed, 76 insertions(+), 9 deletions(-)

diff --git a/cpus.c b/cpus.c
index 554f7bc..ac99afe 100644
--- a/cpus.c
+++ b/cpus.c
@@ -649,6 +649,7 @@ void run_on_cpu(CPUArchState *env, void (*func)(void 
*data), void *data)
 
 wi.func = func;
 wi.data = data;
+qemu_mutex_lock(env->cpu_lock);
 if (!env->queued_work_first) {
 env->queued_work_first = &wi;
 } else {
@@ -657,6 +658,7 @@ void run_on_cpu(CPUArchState *env, void (*func)(void 
*data), void *data)
 env->queued_work_last = &wi;
 wi.next = NULL;
 wi.done = false;
+qemu_mutex_unlock(env->cpu_lock);
 
 qemu_cpu_kick(env);
 while (!wi.done) {
@@ -718,7 +720,7 @@ static void qemu_tcg_wait_io_event(void)
 static void qemu_kvm_wait_io_event(CPUArchState *env)
 {
 while (cpu_thread_is_idle(env)) {
-qemu_cond_wait(env->halt_cond, &qemu_global_mutex);
+qemu_cond_wait(env->halt_cond, env->cpu_lock);
 }
 
 qemu_kvm_eat_signals(env);
@@ -729,8 +731,8 @@ static void *qemu_kvm_cpu_thread_fn(void *arg)
 {
 CPUArchState *env = arg;
 int r;
+qemu_mutex_lock_cpu(env);
 
-qemu_mutex_lock(&qemu_global_mutex);
 qemu_thread_get_self(env->thread);
 env->thread_id = qemu_get_thread_id();
 cpu_single_env = env;
diff --git a/hw/apic.c b/hw/apic.c
index 4eeaf88..b999a40 100644
--- a/hw/apic.c
+++ b/hw/apic.c
@@ -22,6 +22,7 @@
 #include "host-utils.h"
 #include "trace.h"
 #include "pc.h"
+#include "qemu-thread.h"
 
 #define MAX_APIC_WORDS 8
 
@@ -94,6 +95,7 @@ static int get_highest_priority_int(uint32_t *tab)
 return -1;
 }
 
+/* Caller must hold the lock */
 static void apic_sync_vapic(APICCommonState *s, int sync_type)
 {
 VAPICState vapic_state;
@@ -141,11 +143,13 @@ static void apic_sync_vapic(APICCommonState *s, int 
sync_type)
 }
 }
 
+/* Caller must hold lock */
 static void apic_vapic_base_update(APICCommonState *s)
 {
 apic_sync_vapic(s, SYNC_TO_VAPIC);
 }
 
+/* Caller must hold the lock */
 static void apic_local_deliver(APICCommonState *s, int vector)
 {
 uint32_t lvt = s->lvt[vector];
@@ -175,9 +179,11 @@ static void apic_local_deliver(APICCommonState *s, int 
vector)
 (lvt & APIC_LVT_LEVEL_TRIGGER))
 trigger_mode = APIC_TRIGGER_LEVEL;
 apic_set_irq(s, lvt & 0xff, trigger_mode);
+break;
 }
 }
 
+/* Caller must hold the lock */
 void apic_deliver_pic_intr(DeviceState *d, int level)
 {
 APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
@@ -200,9 +206,12 @@ void apic_deliver_pic_intr(DeviceState *d, int level)
 }
 }
 
+/* Must hold lock */
 static void apic_external_nmi(APICCommonState *s)
 {
+qemu_mutex_lock_cpu(s->cpu_env);
 apic_local_deliver(s, APIC_LVT_LINT1);
+qemu_mutex_unlock_cpu(s->cpu_env);
 }
 
 #define foreach_apic(apic, deliver_bitmask, code) \
@@ -215,7 +224,9 @@ static void apic_external_nmi(APICCommonState *s)
 if (__mask & (1 << __j)) {\
 apic = local_apics[__i * 32 + __j];\
 if (apic) {\
+qemu_mutex_lock_cpu(apic->cpu_env);\
 code;\
+qemu_mutex_unlock_cpu(apic->cpu_env);\
 }\
 }\
 }\
@@ -244,7 +255,9 @@ static void apic_bus_deliver(const uint32_t 
*deliver_bitmask,
 if (d >= 0) {
 apic_iter = local_apics[d];
 if (apic_iter) {
+qemu_mutex_lock_cpu(apic_iter->cpu_env);
 apic_set_irq(apic_iter, vector_num, trigger_mode);
+qemu_mutex_unlock_cpu(apic_iter->cpu_env);
 }
 }
 }
@@ -293,6 +306,7 @@ void apic_deliver_irq(uint8_t dest, uint8_t dest_mode, 
uint8_t delivery_mode,
 apic_bus_deliver(deliver_bitmask, delivery_mode, vector_num, trigger_mode);
 }
 
+/* Caller must hold lock */
 static void apic_set_base(APICCommonState *s, uint64_t val)
 {
 s->apicbase = (val & 0xf000) |
@@ -305,6 +319,7 @@ static void apic_set_base(APICCommonState *s, uint64_t val)
 }
 }
 
+/* caller must hold lock */
 static void apic_set_tpr(APICCommonState *s, uint8_t val)
 {
 /* Updates from cr8 are ignored while the VAPIC is active */
@@ -314,12 +329,14 @@ static void apic_set_tpr(APICCommonStat

Re: [Qemu-devel] [Bug 1013888] Re: windows xp sp3 setup blank screen on boot

[Michael Roth]

On Fri, Jun 15, 2012 at 11:49:36PM -, Michael Sabino wrote:
> Qemu 1.0.1 - Doesn't have a problem 
> Qemu 1.1.0 - has the problem
> Qemu master commit eb2aeacf983a2a88a2b31e8fee067c38bd10abd3 - has the problem

I was also able to reproduce with commit:

eb2aeacf983a2a88a2b31e8fee067c38bd10abd3

The problem appears to have been fixed upstream though. A reverse bisect
points to this patch being the fix:

commit c52acf60b6c12ff5eb58eb6ac568c159ae0c8737
Author: Pavel Hrdina 
Date:   Wed Jun 13 15:43:11 2012 +0200

fdc: fix implied seek while there is no media in drive

The Windows uses 'READ' command at the start of an instalation
without checking the 'dir' register. We have to abort the transfer
with an abnormal termination if there is no media in the drive.

Signed-off-by: Pavel Hrdina 
Signed-off-by: Kevin Wolf 

Please try your scenario again using that commit, and if all if it does the
trick we'll get it included in the next stable-1.1 release.

> 
> -- 
> You received this bug notification because you are a member of qemu-
> devel-ml, which is subscribed to QEMU.
> https://bugs.launchpad.net/bugs/1013888
> 
> Title:
>   windows xp sp3 setup blank screen on boot
> 
> Status in QEMU:
>   New
> 
> Bug description:
>   When attempting to run Windows XP SP3 setup in qemu on a Lubuntu host
>   with the following kernel:
> 
>   Linux michael-XPS-M1530 3.2.0-23-generic #36-Ubuntu SMP Tue Apr 10
>   20:39:51 UTC 2012 x86_64 x86_64 x86_64 GNU/Linux
> 
>   Qemu does not get past a blank screen after "Setup is inspecting your
>   computer's hardware configuration"
> 
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/qemu/+bug/1013888/+subscriptions
> 

Re: [Qemu-devel] [RFC] use little granularity lock to substitue qemu_mutex_lock_iothread

[Jan Kiszka]

On 2012-06-21 16:49, Liu Ping Fan wrote:
> Nowadays, we use qemu_mutex_lock_iothread()/qemu_mutex_unlock_iothread() to
> protect the race to access the emulated dev launched by vcpu threads & 
> iothread.
> 
> But this lock is too big. We can break it down.
> These patches separate the CPUArchState's protection from the other devices, 
> so we
> can have a per-cpu lock for each CPUArchState, not the big lock any longer.

Anything that reduces lock dependencies is generally welcome. But can
you specify in more details what you gain, and under which conditions?

I'm skeptical if this is the right area to start. With the in-kernel
irqchip enabled, no CPUArchState field is touched during normal
operations (unless I missed something subtle in the past). At the same
time, this locking is unfortunately fairly complex and invasive, so not
"cheap" to integrate.

IMO more interesting is breaking out some I/O path, e.g. from a NIC to a
network backend, and get this processed in a separate thread without
touching the BQL (Big QEMU Lock). I've experimental patches for this
here, but they need rebasing and polishing.

Thanks,
Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH] msi/msix: added public API to set/get MSI message address, and data

[Alex Williamson]

On Thu, 2012-06-21 at 12:56 +0200, Jan Kiszka wrote:
> On 2012-06-21 12:50, Alexey Kardashevskiy wrote:
> > On 21/06/12 20:38, Jan Kiszka wrote:
> >> On 2012-06-21 12:28, Alexey Kardashevskiy wrote:
> >>> On 21/06/12 17:39, Jan Kiszka wrote:
>  On 2012-06-21 09:18, Alexey Kardashevskiy wrote:
> >
> > agrhhh. sha1 of the patch changed after rebasing :)
> >
> >
> >
> > Added (msi|msix)_(set|get)_message() function for whoever might
> > want to use them.
> >
> > Currently msi_notify()/msix_notify() write to these vectors to
> > signal the guest about an interrupt so the correct values have to
> > written there by the guest or QEMU.
> >
> > For example, POWER guest never initializes MSI/MSIX vectors, instead
> > it uses RTAS hypercalls. So in order to support MSIX for virtio-pci on
> > POWER we have to initialize MSI/MSIX message from QEMU.
> >
> > As only set* function are required by now, the "get" functions were 
> > added
> > or made public for a symmetry.
> >
> > Signed-off-by: Alexey Kardashevskiy 
> > ---
> >  hw/msi.c  |   29 +
> >  hw/msi.h  |2 ++
> >  hw/msix.c |   11 ++-
> >  hw/msix.h |3 +++
> >  4 files changed, 44 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/msi.c b/hw/msi.c
> > index 5233204..9ad84a4 100644
> > --- a/hw/msi.c
> > +++ b/hw/msi.c
> > @@ -105,6 +105,35 @@ static inline uint8_t msi_pending_off(const 
> > PCIDevice* dev, bool msi64bit)
> >  return dev->msi_cap + (msi64bit ? PCI_MSI_PENDING_64 : 
> > PCI_MSI_PENDING_32);
> >  }
> >  
> > +MSIMessage msi_get_message(PCIDevice *dev)
> 
>  MSIMessage msi_get_message(PCIDevice *dev, unsigned vector)
> >>>
> >>>
> >>> Who/how/why is going to calculate the vector here?
> >>>
> 
> > +{
> > +uint16_t flags = pci_get_word(dev->config + msi_flags_off(dev));
> > +bool msi64bit = flags & PCI_MSI_FLAGS_64BIT;
> > +MSIMessage msg;
> > +
> > +if (msi64bit) {
> > +msg.address = pci_get_quad(dev->config + 
> > msi_address_lo_off(dev));
> > +} else {
> > +msg.address = pci_get_long(dev->config + 
> > msi_address_lo_off(dev));
> > +}
> > +msg.data = pci_get_word(dev->config + msi_data_off(dev, msi64bit));
> 
>  And I have this here in addition:
> 
>  unsigned int nr_vectors = msi_nr_vectors(flags);
>  ...
> 
>  if (nr_vectors > 1) {
>  msg.data &= ~(nr_vectors - 1);
>  msg.data |= vector;
>  }
> 
>  See PCI spec and existing code.
> >>>
> >>>
> >>> What for? I really do not get it why someone might want to read something 
> >>> but not real value.
> >>> What PCI code should I look?
> >>
> >> I'm not sure what your use case for reading the message is. For KVM
> >> device assignment it is preparing an alternative message delivery path
> >> for MSI vectors. And for this we will need vector notifier support for
> >> MSI as well. You can check the MSI-X code for corresponding use cases of
> >> msix_get_message.
> > 
> >> And when we already have msi_get_message, another logical use case is
> >> msi_notify. See msix.c again.
> > 
> > Aaaa.
> > 
> > I have no case for reading the message. All I need is writing. And I want 
> > it public as I want to use
> > it from hw/spapr_pci.c. You suggested to add reading, I added "get" to be 
> > _symmetric_ to "set"
> > ("get" returns what "set" wrote). You want a different thing which I can do 
> > but it is not
> > msi_get_message(), it is something like msi_prepare_message(MSImessage msg) 
> > or
> > msi_set_vector(uint16_t data) or simply internal kitchen of msi_notify().
> > 
> > Still can do what you suggested, it just does not seem right.
> 
> It is right - when looking at it from a different angle. ;)
> 
> I don't mind if you add msi_get_message now or leave this to me. Likely
> the latter is better as you have no use case for msi_get_message (and
> also msix_get_message!) outside of their modules, thus we should not
> export those functions anyway.

Curse my timezone for not getting to respond before Alexey dropped the
'get' function.  msi_get_message(PCIDevice *dev, int vector) is
necessary for vfio so that we can make use of irqfds in KVM.  I've
actually got this function coded into vfio for the moment because it's
so much fun to get pci changes into qemu.  When MSI gets enabled, we
setup an eventfd in vfio for each vector (linux only supports a single
MSI vector, but we pretend we can support the fully specification).  If
we can enabled irqfd, we walk through each vector, getting an MSIMessage
to program into KVM, which sets the MSI to be injected when the eventfd
fires.  MSI is non-symmetric because the vectors have to be consecutive.
So it doesn't make sense to have a 'set' with a vector number, but a
'get' wit

Re: [Qemu-devel] hw/Makefile.objs question

[Alex Williamson]

On Thu, 2012-06-21 at 16:04 +0200, Andreas Färber wrote:
> Am 21.06.2012 15:10, schrieb Alexey Kardashevskiy:
> > On 21/06/12 22:19, Andreas Färber wrote:
> >> Am 21.06.2012 13:21, schrieb Alexey Kardashevskiy:
> >>> On 21/06/12 20:36, Andreas Färber wrote:
>  Am 21.06.2012 05:22, schrieb Alexey Kardashevskiy:
> > I am trying to compile the very last qemu with vfio_pci enabled. 
> > VFIO_PCI is added as below:
> >
> > ./configure:
> >
> >  case "$target_arch2" in
> >   i386|x86_64|ppc64)
> >  if test "$vfio_pci" = "yes" -a "$target_softmmu" = "yes" ; then
> >echo "CONFIG_VFIO_PCI=y" >> $config_target_mak
> >  fi
> >  esac
> >
> >
> > ./Makefile.target:
> >
> >  # VFIO PCI device assignment
> > obj-$(CONFIG_VFIO_PCI) += vfio_pci.o
> >
> >
> > And it worked before. However it does not anymore as it seems that 
> > everything in hw/ (and vfio_pci.c
> > as well as is in hw/ and it is a device) can be only compiled via 
> > hw/Makefile.objs and
> > hw/ppc/Makefile.objs (my platform is POWER), it is ignored if to keep 
> > it as is.
> >
> > So I have to move "obj-$(CONFIG_VFIO_PCI) += vfio_pci.o" to 
> > hw/Makefile.objs (and change obj- to
> > hw-obj-) but the hw/Makefile.objs does not include (directly or 
> > indirectly) generated
> > ppc64-softmmu/config-target.mak with CONFIG_VFIO_PCI=y.
> >
> > What is the correct solution?
> 
>  If the file compiles the same for all three, put CONFIG_VFIO_PCI=y into
>  default-configs/{i386,x86_64,ppc64}-softmmu.mak and do
>  hw-obj-$(CONFIG_VFIO_PCI) += in hw/Makefile.objs.
> >>>
> >>>
> >>> It only compiles with ./configure --enable-vfio-pci which may or may not 
> >>> set CONFIG_VFIO_PCI to "y".
> >>> Your proposal makes it always "y" (for selected platforms).
> >>
> >> Apply some creativity, there's surely examples around. The question is
> >> whether the contents of vfio_pci.o changes or not.
> > 
> > Applied already and gave up, this is why I am writing here :)
> > What would be a good example of a device which is enabled by configure 
> > script?
> 
> You're missing my point: We need to know *where* the .o file needs to
> go, then we can point you to appropriate examples. Making things depend
> on configure options should be trivial from there.
> 
> My understanding is that VFIO depends only on Linux/KVM support so I
> don't understand why you'd be excluding VFIO for ppc. s390 has no PCI
> AFAIU so that'd be okay.

FWIW, VFIO really only depends on Linux + PCI, but we probably want to
set defaults so that it doesn't get build on platforms where we know it
won't work (no INTx EOI lookup/notifiers).  Thanks,

Alex

> >> If not, then you only
> >> need to build it once in libhwX/, depending on $config_target_mak, and
> >> link to the appropriate targets. If it accesses CPU internals then it
> >> must be built per target.
> > 
> > $config_target_mak points to ppc64-softmmu/config-target.mak, not in the 
> > root folder.
> > When executed in libhw64, it is not visible to makefile.
> 
> Correct. You keep mixing up both approaches I named...
> 
> *If* the file is built per target (hw/ppc64/Makefile.objs), then you can
> use *-softmmu/config-target.mak and just need to use a different
> Makefile than before.
> 
> *If* the file is built per libhw (hw/Makefile.objs), then you need one
> option whether to compile it and another for whether to link it into a
> particular target.
> 
> You still haven't answered the question of which of these two cases
> applies here, so I cannot say more than I already have. Anthony's 3)
> elaborates on my briefly mentioned ifeq.
> 
> Andreas
> 
>  Otherwise, add to hw/{i386,ppc}/Makefile.objs - or with Anthony's
>  proposal from yesterday hw/Makefile.objs becomes possible, too.
> >>>
> >>> Again, it will be unconditional "y".
> >>
> >> No, in this case the condition would be set from configure as before, it
> >> only moves from Makefile.target to the appropriate Makefile.objs.
> >> Note that to limit it to ppc64 (as opposed to ppc) some additional ifeq
> >> check would be needed, as before.
> 

Re: [Qemu-devel] [PATCH v2 10/11] PPC: BookE: Support 32 and 64 bit wide MAS2

[Scott Wood]

On 06/21/2012 08:33 AM, Alexander Graf wrote:
> The MAS registers on BookE are all 32 bit wide, except for MAS2, which
> can hold up to 64 bit on 64 bit capable CPUs. Reflect this in the SPR
> setting code, so that the guest can never write invalid values in them.
> 
> Signed-off-by: Alexander Graf 
> ---
>  target-ppc/translate_init.c |   19 ++-
>  1 files changed, 18 insertions(+), 1 deletions(-)
> 
> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
> index 8ff47ae..e6580ff 100644
> --- a/target-ppc/translate_init.c
> +++ b/target-ppc/translate_init.c
> @@ -86,6 +86,19 @@ static void spr_write_generic (void *opaque, int sprn, int 
> gprn)
>  }
>  
>  #if !defined(CONFIG_USER_ONLY)
> +static void spr_write_generic32(void *opaque, int sprn, int gprn)
> +{
> +#ifdef TARGET_PPC64
> +TCGv t0 = tcg_temp_new();
> +tcg_gen_ext32u_tl(t0, cpu_gpr[gprn]);
> +gen_store_spr(sprn, t0);
> +tcg_temp_free(t0);
> +spr_store_dump_spr(sprn);
> +#else
> +spr_write_generic(opaque, sprn, gprn);
> +#endif
> +}
> +
>  static void spr_write_clear (void *opaque, int sprn, int gprn)
>  {
>  TCGv t0 = tcg_temp_new();
> @@ -1597,10 +1610,14 @@ static void gen_spr_BookE206(CPUPPCState *env, 
> uint32_t mas_mask,
>  /* TLB assist registers */
>  /* XXX : not implemented */
>  for (i = 0; i < 8; i++) {
> +void (*uea_write)(void *o, int sprn, int gprn) = 
> &spr_write_generic32;
> +if (i == 2 && (mas_mask & (1 << i)) && (env->insns_flags & PPC_64B)) 
> {
> +uea_write = &spr_write_generic;
> +}
>  if (mas_mask & (1 << i)) {
>  spr_register(env, mas_sprn[i], mas_names[i],
>   SPR_NOACCESS, SPR_NOACCESS,
> - &spr_read_generic, &spr_write_generic,
> + &spr_read_generic, uea_write,
>   0x);
>  }

What does "uea" mean?

-Scott

[Qemu-devel] [PULL] Xen PCI Passthrough

[Stefano Stabellini]

Anthony,
please pull Anthony Perard's Xen PCI Passthrough series from:

git://xenbits.xen.org/people/sstabellini/qemu-dm.git xen-pt

All the generic patches have been acked by Michael, the Xen patches have
been reviewed by Konrad and me.



Allen Kay (2):
  Introduce Xen PCI Passthrough, qdevice
  Introduce Xen PCI Passthrough, PCI config space helpers

Anthony PERARD (6):
  pci_ids: Add INTEL_82599_SFP_VF id.
  configure: Introduce --enable-xen-pci-passthrough.
  Introduce XenHostPCIDevice to access a pci device on the host.
  pci.c: Add opaque argument to pci_for_each_device.
  qdev-properties: Introduce pci-host-devaddr.
  Introduce apic-msidef.h

Jiang Yunhong (1):
  Introduce Xen PCI Passthrough, MSI

 configure|   29 +
 hw/apic-msidef.h |   30 +
 hw/apic.c|   11 +-
 hw/i386/Makefile.objs|2 +
 hw/pci.c |   11 +-
 hw/pci.h |4 +-
 hw/pci_ids.h |1 +
 hw/qdev-properties.c |  107 +++
 hw/qdev.h|3 +
 hw/xen-host-pci-device.c |  396 ++
 hw/xen-host-pci-device.h |   55 ++
 hw/xen_common.h  |3 +
 hw/xen_platform.c|8 +-
 hw/xen_pt.c  |  851 +
 hw/xen_pt.h  |  301 
 hw/xen_pt_config_init.c  | 1869 ++
 hw/xen_pt_msi.c  |  620 +++
 qemu-common.h|7 +
 xen-all.c|   12 +
 19 files changed, 4301 insertions(+), 19 deletions(-)

Cheers,

Stefano

[Qemu-devel] [PULL] Xen compile fixes

[Stefano Stabellini]

Anthony,
please pull a couple of small Xen compile fixes to compile against
xen-unstable:

git://xenbits.xen.org/people/sstabellini/qemu-dm.git compile-xs


Anthony PERARD (2):
  xen: Reorganize includes of Xen headers.
  xenstore: Use 

 configure|2 +-
 hw/xen_backend.c |6 ++
 hw/xen_common.h  |6 +-
 hw/xen_console.c |5 ++---
 hw/xen_disk.c|6 +-
 hw/xen_nic.c |7 ++-
 hw/xenfb.c   |   13 +
 7 files changed, 18 insertions(+), 27 deletions(-)

Cheers,

Stefano

[Qemu-devel] [Bug 1013888] Re: windows xp sp3 setup blank screen on boot

[Luigi Tarenga]

I confirm it works.
just compiled from commit c52acf60b6c12ff5eb58eb6ac568c159ae0c8737.
Windows XP SP3 installation iso boot and start installation process.

I tested both i368-softmmu and x86_64-softmmu targets.

thanks
Luigi

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

Title:
  windows xp sp3 setup blank screen on boot

Status in QEMU:
  New

Bug description:
  When attempting to run Windows XP SP3 setup in qemu on a Lubuntu host
  with the following kernel:

  Linux michael-XPS-M1530 3.2.0-23-generic #36-Ubuntu SMP Tue Apr 10
  20:39:51 UTC 2012 x86_64 x86_64 x86_64 GNU/Linux

  Qemu does not get past a blank screen after "Setup is inspecting your
  computer's hardware configuration"

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

Re: [Qemu-devel] [PATCH v2 10/11] PPC: BookE: Support 32 and 64 bit wide MAS2

[Alexander Graf]

On 21.06.2012, at 18:04, Scott Wood wrote:

> On 06/21/2012 08:33 AM, Alexander Graf wrote:
>> The MAS registers on BookE are all 32 bit wide, except for MAS2, which
>> can hold up to 64 bit on 64 bit capable CPUs. Reflect this in the SPR
>> setting code, so that the guest can never write invalid values in them.
>> 
>> Signed-off-by: Alexander Graf 
>> ---
>> target-ppc/translate_init.c |   19 ++-
>> 1 files changed, 18 insertions(+), 1 deletions(-)
>> 
>> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
>> index 8ff47ae..e6580ff 100644
>> --- a/target-ppc/translate_init.c
>> +++ b/target-ppc/translate_init.c
>> @@ -86,6 +86,19 @@ static void spr_write_generic (void *opaque, int sprn, 
>> int gprn)
>> }
>> 
>> #if !defined(CONFIG_USER_ONLY)
>> +static void spr_write_generic32(void *opaque, int sprn, int gprn)
>> +{
>> +#ifdef TARGET_PPC64
>> +TCGv t0 = tcg_temp_new();
>> +tcg_gen_ext32u_tl(t0, cpu_gpr[gprn]);
>> +gen_store_spr(sprn, t0);
>> +tcg_temp_free(t0);
>> +spr_store_dump_spr(sprn);
>> +#else
>> +spr_write_generic(opaque, sprn, gprn);
>> +#endif
>> +}
>> +
>> static void spr_write_clear (void *opaque, int sprn, int gprn)
>> {
>> TCGv t0 = tcg_temp_new();
>> @@ -1597,10 +1610,14 @@ static void gen_spr_BookE206(CPUPPCState *env, 
>> uint32_t mas_mask,
>> /* TLB assist registers */
>> /* XXX : not implemented */
>> for (i = 0; i < 8; i++) {
>> +void (*uea_write)(void *o, int sprn, int gprn) = 
>> &spr_write_generic32;
>> +if (i == 2 && (mas_mask & (1 << i)) && (env->insns_flags & 
>> PPC_64B)) {
>> +uea_write = &spr_write_generic;
>> +}
>> if (mas_mask & (1 << i)) {
>> spr_register(env, mas_sprn[i], mas_names[i],
>>  SPR_NOACCESS, SPR_NOACCESS,
>> - &spr_read_generic, &spr_write_generic,
>> + &spr_read_generic, uea_write,
>>  0x);
>> }
> 
> What does "uea" mean?

Not sure - it's the same definition as what spr_register takes in as parameter.


Alex

Re: [Qemu-devel] [PATCHv3 07/14] unicore32-softmmu: Add puv3 soc/board support

[Blue Swirl]

On Wed, Jun 20, 2012 at 1:56 AM, Guan Xuetao  wrote:
> On Mon, 2012-06-18 at 20:02 +, Blue Swirl wrote:
> [snip]
>> > diff --git a/hw/puv3.h b/hw/puv3.h
>> > new file mode 100644
>> > index 000..bcfc978
>> > --- /dev/null
>> > +++ b/hw/puv3.h
>> > @@ -0,0 +1,49 @@
>> > +/*
>> > + * Misc PKUnity SoC declarations
>> > + *
>> > + * Copyright (C) 2010-2012 Guan Xuetao
>> > + *
>> > + * This program is free software; you can redistribute it and/or modify
>> > + * it under the terms of the GNU General Public License version 2 as
>> > + * published by the Free Software Foundation, or any later version.
>> > + * See the COPYING file in the top-level directory.
>> > + */
>> > +#ifndef __PUV3_H__
>>
>> Use of leading underscores is reserved to Posix, please use for
>> example HW_PUV3_H.
> Ok, I will change it.
> Perhaps I'm wrong, but IMHO, leading underscores are used pervasively
> for headers protection and low-level definitions.

It's unfortunately common, but as mentioned in HACKING file, those are
reserved to C and POSIX use. For example Linux kernel can disregard
this, because it doesn't use libc and GCC does not exercise the right
to use the reserved prefixes. I guess some people copy this style to
applications where it's no longer correct.

>
> Guan Xuetao
>
>
>

Re: [Qemu-devel] [PATCHv3 12/14] unicore32-softmmu: Add puv3 dma support

[Blue Swirl]

On Wed, Jun 20, 2012 at 2:10 AM, Guan Xuetao  wrote:
> On Mon, 2012-06-18 at 19:59 +, Blue Swirl wrote:
> [snip]
>> > +
>> > +#define PUV3_DMA_CH_NR          (6)
>> > +#define PUV3_DMA_CH_MASK        (0xff)
>> > +#define PUV3_DMA_CH(offset)     ((offset) >> 8)
>> > +
>> > +typedef struct {
>> > +    SysBusDevice busdev;
>> > +    MemoryRegion iomem;
>> > +    uint32_t reg_CFG[PUV3_DMA_CH_NR];
>> > +} PUV3DMAState;
>> > +
>> > +static uint64_t puv3_dma_read(void *opaque, target_phys_addr_t offset,
>> > +        unsigned size)
>> > +{
>> > +    PUV3DMAState *s = (PUV3DMAState *) opaque;
>>
>> These casts from void pointer are not needed in C.
> I see. Thanks.
>
>> > +    uint32_t ret;
>> > +
>> > +    assert(PUV3_DMA_CH(offset) < PUV3_DMA_CH_NR);
>> > +
>> > +    switch (offset & PUV3_DMA_CH_MASK) {
>> > +    case 0x10:
>> > +        ret = s->reg_CFG[PUV3_DMA_CH(offset)];
>> > +        break;
>> > +    default:
>> > +        hw_error("%s: Bad offset 0x%x\n", __func__, offset);
>>
>> hw_error() also aborts, it would be nice to avoid that. However, the
>> situation is somewhat different from the instruction case, since only
>> privileged guest code (kernel) can write to hardware. The kernel can
>> also for example power off the machine.
>
> It's the same problem as cpu_abort. Warning information is enough here.
> Is there a global and simple way to do it, g_warning()?

The users will probably not be very interested about any debugging
messages. In cases like this, two common ways are to use trace points
or debugging printf macros.

>
> Guan Xuetao
>

Re: [Qemu-devel] [PATCH v5 00/16] QEMU OpenRISC support

[Blue Swirl]

On Wed, Jun 20, 2012 at 7:10 AM, Jia Liu  wrote:
> Hi Blue,
>
> On Tue, Jun 19, 2012 at 3:11 AM, Blue Swirl  wrote:
>> On Mon, Jun 18, 2012 at 1:02 AM, Jia Liu  wrote:
>>> This is the OpenCores OpenRISC 1200 support for QEMU.
>>> Full implementation of the system-model and linux-user-model support.
>>>
>>> OpenRISC 1200 is a OpenCores open source CPU,
>>> its architecture manual can be found at
>>> http://opencores.org/svnget,or1k?file=/trunk/docs/openrisc_arch.pdf
>>>
>>> A OpenRISC Linux kernel contain initramfs for qemu-system-or32 testing
>>> can be found at
>>> https://docs.google.com/file/d/0BxeTrz3x0CBLSjR3Sk5Vd3h1eDA/edit?pli=1
>>>
>>> A OpenRISC hello-world program for qemu-or32 testing can be found at
>>> https://docs.google.com/file/d/0BxeTrz3x0CBLN3RSWUFNYktrU2M/edit?pli=1
>>>
>>> Signed-off-by: Jia Liu 
>>
>> I had minor comments to a few patches, others looked ready. Please
>> still check the patches with scripts/checkpatch.pl.
>>
>
> I've checked the patches with scripts/checkpatch.pl.
>
> when I run "./scripts/checkpatch.pl *.patch", it is all OK,
>
> but when I run "./scripts/checkpatch.pl 0012-xxx.patch", 0006 and 0012
> have ERROR like this:
>
> ERROR: need consistent spacing around '*' (ctx:WxV)
> #99: FILE: target-openrisc/int_helper.c:53:
> +target_ulong HELPER(mul32)(CPUOpenRISCState *env,
>
> It is really weird. And, I don't know how to handle it.

It's a limitation in checkpatch.pl, it is confused by any CPUxxxState
for some reason. Please just ignore these cases.

>
>
>>> ---
>>>
>>> Version History:
>>> V5:
>>> Addressed Blue's review comments:
>>> - reimplement l.mul* l.mfspr l.add* l.sub* and more.
>>> - shoot bugs with "--enable-debug-tcg"
>>>
>>> V4:
>>> Addressed Max's review comments:
>>> - fix l.div l.mac* l.mul*, and more.
>>>
>>> Addressed Richard, Wei-Ren and Andreas's review comments:
>>> - replace tcg_temp_new_i32 with tcg_temp_local_new_i32 in l.div translation.
>>>
>>> Addressed Andreas's review comments:
>>> - update to suit Makefile system.
>>>
>>> - add UPR CPUCFGR and MMUCFGR impelement.
>>> - add instruction check functions.
>>>
>>> Version History:
>>> V3:
>>> Addressed Stefan and Andreas's review comments:
>>> - use QEMU and OpenRISC's official name.
>>>
>>> Addressed Andreas's review comments:
>>> - reimplement cpu QOM.
>>> - combine target stubs and QOM implement.
>>> - use new commit message and subject.
>>>
>>> Addressed Max's review comments:
>>> - handle div zero exception.
>>> - reimplement float point instructions.
>>> - fix l.mac*, l.mul*, and more.
>>>
>>> V2:
>>> Addressed Malc, Weiren, Andreas and Blue's review comments:
>>> - reimplement cpu QOM.
>>>
>>> Addressed Andreas's review comments:
>>> - reimplement machine.
>>> - rewrite the Copyright Notice using better format.
>>>
>>> Addressed Blue and Weiren's review comments:
>>> - compiling with AREG0 and remove global env, no dyngen-exe longer.
>>>
>>> Addressed Max, Blue and Weiren's review comments:
>>> - handle div zero exception.
>>> - handle illegal instruction.
>>>
>>> Addressed Blue's review comments:
>>> - separate do_interrupt into intrpt.c form intrpt_helper.c.
>>> - add QEMU_NORETURN to raise_exception.
>>> - reimplement float instrutions.
>>> - fix type of linux syscall and termbits.
>>> - reimplement sim board.
>>> - use the LGPL web URL in Copyright Notice.
>>> - reimplemt branch instructions.
>>>
>>> - split taregt stubs, QOM and machine.
>>>
>>> V1:
>>> - add QEMU OpenRISC support.
>>> - well tested on x64 machine, and final tested x86 machine.
>>>
>>> Jia Liu (16):
>>>  target-or32: Add target stubs and cpu support
>>>  target-or32: Add target machine
>>>  target-or32: Add MMU support
>>>  target-or32: Add interrupt support
>>>  target-or32: Add exception support
>>>  target-or32: Add int instruction helpers
>>>  target-or32: Add float instruction helpers
>>>  target-or32: Add translation routines
>>>  target-or32: Add PIC support
>>>  target-or32: Add timer support
>>>  target-or32: Add a IIS dummy board
>>>  target-or32: Add system instructions
>>>  target-or32: Add gdb stub support
>>>  target-or32: Add linux syscall, signal and termbits
>>>  target-or32: Add linux user support
>>>  target-or32: Add testcases
>>>
>>>  arch_init.c                         |    2 +
>>>  arch_init.h                         |    1 +
>>>  configure                           |   15 +-
>>>  cpu-exec.c                          |   19 +
>>>  default-configs/or32-linux-user.mak |    2 +
>>>  default-configs/or32-softmmu.mak    |    6 +
>>>  elf.h                               |    2 +
>>>  gdbstub.c                           |   64 ++
>>>  hw/openrisc/Makefile.objs           |    3 +
>>>  hw/openrisc_cpudev.h                |   29 +
>>>  hw/openrisc_pic.c                   |   78 ++
>>>  hw/openrisc_sim.c                   |  145 +++
>>>  hw/openrisc_timer.c                 |  160 +++
>>>  linux-user/elfload.c                |   41 +
>>>  linux-user/main.c                   |  100 ++
>>>  linux-user/open

Re: [Qemu-devel] [PATCH v5 00/16] QEMU OpenRISC support

[Peter Maydell]

On 21 June 2012 18:24, Blue Swirl  wrote:
> On Wed, Jun 20, 2012 at 7:10 AM, Jia Liu  wrote:
>> ERROR: need consistent spacing around '*' (ctx:WxV)
>> #99: FILE: target-openrisc/int_helper.c:53:
>> +target_ulong HELPER(mul32)(CPUOpenRISCState *env,
>>
>> It is really weird. And, I don't know how to handle it.
>
> It's a limitation in checkpatch.pl, it is confused by any CPUxxxState
> for some reason. Please just ignore these cases.

I think in this case it's more likely to be the HELPER() macro
that is confusing it, but yes, just ignore the error.

-- PMM

Re: [Qemu-devel] [PATCH v5 01/16] target-or32: Add target stubs and cpu support

[Blue Swirl]

On Wed, Jun 20, 2012 at 7:14 AM, Jia Liu  wrote:
> Hi Blue,
>
> On Tue, Jun 19, 2012 at 2:28 AM, Blue Swirl  wrote:
>> On Mon, Jun 18, 2012 at 1:02 AM, Jia Liu  wrote:
>>> Add OpenRISC target stubs and basic cpu support.
>>>
>>> Signed-off-by: Jia Liu 
>>> ---
>>>  arch_init.c                      |    2 +
>>>  arch_init.h                      |    1 +
>>>  configure                        |   14 +-
>>>  cpu-exec.c                       |    2 +
>>>  default-configs/or32-softmmu.mak |    6 +
>>>  elf.h                            |    2 +
>>>  hw/openrisc/Makefile.objs        |    3 +
>>>  hw/openrisc_cpudev.h             |   29 
>>>  hw/openrisc_pic.c                |   30 
>>>  hw/openrisc_timer.c              |   30 
>>>  poison.h                         |    1 +
>>>  target-openrisc/Makefile.objs    |    3 +
>>>  target-openrisc/cpu.c            |  240 +++
>>>  target-openrisc/cpu.h            |  288 
>>> ++
>>>  target-openrisc/intrpt.c         |   30 
>>>  target-openrisc/machine.c        |   30 
>>>  target-openrisc/mmu.c            |   39 ++
>>>  target-openrisc/mmu_helper.c     |   43 ++
>>>  target-openrisc/translate.c      |   75 ++
>>>  19 files changed, 866 insertions(+), 2 deletions(-)
>>>  create mode 100644 default-configs/or32-softmmu.mak
>>>  create mode 100644 hw/openrisc/Makefile.objs
>>>  create mode 100644 hw/openrisc_cpudev.h
>>>  create mode 100644 hw/openrisc_pic.c
>>>  create mode 100644 hw/openrisc_timer.c
>>>  create mode 100644 target-openrisc/Makefile.objs
>>>  create mode 100644 target-openrisc/cpu.c
>>>  create mode 100644 target-openrisc/cpu.h
>>>  create mode 100644 target-openrisc/intrpt.c
>>>  create mode 100644 target-openrisc/machine.c
>>>  create mode 100644 target-openrisc/mmu.c
>>>  create mode 100644 target-openrisc/mmu_helper.c
>>>  create mode 100644 target-openrisc/translate.c
>>>
>>> diff --git a/arch_init.c b/arch_init.c
>>> index a9e8b74..4b521e5 100644
>>> --- a/arch_init.c
>>> +++ b/arch_init.c
>>> @@ -71,6 +71,8 @@ int graphic_depth = 15;
>>>  #define QEMU_ARCH QEMU_ARCH_MICROBLAZE
>>>  #elif defined(TARGET_MIPS)
>>>  #define QEMU_ARCH QEMU_ARCH_MIPS
>>> +#elif defined(TARGET_OPENRISC)
>>> +#define QEMU_ARCH QEMU_ARCH_OPENRISC
>>>  #elif defined(TARGET_PPC)
>>>  #define QEMU_ARCH QEMU_ARCH_PPC
>>>  #elif defined(TARGET_S390X)
>>> diff --git a/arch_init.h b/arch_init.h
>>> index c7cb94a..3dfea3b 100644
>>> --- a/arch_init.h
>>> +++ b/arch_init.h
>>> @@ -16,6 +16,7 @@ enum {
>>>     QEMU_ARCH_SH4 = 1024,
>>>     QEMU_ARCH_SPARC = 2048,
>>>     QEMU_ARCH_XTENSA = 4096,
>>> +    QEMU_ARCH_OPENRISC = 8192,
>>>  };
>>>
>>>  extern const uint32_t arch_type;
>>> diff --git a/configure b/configure
>>> index c2366ee..3ba7c91 100755
>>> --- a/configure
>>> +++ b/configure
>>> @@ -925,6 +925,7 @@ mips-softmmu \
>>>  mipsel-softmmu \
>>>  mips64-softmmu \
>>>  mips64el-softmmu \
>>> +or32-softmmu \
>>>  ppc-softmmu \
>>>  ppcemb-softmmu \
>>>  ppc64-softmmu \
>>> @@ -3482,7 +3483,7 @@ target_arch2=`echo $target | cut -d '-' -f 1`
>>>  target_bigendian="no"
>>>
>>>  case "$target_arch2" in
>>> -  
>>> armeb|lm32|m68k|microblaze|mips|mipsn32|mips64|ppc|ppcemb|ppc64|ppc64abi32|s390x|sh4eb|sparc|sparc64|sparc32plus|xtensaeb)
>>> +  
>>> armeb|lm32|m68k|microblaze|mips|mipsn32|mips64|or32|ppc|ppcemb|ppc64|ppc64abi32|s390x|sh4eb|sparc|sparc64|sparc32plus|xtensaeb)
>>>   target_bigendian=yes
>>>   ;;
>>>  esac
>>> @@ -3598,6 +3599,11 @@ case "$target_arch2" in
>>>     target_phys_bits=64
>>>     target_long_alignment=8
>>>   ;;
>>> +  or32)
>>> +    TARGET_ARCH=openrisc
>>> +    TARGET_BASE_ARCH=openrisc
>>> +    target_phys_bits=32
>>> +  ;;
>>>   ppc)
>>>     gdb_xml_files="power-core.xml power-fpu.xml power-altivec.xml 
>>> power-spe.xml"
>>>     target_phys_bits=64
>>> @@ -3676,7 +3682,7 @@ symlink "$source_path/Makefile.target" 
>>> "$target_dir/Makefile"
>>>
>>>
>>>  case "$target_arch2" in
>>> -  alpha | sparc* | xtensa*)
>>> +  alpha | or32 | sparc* | xtensa*)
>>>     echo "CONFIG_TCG_PASS_AREG0=y" >> $config_target_mak
>>>   ;;
>>>  esac
>>> @@ -3847,6 +3853,10 @@ for i in $ARCH $TARGET_BASE_ARCH ; do
>>>     echo "CONFIG_MIPS_DIS=y"  >> $config_target_mak
>>>     echo "CONFIG_MIPS_DIS=y"  >> $libdis_config_mak
>>>   ;;
>>> +  or32)
>>> +    echo "CONFIG_OPENRISC_DIS=y"  >> $config_target_mak
>>> +    echo "CONFIG_OPENRISC_DIS=y"  >> $libdis_config_mak
>>> +  ;;
>>>   ppc*)
>>>     echo "CONFIG_PPC_DIS=y"  >> $config_target_mak
>>>     echo "CONFIG_PPC_DIS=y"  >> $libdis_config_mak
>>> diff --git a/cpu-exec.c b/cpu-exec.c
>>> index 624c409..7d0d87b 100644
>>> --- a/cpu-exec.c
>>> +++ b/cpu-exec.c
>>> @@ -225,6 +225,7 @@ int cpu_exec(CPUArchState *env)
>>>  #elif defined(TARGET_LM32)
>>>  #elif defined(TARGET_MICROBLAZE)
>>>  #elif defined(TARGET_MIPS)
>>> +#elif defined(TARGET_OPENRISC)
>>>  #elif defined(TARGET_SH4)
>>>  #elif defined(TARGET_CRIS)
>>>  #elif defined(TA

Re: [Qemu-devel] [PATCHv3 02/14] unicore32-softmmu: Add coprocessor 0(sysctrl) and 1(ocd) instruction support

[Blue Swirl]

On Wed, Jun 20, 2012 at 1:40 AM, Guan Xuetao  wrote:
> On Mon, 2012-06-18 at 19:51 +, Blue Swirl wrote:
>> On Mon, Jun 18, 2012 at 9:24 AM, Guan Xuetao  wrote:
>> > Coprocessor 0 is system control coprocessor, and we need get/set its 
>> > contents.
>> > Also, all cache/tlb ops shoule be implemented here, but just ignored with 
>> > no harm.
>> >
>> > Coprocessor 1 is OCD (on-chip-debugger), which is used for faked console,
>> > so we could output chars to this console without graphic card.
>> >
>> > Signed-off-by: Guan Xuetao 
>> > ---
>> >  target-unicore32/helper.c    |  177 
>> > +-
>> >  target-unicore32/helper.h    |   17 ++---
>> >  target-unicore32/translate.c |   75 ++-
>> >  3 files changed, 221 insertions(+), 48 deletions(-)
>> >
>> > diff --git a/target-unicore32/helper.c b/target-unicore32/helper.c
>> > index 9b8ff06..42a39e5 100644
>> > --- a/target-unicore32/helper.c
>> > +++ b/target-unicore32/helper.c
>> > @@ -14,6 +14,14 @@
>> >  #include "helper.h"
>> >  #include "host-utils.h"
>> >
>> > +#undef DEBUG_UC32
>> > +
>> > +#ifdef DEBUG_UC32
>> > +#define DPRINTF(fmt, ...) printf("%s: " fmt , __func__, ## __VA_ARGS__)
>> > +#else
>> > +#define DPRINTF(fmt, ...) do {} while (0)
>> > +#endif
>> > +
>> >  CPUUniCore32State *uc32_cpu_init(const char *cpu_model)
>> >  {
>> >     UniCore32CPU *cpu;
>> > @@ -45,6 +53,138 @@ uint32_t HELPER(clz)(uint32_t x)
>> >     return clz32(x);
>> >  }
>> >
>> > +#ifndef CONFIG_USER_ONLY
>> > +void helper_cp0_set(CPUUniCore32State *env, uint32_t val, uint32_t creg,
>> > +        uint32_t cop)
>> > +{
>> > +    /*
>> > +     * movc pp.nn, rn, #imm9
>> > +     *      rn: UCOP_REG_D
>> > +     *      nn: UCOP_REG_N
>> > +     *          1: sys control reg.
>> > +     *          2: page table base reg.
>> > +     *          3: data fault status reg.
>> > +     *          4: insn fault status reg.
>> > +     *          5: cache op. reg.
>> > +     *          6: tlb op. reg.
>> > +     *      imm9: split UCOP_IMM10 with bit5 is 0
>> > +     */
>> > +    switch (creg) {
>> > +    case 1:
>> > +        if (cop != 0) goto unrecognized;
>>
>> Does this pass scripts/checkpatch.pl? These should become
>> if (cop != 0) {
>>     goto unrecognized;
>> }
> Thanks for pointing it out, and sorry for that.
> I will correct it in next version.
>
>>
>> > +        env->cp0.c1_sys = val;
>> > +        break;
>> > +    case 2:
>> > +        if (cop != 0) goto unrecognized;
>> > +        env->cp0.c2_base = val;
>> > +        break;
>> > +    case 3:
>> > +        if (cop != 0) goto unrecognized;
>> > +        env->cp0.c3_faultstatus = val;
>> > +        break;
>> > +    case 4:
>> > +        if (cop != 0) goto unrecognized;
>> > +        env->cp0.c4_faultaddr = val;
>> > +        break;
>> > +    case 5:
>> > +        switch(cop) {
>> > +        case 28:
>> > +            DPRINTF("Invalidate Entire I&D cache\n");
>> > +            return;
>> > +        case 20:
>> > +            DPRINTF("Invalidate Entire Icache\n");
>> > +            return;
>> > +        case 12:
>> > +            DPRINTF("Invalidate Entire Dcache\n");
>> > +            return;
>> > +        case 10:
>> > +            DPRINTF("Clean Entire Dcache\n");
>> > +            return;
>> > +        case 14:
>> > +            DPRINTF("Flush Entire Dcache\n");
>> > +            return;
>> > +        case 13:
>> > +            DPRINTF("Invalidate Dcache line\n");
>> > +            return;
>> > +        case 11:
>> > +            DPRINTF("Clean Dcache line\n");
>> > +            return;
>> > +        case 15:
>> > +            DPRINTF("Flush Dcache line\n");
>> > +            return;
>> > +        }
>> > +        break;
>> > +    case 6:
>> > +        if ((cop <= 6) && (cop >=2)) {
>> > +            /* invalid all tlb */
>> > +            tlb_flush(env, 1);
>> > +            return;
>> > +        }
>> > +        break;
>> > +    default:
>> > +        goto unrecognized;
>> > +    }
>> > +    return;
>> > +unrecognized:
>> > +    cpu_abort(env, "Wrong register (%d) or wrong operation (%d) in 
>> > cp0_set!\n",
>> > +            creg, cop);
>>
>> The call to cpu_abort() would mean that the guest is able to terminate
>> QEMU at will, which is not OK. What does real HW do?
> In my opinion, I just want to terminate qemu when any unhandled or
> unknown operations happen.

This can make the emulator vulnerable in the security sense. Probably
Unicore CPUs are not used now in an environment where the guest can
not be trusted (like cloud computing), but who knows the future?

>
>>
>> > +}
>> > +
>> > +uint32_t helper_cp0_get(CPUUniCore32State *env, uint32_t creg, uint32_t 
>> > cop)
>> > +{
>> > +    /*
>> > +     * movc rd, pp.nn, #imm9
>> > +     *      rd: UCOP_REG_D
>> > +     *      nn: UCOP_REG_N
>> > +     *          0: cpuid and cachetype
>> > +     *          1: sys control reg.
>> > +     *          2: page table base reg.
>> > +     *          3: data fault status reg.

Re: [Qemu-devel] [PATCH 2/2] fdc: Move floppy geometry guessing back from block.c

[Blue Swirl]

On Wed, Jun 20, 2012 at 8:21 AM, Markus Armbruster  wrote:
> Blue Swirl  writes:
>
>> On Tue, Jun 19, 2012 at 7:45 AM, Markus Armbruster  wrote:
>>> Blue Swirl  writes:
>>>
 On Mon, Jun 18, 2012 at 9:10 AM, Markus Armbruster  
 wrote:
> Commit 5bbdbb46 moved it to block.c because "other geometry guessing
> functions already reside in block.c".  Device-specific functionality
> should be kept in device code, not the block layer.  Move it back.

 As discussed earlier, this is media specific, not device specific
 (except FDriveType). How about media.c?
>>>
>>> It's floppy-(media-)specific, isn't it?
>>>
>>> We discussed separating floppy drive emulation (fdd) from floppy
>>> controller emulation.  Right now, they're mixed up in qdevs isa-fdc,
>>> sysbus-fdc and SUNW,fdtwo.  Separating fdd involves splitting up those
>>> qdevs.  I tried, but ran into QOM infrastructure difficulties.  Since
>>> that part of QOM is being improved, I decided to postpone the splitting
>>> work for a bit.
>>>
>>> I don't remember discussing a separation of floppy drive and floppy
>>> media emulation.
>>
>> OK, maybe I mixed things up. How I see this is that a floppy drive may
>> support several different media types, like 720k, 1.44M and 2.88M
>> floppies, while floppy media may still be formatted differently with
>> various number of sectors.
>
> Yes, there are several types of floppy drives, each capable of dealing
> with a certain set of media geometries.
>
>>                            The media part is similar to CHS for hard
>> disks, while the drive or type parts are not visible outside of the HD
>> unit.
>
> Hard disk geometry is a property of the device, like floppy drive type,
> but unlike floppy geometry.

I guess the hard disk situation is internally not unlike to floppies
at all, the manufacturer probably uses the same mechanisms and
electronics for several drive models, only the platter capacity (with
the tracks, heads and sectors like floppies) and related configuration
changes. But for the end user, this is not visible and even the CHS
can be fake if the drive can for example remap bad sectors or use some
platter areas for internal use. Of course users are not able to
exchange platters at will.

>
>>> Related project: moving hard disk geometry out of the block layer.
>>> Can't move into a device model, because we have three of them sporting
>>> geometry: IDE, SCSI and virtio disks.  I guess I'll move them into a new
>>> file in hw/.  media.c doesn't sound right for hard disks.  disk-geo.c?
>>
>> hd-geometry.c?
>
> Sold.
>
>>> I could move floppy geometry to the same file.  But there's zero overlap
>>> between hard disk and floppy disk geometry, and the only consumer of
>>> floppy geometry is the floppy disk device.  I don't expect that to
>>> change, and that's why I'd prefer to make floppy geometry an
>>> implementation detail of the floppy disk device, and hide it in fdc.c
>>> now, fdd.c after the split.
>>>
>>> But if you insist, I can unhide it.
>>
>> How about fd-geometry.c and hd-geometry.c? Or chs-translation.c to
>> combine both, maybe also other transformations of CHS to linear offset
>> later?
>
> Since there's no overlap between hard and floppy disk geometry, I'd
> rather not mix them up in the same file.
>
> Can do fd-geometry.c.  While I can't see what putting floppy geometry in
> fd-geometry.c rather than next to its only user buys us (apart from
> external linkage), I'll comply with your wish.

On second thought, fd-geometry.c would not be used for anything else
(until we have Jaz? Zip?), so merging it with fdc.c makes sense. I'm
not sure the same also applies to hd-geometry.c.

>
>>> Comments?

[Qemu-devel] qemu -numa option and non-contiguous CPU ranges

[Eduardo Habkost]

Hi,

I just noticed libvirt tries to use the -numa option in a way that qemu
never understood: if a node is configured to have a non-contiguous set
of CPUs, it tries to generate a command-line option that looks like:

"-numa node,nodeid=...,cpus=0,2,4,mem=..."
^

But this format was never supported by qemu. This format is even a bit
weird, as "," is an option separator, and it is being used as a
separator _inside_ an option.

My question is: should we support this option format in qemu, or should
we change libvirt to use another format (that has yet to be implemented,
because currently there's no way to specify a non-contiguous set of CPUs
for a NUMA node).

Any suggestions?

-- 
Eduardo

Re: [Qemu-devel] [PATCH 7/8] PPC: Turn hardcoded reset mask into env variable

[Blue Swirl]

On Wed, Jun 20, 2012 at 8:11 PM, Alexander Graf  wrote:
> Some machines have MSR bits they reset with as enabled. Don't hardcode the
> logic, but let the individual core implementations save their own reset
> mask into an env variable.
>
> Signed-off-by: Alexander Graf 
> ---
>  target-ppc/cpu.h            |    1 +
>  target-ppc/translate_init.c |   14 --
>  2 files changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
> index 652a35a..acf5816 100644
> --- a/target-ppc/cpu.h
> +++ b/target-ppc/cpu.h
> @@ -1043,6 +1043,7 @@ struct CPUPPCState {
>  #if defined(TARGET_PPC64)
>     struct ppc_segment_page_sizes sps;
>  #endif
> +    uint64_t reset_msr;
>
>  #if defined(TARGET_PPC64) && !defined(CONFIG_USER_ONLY)
>     target_phys_addr_t vpa;
> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
> index 57027a2..efa05fc 100644
> --- a/target-ppc/translate_init.c
> +++ b/target-ppc/translate_init.c
> @@ -6273,6 +6273,7 @@ static void init_proc_970 (CPUPPCState *env)
>     env->slb_nr = 32;
>  #endif
>     init_excp_970(env);
> +    env->reset_msr = (1ULL < MSR_SF);

The parenthesis are not necessary.

>     env->dcache_line_size = 128;
>     env->icache_line_size = 128;
>     /* Allocate hardware IRQ controller */
> @@ -6375,6 +6376,7 @@ static void init_proc_970FX (CPUPPCState *env)
>     env->slb_nr = 64;
>  #endif
>     init_excp_970(env);
> +    env->reset_msr = (1ULL < MSR_SF);
>     env->dcache_line_size = 128;
>     env->icache_line_size = 128;
>     /* Allocate hardware IRQ controller */
> @@ -6465,6 +6467,7 @@ static void init_proc_970GX (CPUPPCState *env)
>     env->slb_nr = 32;
>  #endif
>     init_excp_970(env);
> +    env->reset_msr = (1ULL < MSR_SF);
>     env->dcache_line_size = 128;
>     env->icache_line_size = 128;
>     /* Allocate hardware IRQ controller */
> @@ -6555,6 +6558,7 @@ static void init_proc_970MP (CPUPPCState *env)
>     env->slb_nr = 32;
>  #endif
>     init_excp_970(env);
> +    env->reset_msr = (1ULL < MSR_SF);
>     env->dcache_line_size = 128;
>     env->icache_line_size = 128;
>     /* Allocate hardware IRQ controller */
> @@ -6640,6 +6644,7 @@ static void init_proc_POWER7 (CPUPPCState *env)
>     env->slb_nr = 32;
>  #endif
>     init_excp_POWER7(env);
> +    env->reset_msr = (1ULL < MSR_SF);
>     env->dcache_line_size = 128;
>     env->icache_line_size = 128;
>     /* Allocate hardware IRQ controller */
> @@ -6686,6 +6691,7 @@ static void init_proc_620 (CPUPPCState *env)
>     /* Memory management */
>     gen_low_BATs(env);
>     init_excp_620(env);
> +    env->reset_msr = (1ULL < MSR_SF);
>     env->dcache_line_size = 64;
>     env->icache_line_size = 64;
>     /* Allocate hardware IRQ controller */
> @@ -9306,6 +9312,7 @@ static void init_ppc_proc (CPUPPCState *env, const 
> ppc_def_t *def)
>     env->nb_BATs = 0;
>     env->nb_tlb = 0;
>     env->nb_ways = 0;
> +    env->reset_msr = 0;
>     env->tlb_type = TLB_NONE;
>  #endif
>     /* Register SPR common to all PowerPC implementations */
> @@ -10246,7 +10253,7 @@ static void ppc_cpu_reset(CPUState *s)
>
>     pcc->parent_reset(s);
>
> -    msr = (target_ulong)0;
> +    msr = (target_ulong)env->reset_msr;
>     if (0) {
>         /* XXX: find a suitable condition to enable the hypervisor mode */
>         msr |= (target_ulong)MSR_HVB;
> @@ -10272,11 +10279,6 @@ static void ppc_cpu_reset(CPUState *s)
>     }
>  #endif
>     env->msr = msr & env->msr_mask;
> -#if defined(TARGET_PPC64)
> -    if (env->mmu_model & POWERPC_MMU_64) {
> -        env->msr |= (1ULL << MSR_SF);
> -    }
> -#endif
>     hreg_compute_hflags(env);
>     env->reserve_addr = (target_ulong)-1ULL;
>     /* Be sure no exception or interrupt is pending */
> --
> 1.6.0.2
>
>

Re: [Qemu-devel] [PATCH 7/8] PPC: Turn hardcoded reset mask into env variable

[Peter Maydell]

On 20 June 2012 21:11, Alexander Graf  wrote:
> +    env->reset_msr = (1ULL < MSR_SF);

I assume you mean "<<" rather than "<" here and below...

-- PMM

Re: [Qemu-devel] [PATCH 2/2] fdc: Move floppy geometry guessing back from block.c

[Markus Armbruster]

Blue Swirl  writes:

> On Wed, Jun 20, 2012 at 8:21 AM, Markus Armbruster  wrote:
>> Blue Swirl  writes:
>>
>>> On Tue, Jun 19, 2012 at 7:45 AM, Markus Armbruster  
>>> wrote:
 Blue Swirl  writes:

> On Mon, Jun 18, 2012 at 9:10 AM, Markus Armbruster  
> wrote:
>> Commit 5bbdbb46 moved it to block.c because "other geometry guessing
>> functions already reside in block.c".  Device-specific functionality
>> should be kept in device code, not the block layer.  Move it back.
>
> As discussed earlier, this is media specific, not device specific
> (except FDriveType). How about media.c?

 It's floppy-(media-)specific, isn't it?

 We discussed separating floppy drive emulation (fdd) from floppy
 controller emulation.  Right now, they're mixed up in qdevs isa-fdc,
 sysbus-fdc and SUNW,fdtwo.  Separating fdd involves splitting up those
 qdevs.  I tried, but ran into QOM infrastructure difficulties.  Since
 that part of QOM is being improved, I decided to postpone the splitting
 work for a bit.

 I don't remember discussing a separation of floppy drive and floppy
 media emulation.
>>>
>>> OK, maybe I mixed things up. How I see this is that a floppy drive may
>>> support several different media types, like 720k, 1.44M and 2.88M
>>> floppies, while floppy media may still be formatted differently with
>>> various number of sectors.
>>
>> Yes, there are several types of floppy drives, each capable of dealing
>> with a certain set of media geometries.
>>
>>>                            The media part is similar to CHS for hard
>>> disks, while the drive or type parts are not visible outside of the HD
>>> unit.
>>
>> Hard disk geometry is a property of the device, like floppy drive type,
>> but unlike floppy geometry.
>
> I guess the hard disk situation is internally not unlike to floppies
> at all, the manufacturer probably uses the same mechanisms and
> electronics for several drive models, only the platter capacity (with
> the tracks, heads and sectors like floppies) and related configuration
> changes. But for the end user, this is not visible and even the CHS
> can be fake if the drive can for example remap bad sectors or use some
> platter areas for internal use. Of course users are not able to
> exchange platters at will.

Makes sense, but we can ignore such hard disk internals in QEMU.

 Related project: moving hard disk geometry out of the block layer.
 Can't move into a device model, because we have three of them sporting
 geometry: IDE, SCSI and virtio disks.  I guess I'll move them into a new
 file in hw/.  media.c doesn't sound right for hard disks.  disk-geo.c?
>>>
>>> hd-geometry.c?
>>
>> Sold.
>>
 I could move floppy geometry to the same file.  But there's zero overlap
 between hard disk and floppy disk geometry, and the only consumer of
 floppy geometry is the floppy disk device.  I don't expect that to
 change, and that's why I'd prefer to make floppy geometry an
 implementation detail of the floppy disk device, and hide it in fdc.c
 now, fdd.c after the split.

 But if you insist, I can unhide it.
>>>
>>> How about fd-geometry.c and hd-geometry.c? Or chs-translation.c to
>>> combine both, maybe also other transformations of CHS to linear offset
>>> later?
>>
>> Since there's no overlap between hard and floppy disk geometry, I'd
>> rather not mix them up in the same file.
>>
>> Can do fd-geometry.c.  While I can't see what putting floppy geometry in
>> fd-geometry.c rather than next to its only user buys us (apart from
>> external linkage), I'll comply with your wish.
>
> On second thought, fd-geometry.c would not be used for anything else
> (until we have Jaz? Zip?), so merging it with fdc.c makes sense. I'm
> not sure the same also applies to hd-geometry.c.

We agree, excellent.  I'll put the floppy geometry code next to its only
user, with the understanding that we'll move it to its own file as soon
as a second user shows up.  The hard disk geometry code goes into
hw/hd-geometry.c.

[Qemu-devel] [RFC] QEMU mailing list changes

[Stefan Weil]

Hi,

I suggest a small change of the QEMU related mailing lists
(see https://lists.nongnu.org/mailman/listinfo/):

Replace Qemu-devel by QEMU-devel, Qemu-trivial by QEMU-trivial,
and so on.

This matches the official project name which is QEMU (not Qemu).
As far as I know, there is no convention which prefers Qemu-devel,
and there are mailing lists on lists.nongnu.org with any kind
of upper and lower case mixed.

Mailing addresses are case insensitive and the links on the server
are lower case, therefore I expect that the change of the name
on lists.nongnu.org will work without much trouble.

Maybe some servers which mirror mailing lists could get confused
(if they inspect the name of the lists).

And of course the subjects of new mails would start with [QEMU-devel],
so case sensitive filters in private mail readers would need a trivial fix.

We could first rename one of the unused of less frequently
used lists (Qemu-commits, Qemu-stable). If that works well, the
rest can be renamed.

I'd also like to have descriptions of the QEMU related mailing lists.
The listinfo (link above) shows none today, but it could be added
easily by the list maintainer(s).

Regards,

Stefan Weil

Re: [Qemu-devel] [PATCH 7/8] PPC: Turn hardcoded reset mask into env variable

[Alexander Graf]

On 21.06.2012, at 20:09, Blue Swirl  wrote:

> On Wed, Jun 20, 2012 at 8:11 PM, Alexander Graf  wrote:
>> Some machines have MSR bits they reset with as enabled. Don't hardcode the
>> logic, but let the individual core implementations save their own reset
>> mask into an env variable.
>> 
>> Signed-off-by: Alexander Graf 
>> ---
>>  target-ppc/cpu.h|1 +
>>  target-ppc/translate_init.c |   14 --
>>  2 files changed, 9 insertions(+), 6 deletions(-)
>> 
>> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
>> index 652a35a..acf5816 100644
>> --- a/target-ppc/cpu.h
>> +++ b/target-ppc/cpu.h
>> @@ -1043,6 +1043,7 @@ struct CPUPPCState {
>>  #if defined(TARGET_PPC64)
>> struct ppc_segment_page_sizes sps;
>>  #endif
>> +uint64_t reset_msr;
>> 
>>  #if defined(TARGET_PPC64) && !defined(CONFIG_USER_ONLY)
>> target_phys_addr_t vpa;
>> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
>> index 57027a2..efa05fc 100644
>> --- a/target-ppc/translate_init.c
>> +++ b/target-ppc/translate_init.c
>> @@ -6273,6 +6273,7 @@ static void init_proc_970 (CPUPPCState *env)
>> env->slb_nr = 32;
>>  #endif
>> init_excp_970(env);
>> +env->reset_msr = (1ULL < MSR_SF);
> 
> The parenthesis are not necessary.

Already dropped this patch :).

Alex

Re: [Qemu-devel] [RFC] QEMU mailing list changes

[Peter Maydell]

On 21 June 2012 19:37, Stefan Weil  wrote:
> I suggest a small change of the QEMU related mailing lists
> (see https://lists.nongnu.org/mailman/listinfo/):
>
> Replace Qemu-devel by QEMU-devel, Qemu-trivial by QEMU-trivial,
> and so on.
>
> This matches the official project name which is QEMU (not Qemu).
> As far as I know, there is no convention which prefers Qemu-devel,
> and there are mailing lists on lists.nongnu.org with any kind
> of upper and lower case mixed.
>
> Mailing addresses are case insensitive and the links on the server
> are lower case, therefore I expect that the change of the name
> on lists.nongnu.org will work without much trouble.

Technically, local parts in email addresses can be case sensitive
(it's a site decision whether they are). But you're proposing a
change to the Mailman 'real name', as I understand it -- the
'posting name' used in URLs and email addresses by mailman is
always lower case anyway, so it wouldn't change.

-- PMM

Re: [Qemu-devel] [PATCH 6/7] Exit loop if we have been there too long

[Juan Quintela]

Orit Wasserman  wrote:
> On 05/22/2012 09:32 PM, Juan Quintela wrote:
>> cheking each 64 pages is a random magic number as good as any other.
> s/cheking/checking

Done.

>> +*/
>> +if ((i & 63) == 0) {
>> +uint64_t t1 = (qemu_get_clock_ns(rt_clock) - bwidth) / 100;
>> +if (t1 > 50) { /* 50ms, half buffered_file limit */
> can't we use a constant ?

50 is a constant already, no?  Or what do you mean.

>> +printf("big delay %ld milliseconds, %d iterations\n", t1, 
>> i);
> printf ? 

This is the kind of "this shouldn't happen", but still happens, DPRINTF?

Thanks, Juan.

Re: [Qemu-devel] [RFC] QEMU mailing list changes

[Sebastien Douche]

On Thu, Jun 21, 2012 at 8:37 PM, Stefan Weil  wrote:
> Replace Qemu-devel by QEMU-devel, Qemu-trivial by QEMU-trivial,
> and so on.

I suggest to remove Qemu-devel, email clients can handle filtering.



-- 
Sebastien Douche 
Twitter: @sdouche / G+: +sdouche

[Qemu-devel] [PATCH] make: Fix dependencies for fpu/*.c and tcg/*.c

[Stefan Weil]

Commit dcff25f2cd8c11a9368cc2369aeb0319c32d9e26 removed too many *.d
files. The directories fpu/ and tcg/ still don't use the recursive
subdir rules.

Signed-off-by: Stefan Weil 
---
 Makefile.target |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile.target b/Makefile.target
index 550d889..8f12b0f 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -216,4 +216,4 @@ GENERATED_HEADERS += config-target.h
 Makefile: $(GENERATED_HEADERS)
 
 # Include automatically generated dependency files
--include $(wildcard *.d)
+-include $(wildcard *.d fpu/*.d tcg/*.d)
-- 
1.7.10

[Qemu-devel] [PATCH v4 00/17] x86 AREG0 conversion

[Blue Swirl]

This series actually does not do much conversion.

These should be safe, so I'd like to apply them soon.

Further AREG0 conversions need more work.

Blue Swirl (17):
  x86: prepare op_helper.c for splitting
  x86: avoid AREG0 for exceptions
  x86: split off exception handlers
  x86: avoid an extern declaration
  x86: fix coding style in ops_sse.h
  x86: split off FPU helpers
  x86: improve SSE table type safety
  x86: fix coding style in helper_template.h
  x86: split condition code and shift templates
  x86: prepare eflags helpers for general use
  x86: split off condition code helpers
  x86: split off integer helpers
  x86: split off SVM helpers
  x86: split off SMM helpers
  x86: split off misc helpers
  x86: split off memory access helpers
  x86: rename op_helper.c to seg_helper.c

 cpu-exec.c  |   12 +-
 target-i386/Makefile.objs   |   13 +-
 target-i386/cc_helper.c |  387 +++
 target-i386/cc_helper_template.h|  277 ++
 target-i386/cpu.h   |   62 +-
 target-i386/excp_helper.c   |  129 +
 target-i386/fpu_helper.c| 1304 
 target-i386/helper.c|4 +-
 target-i386/helper.h|4 +-
 target-i386/helper_template.h   |  334 --
 target-i386/int_helper.c|  500 +++
 target-i386/mem_helper.c|  161 +
 target-i386/misc_helper.c   |  603 
 target-i386/op_helper.c | 5923 ---
 target-i386/ops_sse.h   | 1049 ---
 target-i386/seg_helper.c| 2475 +++
 target-i386/shift_helper_template.h |  110 +
 target-i386/smm_helper.c|  307 ++
 target-i386/svm_helper.c|  716 +
 target-i386/translate.c |  138 +-
 user-exec.c |2 +-
 21 files changed, 7717 insertions(+), 6793 deletions(-)
 create mode 100644 target-i386/cc_helper.c
 create mode 100644 target-i386/cc_helper_template.h
 create mode 100644 target-i386/excp_helper.c
 create mode 100644 target-i386/fpu_helper.c
 delete mode 100644 target-i386/helper_template.h
 create mode 100644 target-i386/int_helper.c
 create mode 100644 target-i386/mem_helper.c
 create mode 100644 target-i386/misc_helper.c
 delete mode 100644 target-i386/op_helper.c
 create mode 100644 target-i386/seg_helper.c
 create mode 100644 target-i386/shift_helper_template.h
 create mode 100644 target-i386/smm_helper.c
 create mode 100644 target-i386/svm_helper.c

-- 
1.7.2.5

[Qemu-devel] [PATCH 03/17] x86: split off exception handlers

[Blue Swirl]

Move exception handlers from op_helper.c to excp_helper.c.

Signed-off-by: Blue Swirl 
---
 target-i386/Makefile.objs |1 +
 target-i386/cpu.h |   10 +++-
 target-i386/excp_helper.c |  132 +
 target-i386/op_helper.c   |  113 --
 4 files changed, 140 insertions(+), 116 deletions(-)
 create mode 100644 target-i386/excp_helper.c

diff --git a/target-i386/Makefile.objs b/target-i386/Makefile.objs
index f913755..c0feffe 100644
--- a/target-i386/Makefile.objs
+++ b/target-i386/Makefile.objs
@@ -1,4 +1,5 @@
 obj-y += translate.o op_helper.o helper.o cpu.o
+obj-y += excp_helper.o
 obj-$(CONFIG_SOFTMMU) += machine.o arch_memory_mapping.o arch_dump.o
 obj-$(CONFIG_KVM) += kvm.o hyperv.o
 obj-$(CONFIG_LINUX_USER) += ioport-user.o
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index c546723..cea8ecc 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -1071,12 +1071,16 @@ void cpu_x86_inject_mce(Monitor *mon, CPUX86State 
*cenv, int bank,
 uint64_t status, uint64_t mcg_status, uint64_t addr,
 uint64_t misc, int flags);
 
-/* op_helper.c */
-void do_interrupt(CPUX86State *env);
-void do_interrupt_x86_hardirq(CPUX86State *env, int intno, int is_hw);
+/* excp_helper.c */
 void QEMU_NORETURN raise_exception(CPUX86State *env, int exception_index);
 void QEMU_NORETURN raise_exception_err(CPUX86State *env, int exception_index,
int error_code);
+void QEMU_NORETURN raise_interrupt(CPUX86State *nenv, int intno, int is_int,
+   int error_code, int next_eip_addend);
+
+/* op_helper.c */
+void do_interrupt(CPUX86State *env);
+void do_interrupt_x86_hardirq(CPUX86State *env, int intno, int is_hw);
 
 void do_smm_enter(CPUX86State *env1);
 
diff --git a/target-i386/excp_helper.c b/target-i386/excp_helper.c
new file mode 100644
index 000..72bd46d
--- /dev/null
+++ b/target-i386/excp_helper.c
@@ -0,0 +1,132 @@
+/*
+ *  x86 exception helpers
+ *
+ *  Copyright (c) 2003 Fabrice Bellard
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see .
+ */
+
+#include "cpu.h"
+#include "qemu-log.h"
+#include "helper.h"
+
+#if 0
+#define raise_exception_err(env, a, b)  \
+do {\
+qemu_log("raise_exception line=%d\n", __LINE__);\
+(raise_exception_err)(env, a, b);   \
+} while (0)
+#endif
+
+void helper_raise_interrupt(CPUX86State *env, int intno, int next_eip_addend)
+{
+raise_interrupt(env, intno, 1, 0, next_eip_addend);
+}
+
+void helper_raise_exception(CPUX86State *env, int exception_index)
+{
+raise_exception(env, exception_index);
+}
+
+
+/* This should come from sysemu.h - if we could include it here... */
+void qemu_system_reset_request(void);
+
+/*
+ * Check nested exceptions and change to double or triple fault if
+ * needed. It should only be called, if this is not an interrupt.
+ * Returns the new exception number.
+ */
+static int check_exception(CPUX86State *env, int intno, int *error_code)
+{
+int first_contributory = env->old_exception == 0 ||
+  (env->old_exception >= 10 &&
+   env->old_exception <= 13);
+int second_contributory = intno == 0 ||
+   (intno >= 10 && intno <= 13);
+
+qemu_log_mask(CPU_LOG_INT, "check_exception old: 0x%x new 0x%x\n",
+env->old_exception, intno);
+
+#if !defined(CONFIG_USER_ONLY)
+if (env->old_exception == EXCP08_DBLE) {
+if (env->hflags & HF_SVMI_MASK) {
+cpu_vmexit(env, SVM_EXIT_SHUTDOWN, 0); /* does not return */
+}
+
+qemu_log_mask(CPU_LOG_RESET, "Triple fault\n");
+
+qemu_system_reset_request();
+return EXCP_HLT;
+}
+#endif
+
+if ((first_contributory && second_contributory)
+|| (env->old_exception == EXCP0E_PAGE &&
+(second_contributory || (intno == EXCP0E_PAGE {
+intno = EXCP08_DBLE;
+*error_code = 0;
+}
+
+if (second_contributory || (intno == EXCP0E_PAGE) ||
+(intno == EXCP08_DBLE)) {
+env->old_exception = intno;
+}
+
+return intno;
+}
+
+/*
+ * Signal a

[Qemu-devel] [PATCH 14/17] x86: split off SMM helpers

[Blue Swirl]

Move SMM helpers to smm_helper.c.

Signed-off-by: Blue Swirl 
---
 target-i386/Makefile.objs |2 +
 target-i386/op_helper.c   |  285 -
 target-i386/smm_helper.c  |  307 +
 3 files changed, 309 insertions(+), 285 deletions(-)
 create mode 100644 target-i386/smm_helper.c

diff --git a/target-i386/Makefile.objs b/target-i386/Makefile.objs
index 25bea43..a2ba717 100644
--- a/target-i386/Makefile.objs
+++ b/target-i386/Makefile.objs
@@ -1,5 +1,6 @@
 obj-y += translate.o op_helper.o helper.o cpu.o
 obj-y += excp_helper.o fpu_helper.o cc_helper.o int_helper.o svm_helper.o
+obj-y += smm_helper.o
 obj-$(CONFIG_SOFTMMU) += machine.o arch_memory_mapping.o arch_dump.o
 obj-$(CONFIG_KVM) += kvm.o hyperv.o
 obj-$(CONFIG_LINUX_USER) += ioport-user.o
@@ -10,3 +11,4 @@ $(obj)/fpu_helper.o: QEMU_CFLAGS += $(HELPER_CFLAGS)
 $(obj)/cc_helper.o: QEMU_CFLAGS += $(HELPER_CFLAGS)
 $(obj)/int_helper.o: QEMU_CFLAGS += $(HELPER_CFLAGS)
 $(obj)/svm_helper.o: QEMU_CFLAGS += $(HELPER_CFLAGS)
+$(obj)/smm_helper.o: QEMU_CFLAGS += $(HELPER_CFLAGS)
diff --git a/target-i386/op_helper.c b/target-i386/op_helper.c
index 4c4974e..0d31afa 100644
--- a/target-i386/op_helper.c
+++ b/target-i386/op_helper.c
@@ -1353,291 +1353,6 @@ void do_interrupt_x86_hardirq(CPUX86State *env1, int 
intno, int is_hw)
 env = saved_env;
 }
 
-/* SMM support */
-
-#if defined(CONFIG_USER_ONLY)
-
-void do_smm_enter(CPUX86State *env1)
-{
-}
-
-void helper_rsm(void)
-{
-}
-
-#else
-
-#ifdef TARGET_X86_64
-#define SMM_REVISION_ID 0x00020064
-#else
-#define SMM_REVISION_ID 0x0002
-#endif
-
-void do_smm_enter(CPUX86State *env1)
-{
-target_ulong sm_state;
-SegmentCache *dt;
-int i, offset;
-CPUX86State *saved_env;
-
-saved_env = env;
-env = env1;
-
-qemu_log_mask(CPU_LOG_INT, "SMM: enter\n");
-log_cpu_state_mask(CPU_LOG_INT, env, X86_DUMP_CCOP);
-
-env->hflags |= HF_SMM_MASK;
-cpu_smm_update(env);
-
-sm_state = env->smbase + 0x8000;
-
-#ifdef TARGET_X86_64
-for (i = 0; i < 6; i++) {
-dt = &env->segs[i];
-offset = 0x7e00 + i * 16;
-stw_phys(sm_state + offset, dt->selector);
-stw_phys(sm_state + offset + 2, (dt->flags >> 8) & 0xf0ff);
-stl_phys(sm_state + offset + 4, dt->limit);
-stq_phys(sm_state + offset + 8, dt->base);
-}
-
-stq_phys(sm_state + 0x7e68, env->gdt.base);
-stl_phys(sm_state + 0x7e64, env->gdt.limit);
-
-stw_phys(sm_state + 0x7e70, env->ldt.selector);
-stq_phys(sm_state + 0x7e78, env->ldt.base);
-stl_phys(sm_state + 0x7e74, env->ldt.limit);
-stw_phys(sm_state + 0x7e72, (env->ldt.flags >> 8) & 0xf0ff);
-
-stq_phys(sm_state + 0x7e88, env->idt.base);
-stl_phys(sm_state + 0x7e84, env->idt.limit);
-
-stw_phys(sm_state + 0x7e90, env->tr.selector);
-stq_phys(sm_state + 0x7e98, env->tr.base);
-stl_phys(sm_state + 0x7e94, env->tr.limit);
-stw_phys(sm_state + 0x7e92, (env->tr.flags >> 8) & 0xf0ff);
-
-stq_phys(sm_state + 0x7ed0, env->efer);
-
-stq_phys(sm_state + 0x7ff8, EAX);
-stq_phys(sm_state + 0x7ff0, ECX);
-stq_phys(sm_state + 0x7fe8, EDX);
-stq_phys(sm_state + 0x7fe0, EBX);
-stq_phys(sm_state + 0x7fd8, ESP);
-stq_phys(sm_state + 0x7fd0, EBP);
-stq_phys(sm_state + 0x7fc8, ESI);
-stq_phys(sm_state + 0x7fc0, EDI);
-for (i = 8; i < 16; i++) {
-stq_phys(sm_state + 0x7ff8 - i * 8, env->regs[i]);
-}
-stq_phys(sm_state + 0x7f78, env->eip);
-stl_phys(sm_state + 0x7f70, cpu_compute_eflags(env));
-stl_phys(sm_state + 0x7f68, env->dr[6]);
-stl_phys(sm_state + 0x7f60, env->dr[7]);
-
-stl_phys(sm_state + 0x7f48, env->cr[4]);
-stl_phys(sm_state + 0x7f50, env->cr[3]);
-stl_phys(sm_state + 0x7f58, env->cr[0]);
-
-stl_phys(sm_state + 0x7efc, SMM_REVISION_ID);
-stl_phys(sm_state + 0x7f00, env->smbase);
-#else
-stl_phys(sm_state + 0x7ffc, env->cr[0]);
-stl_phys(sm_state + 0x7ff8, env->cr[3]);
-stl_phys(sm_state + 0x7ff4, cpu_compute_eflags(env));
-stl_phys(sm_state + 0x7ff0, env->eip);
-stl_phys(sm_state + 0x7fec, EDI);
-stl_phys(sm_state + 0x7fe8, ESI);
-stl_phys(sm_state + 0x7fe4, EBP);
-stl_phys(sm_state + 0x7fe0, ESP);
-stl_phys(sm_state + 0x7fdc, EBX);
-stl_phys(sm_state + 0x7fd8, EDX);
-stl_phys(sm_state + 0x7fd4, ECX);
-stl_phys(sm_state + 0x7fd0, EAX);
-stl_phys(sm_state + 0x7fcc, env->dr[6]);
-stl_phys(sm_state + 0x7fc8, env->dr[7]);
-
-stl_phys(sm_state + 0x7fc4, env->tr.selector);
-stl_phys(sm_state + 0x7f64, env->tr.base);
-stl_phys(sm_state + 0x7f60, env->tr.limit);
-stl_phys(sm_state + 0x7f5c, (env->tr.flags >> 8) & 0xf0ff);
-
-stl_phys(sm_state + 0x7fc0, env->ldt.selector);
-stl_phys(sm_state + 0x7f80, env->ldt.base);
-stl_phys(sm_state + 0x7f7c, env->ldt.limit);
-stl_phys(sm_state + 0x7f78, (env->ldt.flags >> 8) & 0xf0ff);
-
-stl_phys(sm_state + 0x7f74, 

[Qemu-devel] [PATCH RFT 0/9] x86 AREG0 conversion, part deux

[Blue Swirl]

These still fail the test but I'll send them for review and to
complement the first series.

Blue Swirl (9):
  x86: avoid AREG0 for FPU helpers
  x86: avoid AREG0 for condition code helpers
  x86: avoid AREG0 for integer helpers
  x86: avoid AREG0 for SVM helpers
  x86: avoid AREG0 for SMM helpers
  x86: avoid AREG0 for misc helpers
  x86: use wrappers for memory access helpers
  x86: avoid AREG0 in segmentation helpers
  x86: switch to AREG0 free mode

 configure   |2 +-
 cpu-all.h   |   22 +
 target-i386/Makefile.objs   |9 -
 target-i386/cc_helper.c |  199 
 target-i386/cc_helper_template.h|   36 +-
 target-i386/fpu_helper.c|  429 -
 target-i386/helper.h|  352 +++---
 target-i386/int_helper.c|   44 +-
 target-i386/mem_helper.c|   46 +-
 target-i386/misc_helper.c   |   75 ++--
 target-i386/ops_sse.h   |  378 ---
 target-i386/ops_sse_header.h|  334 +++---
 target-i386/seg_helper.c|  434 -
 target-i386/shift_helper_template.h |   10 +-
 target-i386/smm_helper.c|   14 +-
 target-i386/svm_helper.c|  185 
 target-i386/translate.c |  923 +++
 17 files changed, 1781 insertions(+), 1711 deletions(-)

-- 
1.7.2.5

[Qemu-devel] [PATCH 2/9] x86: avoid AREG0 for condition code helpers

[Blue Swirl]

Add an explicit CPUX86State parameter instead of relying on AREG0.

Signed-off-by: Blue Swirl 
---
 target-i386/Makefile.objs   |1 -
 target-i386/cc_helper.c |  199 +--
 target-i386/cc_helper_template.h|   36 +++---
 target-i386/helper.h|   20 ++--
 target-i386/int_helper.c|8 +-
 target-i386/mem_helper.c|4 +-
 target-i386/misc_helper.c   |2 +-
 target-i386/seg_helper.c|8 +-
 target-i386/shift_helper_template.h |4 +-
 target-i386/translate.c |   66 
 10 files changed, 179 insertions(+), 169 deletions(-)

diff --git a/target-i386/Makefile.objs b/target-i386/Makefile.objs
index af99b81..fab2385 100644
--- a/target-i386/Makefile.objs
+++ b/target-i386/Makefile.objs
@@ -6,7 +6,6 @@ obj-$(CONFIG_KVM) += kvm.o hyperv.o
 obj-$(CONFIG_LINUX_USER) += ioport-user.o
 obj-$(CONFIG_BSD_USER) += ioport-user.o
 
-$(obj)/cc_helper.o: QEMU_CFLAGS += $(HELPER_CFLAGS)
 $(obj)/int_helper.o: QEMU_CFLAGS += $(HELPER_CFLAGS)
 $(obj)/svm_helper.o: QEMU_CFLAGS += $(HELPER_CFLAGS)
 $(obj)/smm_helper.o: QEMU_CFLAGS += $(HELPER_CFLAGS)
diff --git a/target-i386/cc_helper.c b/target-i386/cc_helper.c
index ff654bc..07892f9 100644
--- a/target-i386/cc_helper.c
+++ b/target-i386/cc_helper.c
@@ -18,7 +18,6 @@
  */
 
 #include "cpu.h"
-#include "dyngen-exec.h"
 #include "helper.h"
 
 const uint8_t parity_table[256] = {
@@ -76,184 +75,177 @@ const uint8_t parity_table[256] = {
 
 #endif
 
-static int compute_all_eflags(void)
+static int compute_all_eflags(CPUX86State *env)
 {
 return CC_SRC;
 }
 
-static int compute_c_eflags(void)
+static int compute_c_eflags(CPUX86State *env)
 {
 return CC_SRC & CC_C;
 }
 
-uint32_t helper_cc_compute_all(int op)
+uint32_t helper_cc_compute_all(CPUX86State *env, int op)
 {
 switch (op) {
 default: /* should never happen */
 return 0;
 
 case CC_OP_EFLAGS:
-return compute_all_eflags();
+return compute_all_eflags(env);
 
 case CC_OP_MULB:
-return compute_all_mulb();
+return compute_all_mulb(env);
 case CC_OP_MULW:
-return compute_all_mulw();
+return compute_all_mulw(env);
 case CC_OP_MULL:
-return compute_all_mull();
+return compute_all_mull(env);
 
 case CC_OP_ADDB:
-return compute_all_addb();
+return compute_all_addb(env);
 case CC_OP_ADDW:
-return compute_all_addw();
+return compute_all_addw(env);
 case CC_OP_ADDL:
-return compute_all_addl();
+return compute_all_addl(env);
 
 case CC_OP_ADCB:
-return compute_all_adcb();
+return compute_all_adcb(env);
 case CC_OP_ADCW:
-return compute_all_adcw();
+return compute_all_adcw(env);
 case CC_OP_ADCL:
-return compute_all_adcl();
+return compute_all_adcl(env);
 
 case CC_OP_SUBB:
-return compute_all_subb();
+return compute_all_subb(env);
 case CC_OP_SUBW:
-return compute_all_subw();
+return compute_all_subw(env);
 case CC_OP_SUBL:
-return compute_all_subl();
+return compute_all_subl(env);
 
 case CC_OP_SBBB:
-return compute_all_sbbb();
+return compute_all_sbbb(env);
 case CC_OP_SBBW:
-return compute_all_sbbw();
+return compute_all_sbbw(env);
 case CC_OP_SBBL:
-return compute_all_sbbl();
+return compute_all_sbbl(env);
 
 case CC_OP_LOGICB:
-return compute_all_logicb();
+return compute_all_logicb(env);
 case CC_OP_LOGICW:
-return compute_all_logicw();
+return compute_all_logicw(env);
 case CC_OP_LOGICL:
-return compute_all_logicl();
+return compute_all_logicl(env);
 
 case CC_OP_INCB:
-return compute_all_incb();
+return compute_all_incb(env);
 case CC_OP_INCW:
-return compute_all_incw();
+return compute_all_incw(env);
 case CC_OP_INCL:
-return compute_all_incl();
+return compute_all_incl(env);
 
 case CC_OP_DECB:
-return compute_all_decb();
+return compute_all_decb(env);
 case CC_OP_DECW:
-return compute_all_decw();
+return compute_all_decw(env);
 case CC_OP_DECL:
-return compute_all_decl();
+return compute_all_decl(env);
 
 case CC_OP_SHLB:
-return compute_all_shlb();
+return compute_all_shlb(env);
 case CC_OP_SHLW:
-return compute_all_shlw();
+return compute_all_shlw(env);
 case CC_OP_SHLL:
-return compute_all_shll();
+return compute_all_shll(env);
 
 case CC_OP_SARB:
-return compute_all_sarb();
+return compute_all_sarb(env);
 case CC_OP_SARW:
-return compute_all_sarw();
+return compute_all_sarw(env);
 case CC_OP_SARL:
-return compute_all_sarl();
+return compute_all_sarl(env

[Qemu-devel] [PATCH 5/9] x86: avoid AREG0 for SMM helpers

[Blue Swirl]

Add an explicit CPUX86State parameter instead of relying on AREG0.

Signed-off-by: Blue Swirl 
---
 target-i386/Makefile.objs |1 -
 target-i386/helper.h  |2 +-
 target-i386/smm_helper.c  |   14 --
 target-i386/translate.c   |2 +-
 4 files changed, 6 insertions(+), 13 deletions(-)

diff --git a/target-i386/Makefile.objs b/target-i386/Makefile.objs
index 370fde7..f843fe9 100644
--- a/target-i386/Makefile.objs
+++ b/target-i386/Makefile.objs
@@ -6,7 +6,6 @@ obj-$(CONFIG_KVM) += kvm.o hyperv.o
 obj-$(CONFIG_LINUX_USER) += ioport-user.o
 obj-$(CONFIG_BSD_USER) += ioport-user.o
 
-$(obj)/smm_helper.o: QEMU_CFLAGS += $(HELPER_CFLAGS)
 $(obj)/misc_helper.o: QEMU_CFLAGS += $(HELPER_CFLAGS)
 $(obj)/mem_helper.o: QEMU_CFLAGS += $(HELPER_CFLAGS)
 $(obj)/seg_helper.o: QEMU_CFLAGS += $(HELPER_CFLAGS)
diff --git a/target-i386/helper.h b/target-i386/helper.h
index 601b8dd..ec7edca 100644
--- a/target-i386/helper.h
+++ b/target-i386/helper.h
@@ -71,7 +71,7 @@ DEF_HELPER_1(set_inhibit_irq, void, env)
 DEF_HELPER_1(reset_inhibit_irq, void, env)
 DEF_HELPER_2(boundw, void, tl, int)
 DEF_HELPER_2(boundl, void, tl, int)
-DEF_HELPER_0(rsm, void)
+DEF_HELPER_1(rsm, void, env)
 DEF_HELPER_1(into, void, int)
 DEF_HELPER_1(cmpxchg8b, void, tl)
 #ifdef TARGET_X86_64
diff --git a/target-i386/smm_helper.c b/target-i386/smm_helper.c
index bc1bfa2..8b04eb2 100644
--- a/target-i386/smm_helper.c
+++ b/target-i386/smm_helper.c
@@ -18,18 +18,17 @@
  */
 
 #include "cpu.h"
-#include "dyngen-exec.h"
 #include "helper.h"
 
 /* SMM support */
 
 #if defined(CONFIG_USER_ONLY)
 
-void do_smm_enter(CPUX86State *env1)
+void do_smm_enter(CPUX86State *env)
 {
 }
 
-void helper_rsm(void)
+void helper_rsm(CPUX86State *env)
 {
 }
 
@@ -41,15 +40,11 @@ void helper_rsm(void)
 #define SMM_REVISION_ID 0x0002
 #endif
 
-void do_smm_enter(CPUX86State *env1)
+void do_smm_enter(CPUX86State *env)
 {
 target_ulong sm_state;
 SegmentCache *dt;
 int i, offset;
-CPUX86State *saved_env;
-
-saved_env = env;
-env = env1;
 
 qemu_log_mask(CPU_LOG_INT, "SMM: enter\n");
 log_cpu_state_mask(CPU_LOG_INT, env, X86_DUMP_CCOP);
@@ -180,10 +175,9 @@ void do_smm_enter(CPUX86State *env1)
 cpu_x86_update_cr4(env, 0);
 env->dr[7] = 0x0400;
 CC_OP = CC_OP_EFLAGS;
-env = saved_env;
 }
 
-void helper_rsm(void)
+void helper_rsm(CPUX86State *env)
 {
 target_ulong sm_state;
 int i, offset;
diff --git a/target-i386/translate.c b/target-i386/translate.c
index 126a531..2ea52ca 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -7700,7 +7700,7 @@ static target_ulong disas_insn(DisasContext *s, 
target_ulong pc_start)
 goto illegal_op;
 gen_update_cc_op(s);
 gen_jmp_im(s->pc - s->cs_base);
-gen_helper_rsm();
+gen_helper_rsm(cpu_env);
 gen_eob(s);
 break;
 case 0x1b8: /* SSE4.2 popcnt */
-- 
1.7.2.5

[Qemu-devel] [PATCH 4/9] x86: avoid AREG0 for SVM helpers

[Blue Swirl]

Add an explicit CPUX86State parameter instead of relying on AREG0.

Signed-off-by: Blue Swirl 
---
 target-i386/Makefile.objs |1 -
 target-i386/helper.h  |   22 +++---
 target-i386/svm_helper.c  |  181 ++---
 target-i386/translate.c   |   21 +++---
 4 files changed, 110 insertions(+), 115 deletions(-)

diff --git a/target-i386/Makefile.objs b/target-i386/Makefile.objs
index 71b7c7b..370fde7 100644
--- a/target-i386/Makefile.objs
+++ b/target-i386/Makefile.objs
@@ -6,7 +6,6 @@ obj-$(CONFIG_KVM) += kvm.o hyperv.o
 obj-$(CONFIG_LINUX_USER) += ioport-user.o
 obj-$(CONFIG_BSD_USER) += ioport-user.o
 
-$(obj)/svm_helper.o: QEMU_CFLAGS += $(HELPER_CFLAGS)
 $(obj)/smm_helper.o: QEMU_CFLAGS += $(HELPER_CFLAGS)
 $(obj)/misc_helper.o: QEMU_CFLAGS += $(HELPER_CFLAGS)
 $(obj)/mem_helper.o: QEMU_CFLAGS += $(HELPER_CFLAGS)
diff --git a/target-i386/helper.h b/target-i386/helper.h
index 67c81bf..601b8dd 100644
--- a/target-i386/helper.h
+++ b/target-i386/helper.h
@@ -95,17 +95,17 @@ DEF_HELPER_1(inw, tl, i32)
 DEF_HELPER_2(outl, void, i32, i32)
 DEF_HELPER_1(inl, tl, i32)
 
-DEF_HELPER_2(svm_check_intercept_param, void, i32, i64)
-DEF_HELPER_2(vmexit, void, i32, i64)
-DEF_HELPER_3(svm_check_io, void, i32, i32, i32)
-DEF_HELPER_2(vmrun, void, int, int)
-DEF_HELPER_0(vmmcall, void)
-DEF_HELPER_1(vmload, void, int)
-DEF_HELPER_1(vmsave, void, int)
-DEF_HELPER_0(stgi, void)
-DEF_HELPER_0(clgi, void)
-DEF_HELPER_0(skinit, void)
-DEF_HELPER_1(invlpga, void, int)
+DEF_HELPER_3(svm_check_intercept_param, void, env, i32, i64)
+DEF_HELPER_3(vmexit, void, env, i32, i64)
+DEF_HELPER_4(svm_check_io, void, env, i32, i32, i32)
+DEF_HELPER_3(vmrun, void, env, int, int)
+DEF_HELPER_1(vmmcall, void, env)
+DEF_HELPER_2(vmload, void, env, int)
+DEF_HELPER_2(vmsave, void, env, int)
+DEF_HELPER_1(stgi, void, env)
+DEF_HELPER_1(clgi, void, env)
+DEF_HELPER_1(skinit, void, env)
+DEF_HELPER_2(invlpga, void, env, int)
 
 /* x86 FPU */
 
diff --git a/target-i386/svm_helper.c b/target-i386/svm_helper.c
index 64d842c..f370ac5 100644
--- a/target-i386/svm_helper.c
+++ b/target-i386/svm_helper.c
@@ -18,46 +18,46 @@
  */
 
 #include "cpu.h"
-#include "dyngen-exec.h"
+#include "cpu-all.h"
 #include "helper.h"
 
 /* Secure Virtual Machine helpers */
 
 #if defined(CONFIG_USER_ONLY)
 
-void helper_vmrun(int aflag, int next_eip_addend)
+void helper_vmrun(CPUX86State *env, int aflag, int next_eip_addend)
 {
 }
 
-void helper_vmmcall(void)
+void helper_vmmcall(CPUX86State *env)
 {
 }
 
-void helper_vmload(int aflag)
+void helper_vmload(CPUX86State *env, int aflag)
 {
 }
 
-void helper_vmsave(int aflag)
+void helper_vmsave(CPUX86State *env, int aflag)
 {
 }
 
-void helper_stgi(void)
+void helper_stgi(CPUX86State *env)
 {
 }
 
-void helper_clgi(void)
+void helper_clgi(CPUX86State *env)
 {
 }
 
-void helper_skinit(void)
+void helper_skinit(CPUX86State *env)
 {
 }
 
-void helper_invlpga(int aflag)
+void helper_invlpga(CPUX86State *env, int aflag)
 {
 }
 
-void helper_vmexit(uint32_t exit_code, uint64_t exit_info_1)
+void helper_vmexit(CPUX86State *env, uint32_t exit_code, uint64_t exit_info_1)
 {
 }
 
@@ -65,7 +65,8 @@ void cpu_vmexit(CPUX86State *nenv, uint32_t exit_code, 
uint64_t exit_info_1)
 {
 }
 
-void helper_svm_check_intercept_param(uint32_t type, uint64_t param)
+void helper_svm_check_intercept_param(CPUX86State *env, uint32_t type,
+  uint64_t param)
 {
 }
 
@@ -74,13 +75,13 @@ void cpu_svm_check_intercept_param(CPUX86State *env, 
uint32_t type,
 {
 }
 
-void helper_svm_check_io(uint32_t port, uint32_t param,
+void helper_svm_check_io(CPUX86State *env, uint32_t port, uint32_t param,
  uint32_t next_eip_addend)
 {
 }
 #else
 
-static inline void svm_save_seg(target_phys_addr_t addr,
+static inline void svm_save_seg(CPUX86State *env, target_phys_addr_t addr,
 const SegmentCache *sc)
 {
 stw_phys(addr + offsetof(struct vmcb_seg, selector),
@@ -93,7 +94,8 @@ static inline void svm_save_seg(target_phys_addr_t addr,
  ((sc->flags >> 8) & 0xff) | ((sc->flags >> 12) & 0x0f00));
 }
 
-static inline void svm_load_seg(target_phys_addr_t addr, SegmentCache *sc)
+static inline void svm_load_seg(CPUX86State *env, target_phys_addr_t addr,
+SegmentCache *sc)
 {
 unsigned int flags;
 
@@ -104,23 +106,23 @@ static inline void svm_load_seg(target_phys_addr_t addr, 
SegmentCache *sc)
 sc->flags = ((flags & 0xff) << 8) | ((flags & 0x0f00) << 12);
 }
 
-static inline void svm_load_seg_cache(target_phys_addr_t addr,
-  CPUX86State *env, int seg_reg)
+static inline void svm_load_seg_cache(CPUX86State *env, target_phys_addr_t 
addr,
+  int seg_reg)
 {
 SegmentCache sc1, *sc = &sc1;
 
-svm_load_seg(addr, sc);
+svm_load_seg(env, addr, sc);
 cpu_x86_load_seg_cache(env, seg_reg, sc->selector,
  

[Qemu-devel] [PATCH 15/17] x86: split off misc helpers

[Blue Swirl]

Move various functions to misc_helper.c.

Signed-off-by: Blue Swirl 
---
 target-i386/Makefile.objs |3 +-
 target-i386/misc_helper.c |  603 +
 target-i386/op_helper.c   |  578 ---
 3 files changed, 605 insertions(+), 579 deletions(-)
 create mode 100644 target-i386/misc_helper.c

diff --git a/target-i386/Makefile.objs b/target-i386/Makefile.objs
index a2ba717..72bd423 100644
--- a/target-i386/Makefile.objs
+++ b/target-i386/Makefile.objs
@@ -1,6 +1,6 @@
 obj-y += translate.o op_helper.o helper.o cpu.o
 obj-y += excp_helper.o fpu_helper.o cc_helper.o int_helper.o svm_helper.o
-obj-y += smm_helper.o
+obj-y += smm_helper.o misc_helper.o
 obj-$(CONFIG_SOFTMMU) += machine.o arch_memory_mapping.o arch_dump.o
 obj-$(CONFIG_KVM) += kvm.o hyperv.o
 obj-$(CONFIG_LINUX_USER) += ioport-user.o
@@ -12,3 +12,4 @@ $(obj)/cc_helper.o: QEMU_CFLAGS += $(HELPER_CFLAGS)
 $(obj)/int_helper.o: QEMU_CFLAGS += $(HELPER_CFLAGS)
 $(obj)/svm_helper.o: QEMU_CFLAGS += $(HELPER_CFLAGS)
 $(obj)/smm_helper.o: QEMU_CFLAGS += $(HELPER_CFLAGS)
+$(obj)/misc_helper.o: QEMU_CFLAGS += $(HELPER_CFLAGS)
diff --git a/target-i386/misc_helper.c b/target-i386/misc_helper.c
new file mode 100644
index 000..ce675b7
--- /dev/null
+++ b/target-i386/misc_helper.c
@@ -0,0 +1,603 @@
+/*
+ *  x86 misc helpers
+ *
+ *  Copyright (c) 2003 Fabrice Bellard
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see .
+ */
+
+#include "cpu.h"
+#include "dyngen-exec.h"
+#include "ioport.h"
+#include "helper.h"
+
+#if !defined(CONFIG_USER_ONLY)
+#include "softmmu_exec.h"
+#endif /* !defined(CONFIG_USER_ONLY) */
+
+/* check if Port I/O is allowed in TSS */
+static inline void check_io(int addr, int size)
+{
+int io_offset, val, mask;
+
+/* TSS must be a valid 32 bit one */
+if (!(env->tr.flags & DESC_P_MASK) ||
+((env->tr.flags >> DESC_TYPE_SHIFT) & 0xf) != 9 ||
+env->tr.limit < 103) {
+goto fail;
+}
+io_offset = lduw_kernel(env->tr.base + 0x66);
+io_offset += (addr >> 3);
+/* Note: the check needs two bytes */
+if ((io_offset + 1) > env->tr.limit) {
+goto fail;
+}
+val = lduw_kernel(env->tr.base + io_offset);
+val >>= (addr & 7);
+mask = (1 << size) - 1;
+/* all bits must be zero to allow the I/O */
+if ((val & mask) != 0) {
+fail:
+raise_exception_err(env, EXCP0D_GPF, 0);
+}
+}
+
+void helper_check_iob(uint32_t t0)
+{
+check_io(t0, 1);
+}
+
+void helper_check_iow(uint32_t t0)
+{
+check_io(t0, 2);
+}
+
+void helper_check_iol(uint32_t t0)
+{
+check_io(t0, 4);
+}
+
+void helper_outb(uint32_t port, uint32_t data)
+{
+cpu_outb(port, data & 0xff);
+}
+
+target_ulong helper_inb(uint32_t port)
+{
+return cpu_inb(port);
+}
+
+void helper_outw(uint32_t port, uint32_t data)
+{
+cpu_outw(port, data & 0x);
+}
+
+target_ulong helper_inw(uint32_t port)
+{
+return cpu_inw(port);
+}
+
+void helper_outl(uint32_t port, uint32_t data)
+{
+cpu_outl(port, data);
+}
+
+target_ulong helper_inl(uint32_t port)
+{
+return cpu_inl(port);
+}
+
+void helper_into(int next_eip_addend)
+{
+int eflags;
+
+eflags = helper_cc_compute_all(CC_OP);
+if (eflags & CC_O) {
+raise_interrupt(env, EXCP04_INTO, 1, 0, next_eip_addend);
+}
+}
+
+void helper_single_step(void)
+{
+#ifndef CONFIG_USER_ONLY
+check_hw_breakpoints(env, 1);
+env->dr[6] |= DR6_BS;
+#endif
+raise_exception(env, EXCP01_DB);
+}
+
+void helper_cpuid(void)
+{
+uint32_t eax, ebx, ecx, edx;
+
+cpu_svm_check_intercept_param(env, SVM_EXIT_CPUID, 0);
+
+cpu_x86_cpuid(env, (uint32_t)EAX, (uint32_t)ECX, &eax, &ebx, &ecx, &edx);
+EAX = eax;
+EBX = ebx;
+ECX = ecx;
+EDX = edx;
+}
+
+#if defined(CONFIG_USER_ONLY)
+target_ulong helper_read_crN(int reg)
+{
+return 0;
+}
+
+void helper_write_crN(int reg, target_ulong t0)
+{
+}
+
+void helper_movl_drN_T0(int reg, target_ulong t0)
+{
+}
+#else
+target_ulong helper_read_crN(int reg)
+{
+target_ulong val;
+
+cpu_svm_check_intercept_param(env, SVM_EXIT_READ_CR0 + reg, 0);
+switch (reg) {
+default:
+val = env->cr[reg];
+break;
+case 8:
+if (!(env->hflags2 & HF2_VINTR_MASK)) {
+val = cpu_get_apic_tpr(env->apic_state);
+} else {
+va

[Qemu-devel] [PATCH 12/17] x86: split off integer helpers

[Blue Swirl]

Move integer and bit field helpers to int_helper.c.

Signed-off-by: Blue Swirl 
---
 target-i386/Makefile.objs |3 +-
 target-i386/int_helper.c  |  500 +
 target-i386/op_helper.c   |  478 ---
 3 files changed, 502 insertions(+), 479 deletions(-)
 create mode 100644 target-i386/int_helper.c

diff --git a/target-i386/Makefile.objs b/target-i386/Makefile.objs
index 96a2266..d4cbcd7 100644
--- a/target-i386/Makefile.objs
+++ b/target-i386/Makefile.objs
@@ -1,5 +1,5 @@
 obj-y += translate.o op_helper.o helper.o cpu.o
-obj-y += excp_helper.o fpu_helper.o cc_helper.o
+obj-y += excp_helper.o fpu_helper.o cc_helper.o int_helper.o
 obj-$(CONFIG_SOFTMMU) += machine.o arch_memory_mapping.o arch_dump.o
 obj-$(CONFIG_KVM) += kvm.o hyperv.o
 obj-$(CONFIG_LINUX_USER) += ioport-user.o
@@ -8,3 +8,4 @@ obj-$(CONFIG_BSD_USER) += ioport-user.o
 $(obj)/op_helper.o: QEMU_CFLAGS += $(HELPER_CFLAGS)
 $(obj)/fpu_helper.o: QEMU_CFLAGS += $(HELPER_CFLAGS)
 $(obj)/cc_helper.o: QEMU_CFLAGS += $(HELPER_CFLAGS)
+$(obj)/int_helper.o: QEMU_CFLAGS += $(HELPER_CFLAGS)
diff --git a/target-i386/int_helper.c b/target-i386/int_helper.c
new file mode 100644
index 000..e1f66f5
--- /dev/null
+++ b/target-i386/int_helper.c
@@ -0,0 +1,500 @@
+/*
+ *  x86 integer helpers
+ *
+ *  Copyright (c) 2003 Fabrice Bellard
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see .
+ */
+
+#include "cpu.h"
+#include "dyngen-exec.h"
+#include "host-utils.h"
+#include "helper.h"
+
+//#define DEBUG_MULDIV
+
+/* modulo 9 table */
+static const uint8_t rclb_table[32] = {
+0, 1, 2, 3, 4, 5, 6, 7,
+8, 0, 1, 2, 3, 4, 5, 6,
+7, 8, 0, 1, 2, 3, 4, 5,
+6, 7, 8, 0, 1, 2, 3, 4,
+};
+
+/* modulo 17 table */
+static const uint8_t rclw_table[32] = {
+0, 1, 2, 3, 4, 5, 6, 7,
+8, 9, 10, 11, 12, 13, 14, 15,
+16, 0, 1, 2, 3, 4, 5, 6,
+7, 8, 9, 10, 11, 12, 13, 14,
+};
+
+/* division, flags are undefined */
+
+void helper_divb_AL(target_ulong t0)
+{
+unsigned int num, den, q, r;
+
+num = (EAX & 0x);
+den = (t0 & 0xff);
+if (den == 0) {
+raise_exception(env, EXCP00_DIVZ);
+}
+q = (num / den);
+if (q > 0xff) {
+raise_exception(env, EXCP00_DIVZ);
+}
+q &= 0xff;
+r = (num % den) & 0xff;
+EAX = (EAX & ~0x) | (r << 8) | q;
+}
+
+void helper_idivb_AL(target_ulong t0)
+{
+int num, den, q, r;
+
+num = (int16_t)EAX;
+den = (int8_t)t0;
+if (den == 0) {
+raise_exception(env, EXCP00_DIVZ);
+}
+q = (num / den);
+if (q != (int8_t)q) {
+raise_exception(env, EXCP00_DIVZ);
+}
+q &= 0xff;
+r = (num % den) & 0xff;
+EAX = (EAX & ~0x) | (r << 8) | q;
+}
+
+void helper_divw_AX(target_ulong t0)
+{
+unsigned int num, den, q, r;
+
+num = (EAX & 0x) | ((EDX & 0x) << 16);
+den = (t0 & 0x);
+if (den == 0) {
+raise_exception(env, EXCP00_DIVZ);
+}
+q = (num / den);
+if (q > 0x) {
+raise_exception(env, EXCP00_DIVZ);
+}
+q &= 0x;
+r = (num % den) & 0x;
+EAX = (EAX & ~0x) | q;
+EDX = (EDX & ~0x) | r;
+}
+
+void helper_idivw_AX(target_ulong t0)
+{
+int num, den, q, r;
+
+num = (EAX & 0x) | ((EDX & 0x) << 16);
+den = (int16_t)t0;
+if (den == 0) {
+raise_exception(env, EXCP00_DIVZ);
+}
+q = (num / den);
+if (q != (int16_t)q) {
+raise_exception(env, EXCP00_DIVZ);
+}
+q &= 0x;
+r = (num % den) & 0x;
+EAX = (EAX & ~0x) | q;
+EDX = (EDX & ~0x) | r;
+}
+
+void helper_divl_EAX(target_ulong t0)
+{
+unsigned int den, r;
+uint64_t num, q;
+
+num = ((uint32_t)EAX) | ((uint64_t)((uint32_t)EDX) << 32);
+den = t0;
+if (den == 0) {
+raise_exception(env, EXCP00_DIVZ);
+}
+q = (num / den);
+r = (num % den);
+if (q > 0x) {
+raise_exception(env, EXCP00_DIVZ);
+}
+EAX = (uint32_t)q;
+EDX = (uint32_t)r;
+}
+
+void helper_idivl_EAX(target_ulong t0)
+{
+int den, r;
+int64_t num, q;
+
+num = ((uint32_t)EAX) | ((uint64_t)((uint32_t)EDX) << 32);
+den = t0;
+if (den == 0) {
+raise_exception(env, EXCP00_DIVZ);
+}
+q = (num / den);
+r = (num % den);
+if (q != (int32_t)q) {
+raise_exce

[Qemu-devel] [PATCH 6/9] x86: avoid AREG0 for misc helpers

[Blue Swirl]

Add an explicit CPUX86State parameter instead of relying on AREG0.

Signed-off-by: Blue Swirl 
---
 target-i386/Makefile.objs |1 -
 target-i386/helper.h  |   40 
 target-i386/misc_helper.c |   73 ++---
 target-i386/translate.c   |   49 +-
 4 files changed, 84 insertions(+), 79 deletions(-)

diff --git a/target-i386/Makefile.objs b/target-i386/Makefile.objs
index f843fe9..04e34f8 100644
--- a/target-i386/Makefile.objs
+++ b/target-i386/Makefile.objs
@@ -6,6 +6,5 @@ obj-$(CONFIG_KVM) += kvm.o hyperv.o
 obj-$(CONFIG_LINUX_USER) += ioport-user.o
 obj-$(CONFIG_BSD_USER) += ioport-user.o
 
-$(obj)/misc_helper.o: QEMU_CFLAGS += $(HELPER_CFLAGS)
 $(obj)/mem_helper.o: QEMU_CFLAGS += $(HELPER_CFLAGS)
 $(obj)/seg_helper.o: QEMU_CFLAGS += $(HELPER_CFLAGS)
diff --git a/target-i386/helper.h b/target-i386/helper.h
index ec7edca..9a9c064 100644
--- a/target-i386/helper.h
+++ b/target-i386/helper.h
@@ -41,12 +41,12 @@ DEF_HELPER_4(lcall_protected, void, int, tl, int, int)
 DEF_HELPER_1(iret_real, void, int)
 DEF_HELPER_2(iret_protected, void, int, int)
 DEF_HELPER_2(lret_protected, void, int, int)
-DEF_HELPER_1(read_crN, tl, int)
-DEF_HELPER_2(write_crN, void, int, tl)
-DEF_HELPER_1(lmsw, void, tl)
+DEF_HELPER_2(read_crN, tl, env, int)
+DEF_HELPER_3(write_crN, void, env, int, tl)
+DEF_HELPER_2(lmsw, void, env, tl)
 DEF_HELPER_1(clts, void, env)
-DEF_HELPER_2(movl_drN_T0, void, int, tl)
-DEF_HELPER_1(invlpg, void, tl)
+DEF_HELPER_3(movl_drN_T0, void, env, int, tl)
+DEF_HELPER_2(invlpg, void, env, tl)
 
 DEF_HELPER_3(enter_level, void, int, int, tl)
 #ifdef TARGET_X86_64
@@ -58,10 +58,10 @@ DEF_HELPER_1(sysexit, void, int)
 DEF_HELPER_1(syscall, void, int)
 DEF_HELPER_1(sysret, void, int)
 #endif
-DEF_HELPER_1(hlt, void, int)
-DEF_HELPER_1(monitor, void, tl)
-DEF_HELPER_1(mwait, void, int)
-DEF_HELPER_0(debug, void)
+DEF_HELPER_2(hlt, void, env, int)
+DEF_HELPER_2(monitor, void, env, tl)
+DEF_HELPER_2(mwait, void, env, int)
+DEF_HELPER_1(debug, void, env)
 DEF_HELPER_1(reset_rf, void, env)
 DEF_HELPER_3(raise_interrupt, void, env, int, int)
 DEF_HELPER_2(raise_exception, void, env, int)
@@ -72,22 +72,22 @@ DEF_HELPER_1(reset_inhibit_irq, void, env)
 DEF_HELPER_2(boundw, void, tl, int)
 DEF_HELPER_2(boundl, void, tl, int)
 DEF_HELPER_1(rsm, void, env)
-DEF_HELPER_1(into, void, int)
+DEF_HELPER_2(into, void, env, int)
 DEF_HELPER_1(cmpxchg8b, void, tl)
 #ifdef TARGET_X86_64
 DEF_HELPER_1(cmpxchg16b, void, tl)
 #endif
-DEF_HELPER_0(single_step, void)
-DEF_HELPER_0(cpuid, void)
-DEF_HELPER_0(rdtsc, void)
-DEF_HELPER_0(rdtscp, void)
-DEF_HELPER_0(rdpmc, void)
-DEF_HELPER_0(rdmsr, void)
-DEF_HELPER_0(wrmsr, void)
+DEF_HELPER_1(single_step, void, env)
+DEF_HELPER_1(cpuid, void, env)
+DEF_HELPER_1(rdtsc, void, env)
+DEF_HELPER_1(rdtscp, void, env)
+DEF_HELPER_1(rdpmc, void, env)
+DEF_HELPER_1(rdmsr, void, env)
+DEF_HELPER_1(wrmsr, void, env)
 
-DEF_HELPER_1(check_iob, void, i32)
-DEF_HELPER_1(check_iow, void, i32)
-DEF_HELPER_1(check_iol, void, i32)
+DEF_HELPER_2(check_iob, void, env, i32)
+DEF_HELPER_2(check_iow, void, env, i32)
+DEF_HELPER_2(check_iol, void, env, i32)
 DEF_HELPER_2(outb, void, i32, i32)
 DEF_HELPER_1(inb, tl, i32)
 DEF_HELPER_2(outw, void, i32, i32)
diff --git a/target-i386/misc_helper.c b/target-i386/misc_helper.c
index 272a636..a020379 100644
--- a/target-i386/misc_helper.c
+++ b/target-i386/misc_helper.c
@@ -18,7 +18,6 @@
  */
 
 #include "cpu.h"
-#include "dyngen-exec.h"
 #include "ioport.h"
 #include "helper.h"
 
@@ -27,7 +26,7 @@
 #endif /* !defined(CONFIG_USER_ONLY) */
 
 /* check if Port I/O is allowed in TSS */
-static inline void check_io(int addr, int size)
+static inline void check_io(CPUX86State *env, int addr, int size)
 {
 int io_offset, val, mask;
 
@@ -37,13 +36,13 @@ static inline void check_io(int addr, int size)
 env->tr.limit < 103) {
 goto fail;
 }
-io_offset = lduw_kernel(env->tr.base + 0x66);
+io_offset = cpu_lduw_kernel(env, env->tr.base + 0x66);
 io_offset += (addr >> 3);
 /* Note: the check needs two bytes */
 if ((io_offset + 1) > env->tr.limit) {
 goto fail;
 }
-val = lduw_kernel(env->tr.base + io_offset);
+val = cpu_lduw_kernel(env, env->tr.base + io_offset);
 val >>= (addr & 7);
 mask = (1 << size) - 1;
 /* all bits must be zero to allow the I/O */
@@ -53,19 +52,19 @@ static inline void check_io(int addr, int size)
 }
 }
 
-void helper_check_iob(uint32_t t0)
+void helper_check_iob(CPUX86State *env, uint32_t t0)
 {
-check_io(t0, 1);
+check_io(env, t0, 1);
 }
 
-void helper_check_iow(uint32_t t0)
+void helper_check_iow(CPUX86State *env, uint32_t t0)
 {
-check_io(t0, 2);
+check_io(env, t0, 2);
 }
 
-void helper_check_iol(uint32_t t0)
+void helper_check_iol(CPUX86State *env, uint32_t t0)
 {
-check_io(t0, 4);
+check_io(env, t0, 4);
 }
 
 void helper_outb(uint32_t port, uint32_t data)
@@ -

[Qemu-devel] [PATCH 7/9] x86: use wrappers for memory access helpers

[Blue Swirl]

Switch to wrapped versions of memory access functions.

Signed-off-by: Blue Swirl 
---
 target-i386/cpu.h|   10 ++
 target-i386/mem_helper.c |   10 ++
 target-i386/seg_helper.c |  209 +++---
 3 files changed, 126 insertions(+), 103 deletions(-)

diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index a010a57..6d196e6 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -1145,4 +1145,14 @@ void cpu_stw_data(CPUX86State *env, target_ulong ptr, 
uint32_t data);
 void cpu_stl_data(CPUX86State *env, target_ulong ptr, uint32_t data);
 void cpu_stq_data(CPUX86State *env, target_ulong ptr, uint64_t data);
 
+uint32_t cpu_ldub_kernel(CPUX86State *env, target_ulong ptr);
+uint32_t cpu_lduw_kernel(CPUX86State *env, target_ulong ptr);
+uint32_t cpu_ldl_kernel(CPUX86State *env, target_ulong ptr);
+uint64_t cpu_ldq_kernel(CPUX86State *env, target_ulong ptr);
+
+void cpu_stb_kernel(CPUX86State *env, target_ulong ptr, uint32_t data);
+void cpu_stw_kernel(CPUX86State *env, target_ulong ptr, uint32_t data);
+void cpu_stl_kernel(CPUX86State *env, target_ulong ptr, uint32_t data);
+void cpu_stq_kernel(CPUX86State *env, target_ulong ptr, uint64_t data);
+
 #endif /* CPU_I386_H */
diff --git a/target-i386/mem_helper.c b/target-i386/mem_helper.c
index 30c3bd0..3dd4406 100644
--- a/target-i386/mem_helper.c
+++ b/target-i386/mem_helper.c
@@ -190,6 +190,11 @@ WRAP_LD(uint32_t, ldub_data)
 WRAP_LD(uint32_t, lduw_data)
 WRAP_LD(uint32_t, ldl_data)
 WRAP_LD(uint64_t, ldq_data)
+
+WRAP_LD(uint32_t, ldub_kernel)
+WRAP_LD(uint32_t, lduw_kernel)
+WRAP_LD(uint32_t, ldl_kernel)
+WRAP_LD(uint64_t, ldq_kernel)
 #undef WRAP_LD
 
 #define WRAP_ST(datatype, fn)   \
@@ -207,4 +212,9 @@ WRAP_ST(uint32_t, stb_data)
 WRAP_ST(uint32_t, stw_data)
 WRAP_ST(uint32_t, stl_data)
 WRAP_ST(uint64_t, stq_data)
+
+WRAP_ST(uint32_t, stb_kernel)
+WRAP_ST(uint32_t, stw_kernel)
+WRAP_ST(uint32_t, stl_kernel)
+WRAP_ST(uint64_t, stq_kernel)
 #undef WRAP_ST
diff --git a/target-i386/seg_helper.c b/target-i386/seg_helper.c
index 41d146c..f5dcf01 100644
--- a/target-i386/seg_helper.c
+++ b/target-i386/seg_helper.c
@@ -23,10 +23,6 @@
 #include "qemu-log.h"
 #include "helper.h"
 
-#if !defined(CONFIG_USER_ONLY)
-#include "softmmu_exec.h"
-#endif /* !defined(CONFIG_USER_ONLY) */
-
 //#define DEBUG_PCALL
 
 #ifdef DEBUG_PCALL
@@ -56,8 +52,8 @@ static inline int load_segment(uint32_t *e1_ptr, uint32_t 
*e2_ptr,
 return -1;
 }
 ptr = dt->base + index;
-*e1_ptr = ldl_kernel(ptr);
-*e2_ptr = ldl_kernel(ptr + 4);
+*e1_ptr = cpu_ldl_kernel(env, ptr);
+*e2_ptr = cpu_ldl_kernel(env, ptr + 4);
 return 0;
 }
 
@@ -125,11 +121,11 @@ static inline void get_ss_esp_from_tss(uint32_t *ss_ptr,
 raise_exception_err(env, EXCP0A_TSS, env->tr.selector & 0xfffc);
 }
 if (shift == 0) {
-*esp_ptr = lduw_kernel(env->tr.base + index);
-*ss_ptr = lduw_kernel(env->tr.base + index + 2);
+*esp_ptr = cpu_lduw_kernel(env, env->tr.base + index);
+*ss_ptr = cpu_lduw_kernel(env, env->tr.base + index + 2);
 } else {
-*esp_ptr = ldl_kernel(env->tr.base + index);
-*ss_ptr = lduw_kernel(env->tr.base + index + 4);
+*esp_ptr = cpu_ldl_kernel(env, env->tr.base + index);
+*ss_ptr = cpu_lduw_kernel(env, env->tr.base + index + 4);
 }
 }
 
@@ -262,29 +258,30 @@ static void switch_tss(int tss_selector,
 /* read all the registers from the new TSS */
 if (type & 8) {
 /* 32 bit */
-new_cr3 = ldl_kernel(tss_base + 0x1c);
-new_eip = ldl_kernel(tss_base + 0x20);
-new_eflags = ldl_kernel(tss_base + 0x24);
+new_cr3 = cpu_ldl_kernel(env, tss_base + 0x1c);
+new_eip = cpu_ldl_kernel(env, tss_base + 0x20);
+new_eflags = cpu_ldl_kernel(env, tss_base + 0x24);
 for (i = 0; i < 8; i++) {
-new_regs[i] = ldl_kernel(tss_base + (0x28 + i * 4));
+new_regs[i] = cpu_ldl_kernel(env, tss_base + (0x28 + i * 4));
 }
 for (i = 0; i < 6; i++) {
-new_segs[i] = lduw_kernel(tss_base + (0x48 + i * 4));
+new_segs[i] = cpu_lduw_kernel(env, tss_base + (0x48 + i * 4));
 }
-new_ldt = lduw_kernel(tss_base + 0x60);
-new_trap = ldl_kernel(tss_base + 0x64);
+new_ldt = cpu_lduw_kernel(env, tss_base + 0x60);
+new_trap = cpu_ldl_kernel(env, tss_base + 0x64);
 } else {
 /* 16 bit */
 new_cr3 = 0;
-new_eip = lduw_kernel(tss_base + 0x0e);
-new_eflags = lduw_kernel(tss_base + 0x10);
+new_eip = cpu_lduw_kernel(env, tss_base + 0x0e);
+new_eflags = cpu_lduw_kernel(env, tss_base + 0x10);
 for (i = 0; i < 8; i++) {
-new_regs[i] = lduw_kernel(tss_base + (0x12 + i * 2)) | 0x;
+new_regs[i] = cpu_lduw_kernel(env, tss_base + (0x12 + i * 2)) |
+0x;
 }
 for (i