Re: [Qemu-devel] [RFC 0/3] Draft implementation of HPT resizing (qemu side)

[Bharata B Rao]

On Mon, Jan 18, 2016 at 04:44:38PM +1100, David Gibson wrote:
> Here is a draft qemu implementation of my proposed PAPR extension for
> allowing runtime resizing of a KVM/ppc64 guest's hash page table.
> That in turn will allow for more flexible memory hotplug.
> 
> This should work with the guest kernel side patches I also posted
> recently [1].
> 
> Still required to make this into a full implementation:
>   * Guest needs to auto-resize HPT on memory hotplug events
> 
>   * qemu needs to allocate HPT size based on current rather than
> maximum memory if the guest is HPT resize aware
> 
>   * KVM host side implementation
> 
>   * PAPR standardization

So with the current patchset (QEMU and guest kernel changes), I should
be able to change the HTAB size of a PR guest right ? I see the below
failure though:

[root@localhost ~]# cat /sys/kernel/debug/powerpc/pft-size 
24
[root@localhost ~]# echo 26 > /sys/kernel/debug/powerpc/pft-size
[   65.996845] lpar: Attempting to resize HPT to shift 26
[   65.996845] lpar: Attempting to resize HPT to shift 26
[   66.113596] lpar: HPT resize to shift 26 complete (109 ms / 6 ms)
[   66.113596] lpar: HPT resize to shift 26 complete (109 ms / 6 ms)

PR guest just hangs here while I see tons of below messages in
the 1st level guest:

KVM can't copy data from 0x3fff99e91400!
...
Couldn't emulate instruction 0x (op 0 xop 0)
kvmppc_handle_exit_pr: emulation at 700 failed ()

Regards,
Bharata.

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

[Denis V. Lunev]

On 01/12/2016 01:50 PM, Andrey Smetanin wrote:

The patch implements userspace exit 'KVM_EXIT_HYPERV_HCALL'
for Hyper-V VMBus hypercalls(postmsg, signalevent)
to handle these hypercalls by QEMU.

Signed-off-by: Andrey Smetanin 
Reviewed-by: Roman Kagan 
CC: Gleb Natapov 
CC: Paolo Bonzini 
CC: Joerg Roedel 
CC: "K. Y. Srinivasan" 
CC: Haiyang Zhang 
CC: Roman Kagan 
CC: Denis V. Lunev 
CC: qemu-devel@nongnu.org

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

  Documentation/virtual/kvm/api.txt  |  8 
  arch/x86/include/uapi/asm/hyperv.h |  4 +++-
  arch/x86/kvm/hyperv.c  | 39 +-
  arch/x86/kvm/hyperv.h  |  1 +
  arch/x86/kvm/svm.c |  3 +--
  arch/x86/kvm/vmx.c |  3 +--
  arch/x86/kvm/x86.c |  3 +++
  drivers/hv/hv.c|  5 +++--
  drivers/hv/hyperv_vmbus.h  |  6 --
  include/uapi/linux/kvm.h   |  7 +++
  10 files changed, 57 insertions(+), 22 deletions(-)


ping

Re: [Qemu-devel] [PATCH v1] kvm/x86: Hyper-V tsc page setup

[Denis V. Lunev]

On 12/24/2015 12:33 PM, Andrey Smetanin wrote:

Lately tsc page was implemented but filled with empty
values. This patch setup tsc page scale and offset based
on vcpu tsc, tsc_khz and  HV_X64_MSR_TIME_REF_COUNT value.

The valid tsc page drops HV_X64_MSR_TIME_REF_COUNT msr
reads count to zero which potentially improves performance.

The patch applies on top of
'kvm: Make vcpu->requests as 64 bit bitmap'
previously sent.

Signed-off-by: Andrey Smetanin 
CC: Paolo Bonzini 
CC: Gleb Natapov 
CC: Roman Kagan 
CC: Denis V. Lunev 
CC: qemu-devel@nongnu.org

---
  arch/x86/kvm/hyperv.c| 117 +--
  arch/x86/kvm/hyperv.h|   2 +
  arch/x86/kvm/x86.c   |  12 +
  include/linux/kvm_host.h |   1 +
  4 files changed, 117 insertions(+), 15 deletions(-)

diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index d50675a..504fdc7 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -753,6 +753,105 @@ static int kvm_hv_msr_set_crash_data(struct kvm_vcpu 
*vcpu,
return 0;
  }
  
+static u64 calc_tsc_page_scale(u32 tsc_khz)

+{
+   /*
+* reftime (in 100ns) = tsc * tsc_scale / 2^64 + tsc_offset
+* so reftime_delta = (tsc_delta * tsc_scale) / 2^64
+* so tsc_scale = (2^64 * reftime_delta)/tsc_delta
+* so tsc_scale = (2^64 * 10 * 10^6) / tsc_hz = (2^64 * 1) / tsc_khz
+* so tsc_scale = (2^63 * 2 * 1) / tsc_khz
+*/
+   return mul_u64_u32_div(1ULL << 63, 2 * 1, tsc_khz);
+}
+
+static int write_tsc_page(struct kvm *kvm, u64 gfn,
+ PHV_REFERENCE_TSC_PAGE tsc_ref)
+{
+   if (kvm_write_guest(kvm, gfn_to_gpa(gfn),
+   tsc_ref, sizeof(*tsc_ref)))
+   return 1;
+   mark_page_dirty(kvm, gfn);
+   return 0;
+}
+
+static int read_tsc_page(struct kvm *kvm, u64 gfn,
+PHV_REFERENCE_TSC_PAGE tsc_ref)
+{
+   if (kvm_read_guest(kvm, gfn_to_gpa(gfn),
+  tsc_ref, sizeof(*tsc_ref)))
+   return 1;
+   return 0;
+}
+
+static u64 calc_tsc_page_time(struct kvm_vcpu *vcpu,
+ PHV_REFERENCE_TSC_PAGE tsc_ref)
+{
+
+   u64 tsc = kvm_read_l1_tsc(vcpu, rdtsc());
+
+   return mul_u64_u64_shr(tsc, tsc_ref->tsc_scale, 64)
+   + tsc_ref->tsc_offset;
+}
+
+static int setup_blank_tsc_page(struct kvm_vcpu *vcpu, u64 gfn)
+{
+   HV_REFERENCE_TSC_PAGE tsc_ref;
+
+   memset(&tsc_ref, 0, sizeof(tsc_ref));
+   return write_tsc_page(vcpu->kvm, gfn, &tsc_ref);
+}
+
+int kvm_hv_setup_tsc_page(struct kvm_vcpu *vcpu)
+{
+   struct kvm *kvm = vcpu->kvm;
+   struct kvm_hv *hv = &kvm->arch.hyperv;
+   HV_REFERENCE_TSC_PAGE tsc_ref;
+   u32 tsc_khz;
+   int r;
+   u64 gfn, ref_time, tsc_scale, tsc_offset, tsc;
+
+   if (WARN_ON_ONCE(!(hv->hv_tsc_page & HV_X64_MSR_TSC_REFERENCE_ENABLE)))
+   return -EINVAL;
+
+   gfn = hv->hv_tsc_page >> HV_X64_MSR_TSC_REFERENCE_ADDRESS_SHIFT;
+   vcpu_debug(vcpu, "tsc page gfn 0x%llx\n", gfn);
+
+   tsc_khz = vcpu->arch.virtual_tsc_khz;
+   if (!tsc_khz) {
+   vcpu_unimpl(vcpu, "no tsc khz\n");
+   return setup_blank_tsc_page(vcpu, gfn);
+   }
+
+   r = read_tsc_page(kvm, gfn, &tsc_ref);
+   if (r) {
+   vcpu_err(vcpu, "can't access tsc page gfn 0x%llx\n", gfn);
+   return r;
+   }
+
+   tsc_scale = calc_tsc_page_scale(tsc_khz);
+   ref_time = get_time_ref_counter(kvm);
+   tsc = kvm_read_l1_tsc(vcpu, rdtsc());
+
+   /* tsc_offset = reftime - tsc * tsc_scale / 2^64 */
+   tsc_offset = ref_time - mul_u64_u64_shr(tsc, tsc_scale, 64);
+   vcpu_debug(vcpu, "tsc khz %u tsc %llu scale %llu offset %llu\n",
+  tsc_khz, tsc, tsc_scale, tsc_offset);
+
+   tsc_ref.tsc_sequence++;
+   if (tsc_ref.tsc_sequence == 0)
+   tsc_ref.tsc_sequence = 1;
+
+   tsc_ref.tsc_scale = tsc_scale;
+   tsc_ref.tsc_offset = tsc_offset;
+
+   vcpu_debug(vcpu, "tsc page calibration time %llu vs. reftime %llu\n",
+  calc_tsc_page_time(vcpu, &tsc_ref),
+  get_time_ref_counter(kvm));
+
+   return write_tsc_page(kvm, gfn, &tsc_ref);
+}
+
  static int kvm_hv_set_msr_pw(struct kvm_vcpu *vcpu, u32 msr, u64 data,
 bool host)
  {
@@ -790,23 +889,11 @@ static int kvm_hv_set_msr_pw(struct kvm_vcpu *vcpu, u32 
msr, u64 data,
mark_page_dirty(kvm, gfn);
break;
}
-   case HV_X64_MSR_REFERENCE_TSC: {
-   u64 gfn;
-   HV_REFERENCE_TSC_PAGE tsc_ref;
-
-   memset(&tsc_ref, 0, sizeof(tsc_ref));
+   case HV_X64_MSR_REFERENCE_TSC:
hv->hv_tsc_page = data;
-   if (!(data & HV_X64_MSR_TSC_REFERENCE_ENABLE))
-   break;
-   gfn = data >> HV_X64_MSR_TSC_REFERENCE_ADDRES

Re: [Qemu-devel] [PATCHv4 8/8] pseries: Clean up error reporting in htab migration functions

[Markus Armbruster]

David Gibson  writes:

> The functions for migrating the hash page table on pseries machine type
> (htab_save_setup() and htab_load()) can report some errors with an
> explicit fprintf() before returning an appropriate error code.  Change these
> to use error_report() instead.
>
> Signed-off-by: David Gibson 
> Reviewed-by: Thomas Huth 
> ---
>  hw/ppc/spapr.c | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 3cfacb9..1eb7d03 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c

Lost this hunk:

  @@ -1309,8 +1309,9 @@ static int htab_save_setup(QEMUFile *f, void *opaque)
   spapr->htab_fd = kvmppc_get_htab_fd(false);
   spapr->htab_fd_stale = false;
   if (spapr->htab_fd < 0) {
  -fprintf(stderr, "Unable to open fd for reading hash table from 
KVM: %s\n",
  -strerror(errno));
  +error_report(
  +"Unable to open fd for reading hash table from KVM: %s",
  +strerror(errno));
   return -1;
   }
   }

Intentional?

> @@ -1526,7 +1526,7 @@ static int htab_load(QEMUFile *f, void *opaque, int 
> version_id)
>  int fd = -1;
>  
>  if (version_id < 1 || version_id > 1) {
> -fprintf(stderr, "htab_load() bad version\n");
> +error_report("htab_load() bad version");
>  return -EINVAL;
>  }
>  
> @@ -1547,8 +1547,8 @@ static int htab_load(QEMUFile *f, void *opaque, int 
> version_id)
>  
>  fd = kvmppc_get_htab_fd(true);
>  if (fd < 0) {
> -fprintf(stderr, "Unable to open fd to restore KVM hash table: 
> %s\n",
> -strerror(errno));
> +error_report("Unable to open fd to restore KVM hash table: %s",
> + strerror(errno));
>  }
>  }
>  
> @@ -1568,9 +1568,9 @@ static int htab_load(QEMUFile *f, void *opaque, int 
> version_id)
>  if ((index + n_valid + n_invalid) >
>  (HTAB_SIZE(spapr) / HASH_PTE_SIZE_64)) {
>  /* Bad index in stream */
> -fprintf(stderr, "htab_load() bad index %d (%hd+%hd entries) "
> -"in htab stream (htab_shift=%d)\n", index, n_valid, 
> n_invalid,
> -spapr->htab_shift);
> +error_report(
> +"htab_load() bad index %d (%hd+%hd entries) in htab stream 
> (htab_shift=%d)",
> +index, n_valid, n_invalid, spapr->htab_shift);
>  return -EINVAL;
>  }

Re: [Qemu-devel] [PATCH v2 0/4] set the OEM fields in the RSDT and the FADT from the SLIC

[Steven Newbury]

On Tue Jan 19 07:35:27 2016 GMT, Steven Newbury wrote:
> 
> 
> On Mon Jan 18 14:12:09 2016 GMT, Laszlo Ersek wrote:
> > This is version 2 of
> > .
> > 
> > Changes in v2 (also annotated per patch):
> > - introduce acpi_get_slic_oem() [MST],
> > - drop machine property that disables the SLIC-based override [MST].
> >
> Series looks good to me.  This is much cleaner (less hacky) than the patch I 
> posted. I started out with a similar approach but felt I was being too 
> invasive, so decided to keep it simple.   

For what it's worth:

Reviewed-by: Steven Newbury 

> > Cc: "Michael S. Tsirkin" 
> > Cc: Aleksei Kovura 
> > Cc: Igor Mammedov 
> > Cc: Michael Tokarev 
> > Cc: Paolo Bonzini 
> > Cc: Richard W.M. Jones 
> > Cc: Shannon Zhao 
> > Cc: Steven Newbury 
> > Cc: Xiao Guangrong 
> > 
> > Thanks
> > Laszlo
> > 
> > Laszlo Ersek (4):
> >   acpi: take oem_id in build_header(), optionally
> >   acpi: expose oem_id and oem_table_id in build_rsdt()
> >   acpi: add function to extract oem_id and oem_table_id from the user's
> > SLIC
> >   pc: set the OEM fields in the RSDT and the FADT from the SLIC
> > 
> >  include/hw/acpi/acpi.h  |  7 +++
> >  include/hw/acpi/aml-build.h |  5 +++--
> >  hw/acpi/aml-build.c | 14 ++
> >  hw/acpi/core.c  | 16 
> >  hw/acpi/nvdimm.c|  4 ++--
> >  hw/arm/virt-acpi-build.c| 14 +++---
> >  hw/i386/acpi-build.c| 31 ++-
> >  qemu-options.hx |  4 
> >  8 files changed, 67 insertions(+), 28 deletions(-)
> > 
> > -- 
> > 1.8.3.1
> > 
> >
> 
> -- 
> Sent from my Joll

-- 
Sent from my Jolla

Re: [Qemu-devel] [PATCH v2 0/4] set the OEM fields in the RSDT and the FADT from the SLIC

[Steven Newbury]

On Mon Jan 18 14:12:09 2016 GMT, Laszlo Ersek wrote:
> This is version 2 of
> .
> 
> Changes in v2 (also annotated per patch):
> - introduce acpi_get_slic_oem() [MST],
> - drop machine property that disables the SLIC-based override [MST].
>
Series looks good to me.  This is much cleaner (less hacky) than the patch I 
posted. I started out with a similar approach but felt I was being too 
invasive, so decided to keep it simple.   
> Cc: "Michael S. Tsirkin" 
> Cc: Aleksei Kovura 
> Cc: Igor Mammedov 
> Cc: Michael Tokarev 
> Cc: Paolo Bonzini 
> Cc: Richard W.M. Jones 
> Cc: Shannon Zhao 
> Cc: Steven Newbury 
> Cc: Xiao Guangrong 
> 
> Thanks
> Laszlo
> 
> Laszlo Ersek (4):
>   acpi: take oem_id in build_header(), optionally
>   acpi: expose oem_id and oem_table_id in build_rsdt()
>   acpi: add function to extract oem_id and oem_table_id from the user's
> SLIC
>   pc: set the OEM fields in the RSDT and the FADT from the SLIC
> 
>  include/hw/acpi/acpi.h  |  7 +++
>  include/hw/acpi/aml-build.h |  5 +++--
>  hw/acpi/aml-build.c | 14 ++
>  hw/acpi/core.c  | 16 
>  hw/acpi/nvdimm.c|  4 ++--
>  hw/arm/virt-acpi-build.c| 14 +++---
>  hw/i386/acpi-build.c| 31 ++-
>  qemu-options.hx |  4 
>  8 files changed, 67 insertions(+), 28 deletions(-)
> 
> -- 
> 1.8.3.1
> 
>

-- 
Sent from my Jolla

Re: [Qemu-devel] [PATCH] trace: drop trailing empty strings

[Markus Armbruster]

Greg Kurz  writes:

> Also fix a typo in the virtio_balloon_handle_output() trace while here.
>
> Signed-off-by: Greg Kurz 

Reviewed-by: Markus Armbruster 

Re: [Qemu-devel] [PATCH 0/3] clean-includes script to add osdep.h to everything

[Markus Armbruster]

Peter Maydell  writes:

> On 11 January 2016 at 15:19, Daniel P. Berrange  wrote:
>> On Mon, Dec 07, 2015 at 04:23:42PM +, Peter Maydell wrote:
>>> We've had some discussion previously (on list and IRC) about adding an
>>> include of "qemu/osdep.h" to everything. The basic idea is that every
>>> .c file should include "qemu/osdep.h" as its first include; then every
>>> other header (and the .c file itself) can rely on the facilities that
>>> osdep.h provides.
>>>
>>> This patchset is mostly here to get comment and review on the script
>>> I've written to do the job of automatically updating the source files.
>
> In the absence of any other comments from anybody I guess we can
> go ahead and commit this series... (the osdep.h patch it depends
> on has been committed already).
>
>>> Patches 2 and 3 are examples of its output, produced via
>>>  scripts/clean-includes --git target-arm target-arm/*.c
>>>  scripts/clean-includes --git hw/arm hw/arm/*.c
>>>
>>> NB: the script assumes my patch to make osdep.h include
>>> glib-compat.h has already been applied:
>>>   http://patchwork.ozlabs.org/patch/552828/
>>>
>>> Once we're happy with the set of transformations it produces the
>>> next question is how we want to apply it to the tree. The good
>>> news is that the changes to the .c files are idempotent and don't
>>> depend on each other, so we could send things via different
>>> submaintainer trees. Or we could have a single patchseries which we
>>> apply all at once on the theory that this minimises the pain overall.
>>
>> I think either approach would work as long as we don't let it drag
>> out too long in sub-maintainer trees. ie aim to get every file
>> cleaned & merged before 2.6 rc.
>
> I guess I'll start off with some series for the obviously maintained
> areas of the tree and then we can mop up the rest later.
>
> (At some point a script which identifies files which haven't been
> cleaned up yet would be handy.)

Do what you think is best.  I'd go for a single patch fixing up the
complete tree.

>>> A question I had about including osdep.h everywhere:
>>> are there any files in the tree where we *can't* include it?
>>> (Obvious possible candidates would be standalone test programs
>>> and the guest-agent code.)
>>
>> I think even guest-agent code & tests could include it in order to
>> get clean includes, even if they don't use any of the QEMU functions
>> defined in it. So I think its simplest to just say every .c file must
>> use it and leave it at that.
>
> OK, let's assume that works.

If it doesn't, we need a header with just configuration results that is
included in every .c file first.  Just like config.h should be when
using autoconf.

[Qemu-devel] [PATCH v3 7/7] MAINTAINERS: Add Alistair to the maintainers list

[Alistair Francis]

Add Alistair Francis as the maintainer for the Netduino 2
and SMM32F205 SoC.

Signed-off-by: Alistair Francis 
Reviewed-by: Peter Crosthwaite 
---

 MAINTAINERS | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 4030e27..24aaef4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -457,6 +457,21 @@ S: Maintained
 F: hw/arm/virt-acpi-build.c
 F: include/hw/arm/virt-acpi-build.h
 
+STM32F205
+M: Alistair Francis 
+S: Maintained
+F: hw/arm/stm32f205_soc.c
+F: hw/misc/stm32f2xx_syscfg.c
+F: hw/char/stm32f2xx_usart.c
+F: hw/timer/stm32f2xx_timer.c
+F: hw/adc/*
+F: hw/ssi/stm32f2xx_spi.c
+
+Netduino 2
+M: Alistair Francis 
+S: Maintained
+F: hw/arm/netduino2.c
+
 CRIS Machines
 -
 Axis Dev88
-- 
2.5.0

[Qemu-devel] [PATCH v3 5/7] STM32F205: Connect the ADC devices

[Alistair Francis]

Connect the ADC devices to the STM32F205 SoC.

Signed-off-by: Alistair Francis 
---
V2:
 - Fix up the device/devices commit message

 hw/arm/stm32f205_soc.c | 22 ++
 include/hw/arm/stm32f205_soc.h |  3 +++
 2 files changed, 25 insertions(+)

diff --git a/hw/arm/stm32f205_soc.c b/hw/arm/stm32f205_soc.c
index a2bd970..28d4301 100644
--- a/hw/arm/stm32f205_soc.c
+++ b/hw/arm/stm32f205_soc.c
@@ -32,9 +32,12 @@ static const uint32_t timer_addr[STM_NUM_TIMERS] = { 
0x4000, 0x4400,
 0x4800, 0x4C00 };
 static const uint32_t usart_addr[STM_NUM_USARTS] = { 0x40011000, 0x40004400,
 0x40004800, 0x40004C00, 0x40005000, 0x40011400 };
+static const uint32_t adc_addr[STM_NUM_ADCS] = { 0x40012000, 0x40012100,
+0x40012200 };
 
 static const int timer_irq[STM_NUM_TIMERS] = {28, 29, 30, 50};
 static const int usart_irq[STM_NUM_USARTS] = {37, 38, 39, 52, 53, 71};
+#define ADC_IRQ 18
 
 static void stm32f205_soc_initfn(Object *obj)
 {
@@ -55,6 +58,12 @@ static void stm32f205_soc_initfn(Object *obj)
   TYPE_STM32F2XX_TIMER);
 qdev_set_parent_bus(DEVICE(&s->timer[i]), sysbus_get_default());
 }
+
+for (i = 0; i < STM_NUM_ADCS; i++) {
+object_initialize(&s->adc[i], sizeof(s->adc[i]),
+  TYPE_STM32F2XX_ADC);
+qdev_set_parent_bus(DEVICE(&s->adc[i]), sysbus_get_default());
+}
 }
 
 static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
@@ -128,6 +137,19 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, 
Error **errp)
 sysbus_mmio_map(busdev, 0, timer_addr[i]);
 sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(nvic, timer_irq[i]));
 }
+
+/* ADC 1 to 3 */
+for (i = 0; i < STM_NUM_ADCS; i++) {
+dev = DEVICE(&(s->adc[i]));
+object_property_set_bool(OBJECT(&s->adc[i]), true, "realized", &err);
+if (err != NULL) {
+error_propagate(errp, err);
+return;
+}
+busdev = SYS_BUS_DEVICE(dev);
+sysbus_mmio_map(busdev, 0, adc_addr[i]);
+sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(nvic, ADC_IRQ));
+}
 }
 
 static Property stm32f205_soc_properties[] = {
diff --git a/include/hw/arm/stm32f205_soc.h b/include/hw/arm/stm32f205_soc.h
index 0390eff..091e4be 100644
--- a/include/hw/arm/stm32f205_soc.h
+++ b/include/hw/arm/stm32f205_soc.h
@@ -28,6 +28,7 @@
 #include "hw/misc/stm32f2xx_syscfg.h"
 #include "hw/timer/stm32f2xx_timer.h"
 #include "hw/char/stm32f2xx_usart.h"
+#include "hw/adc/stm32f2xx_adc.h"
 
 #define TYPE_STM32F205_SOC "stm32f205-soc"
 #define STM32F205_SOC(obj) \
@@ -35,6 +36,7 @@
 
 #define STM_NUM_USARTS 6
 #define STM_NUM_TIMERS 4
+#define STM_NUM_ADCS 3
 
 #define FLASH_BASE_ADDRESS 0x0800
 #define FLASH_SIZE (1024 * 1024)
@@ -52,6 +54,7 @@ typedef struct STM32F205State {
 STM32F2XXSyscfgState syscfg;
 STM32F2XXUsartState usart[STM_NUM_USARTS];
 STM32F2XXTimerState timer[STM_NUM_TIMERS];
+STM32F2XXADCState adc[STM_NUM_ADCS];
 } STM32F205State;
 
 #endif
-- 
2.5.0

[Qemu-devel] [PATCH v3 3/7] STM32F2xx: Add the ADC device

[Alistair Francis]

Add the STM32F2xx ADC device. This device randomly
generates values on each read.

This also includes creating a hw/adc directory.

Signed-off-by: Alistair Francis 
---
V2:
 - Address Peter C's comments
 - Create a ADC folder and move the file in there
 - Move some of the registers into arrays

 default-configs/arm-softmmu.mak |   1 +
 hw/Makefile.objs|   1 +
 hw/adc/Makefile.objs|   1 +
 hw/adc/stm32f2xx_adc.c  | 283 
 include/hw/adc/stm32f2xx_adc.h  |  94 +
 5 files changed, 380 insertions(+)
 create mode 100644 hw/adc/Makefile.objs
 create mode 100644 hw/adc/stm32f2xx_adc.c
 create mode 100644 include/hw/adc/stm32f2xx_adc.h

diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index d9b90a5..520b209 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -85,6 +85,7 @@ CONFIG_ZYNQ=y
 CONFIG_STM32F2XX_TIMER=y
 CONFIG_STM32F2XX_USART=y
 CONFIG_STM32F2XX_SYSCFG=y
+CONFIG_STM32F2XX_ADC=y
 CONFIG_STM32F205_SOC=y
 
 CONFIG_VERSATILE_PCI=y
diff --git a/hw/Makefile.objs b/hw/Makefile.objs
index 4a07ed4..0ffd281 100644
--- a/hw/Makefile.objs
+++ b/hw/Makefile.objs
@@ -1,5 +1,6 @@
 devices-dirs-$(call land, $(CONFIG_VIRTIO),$(call 
land,$(CONFIG_VIRTFS),$(CONFIG_PCI))) += 9pfs/
 devices-dirs-$(CONFIG_ACPI) += acpi/
+devices-dirs-$(CONFIG_SOFTMMU) += adc/
 devices-dirs-$(CONFIG_SOFTMMU) += audio/
 devices-dirs-$(CONFIG_SOFTMMU) += block/
 devices-dirs-$(CONFIG_SOFTMMU) += bt/
diff --git a/hw/adc/Makefile.objs b/hw/adc/Makefile.objs
new file mode 100644
index 000..3f6dfde
--- /dev/null
+++ b/hw/adc/Makefile.objs
@@ -0,0 +1 @@
+obj-$(CONFIG_STM32F2XX_ADC) += stm32f2xx_adc.o
diff --git a/hw/adc/stm32f2xx_adc.c b/hw/adc/stm32f2xx_adc.c
new file mode 100644
index 000..f7249b7
--- /dev/null
+++ b/hw/adc/stm32f2xx_adc.c
@@ -0,0 +1,283 @@
+/*
+ * STM32F2XX ADC
+ *
+ * Copyright (c) 2014 Alistair Francis 
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "hw/sysbus.h"
+#include "hw/hw.h"
+#include "hw/adc/stm32f2xx_adc.h"
+
+#ifndef STM_ADC_ERR_DEBUG
+#define STM_ADC_ERR_DEBUG 0
+#endif
+
+#define DB_PRINT_L(lvl, fmt, args...) do { \
+if (STM_ADC_ERR_DEBUG >= lvl) { \
+qemu_log("%s: " fmt, __func__, ## args); \
+} \
+} while (0);
+
+#define DB_PRINT(fmt, args...) DB_PRINT_L(1, fmt, ## args)
+
+static void stm32f2xx_adc_reset(DeviceState *dev)
+{
+STM32F2XXADCState *s = STM32F2XX_ADC(dev);
+
+s->adc_sr = 0x;
+s->adc_cr1 = 0x;
+s->adc_cr2 = 0x;
+s->adc_smpr1 = 0x;
+s->adc_smpr2 = 0x;
+s->adc_jofr[0] = 0x;
+s->adc_jofr[1] = 0x;
+s->adc_jofr[2] = 0x;
+s->adc_jofr[3] = 0x;
+s->adc_htr = 0x0FFF;
+s->adc_ltr = 0x;
+s->adc_sqr1 = 0x;
+s->adc_sqr2 = 0x;
+s->adc_sqr3 = 0x;
+s->adc_jsqr = 0x;
+s->adc_jdr[0] = 0x;
+s->adc_jdr[1] = 0x;
+s->adc_jdr[2] = 0x;
+s->adc_jdr[3] = 0x;
+s->adc_dr = 0x;
+}
+
+static uint32_t stm32f2xx_adc_generate_value(STM32F2XXADCState *s)
+{
+/* Attempts to fake some ADC values */
+#ifdef RAND_AVALIABLE
+s->adc_dr = s->adc_dr + rand();
+#else
+s->adc_dr = s->adc_dr + 7;
+#endif
+
+switch ((s->adc_cr1 & ADC_CR1_RES) >> 24) {
+case 0:
+/* 12-bit */
+s->adc_dr &= 0xFFF;
+break;
+case 1:
+/* 10-bit */
+s->adc_dr &= 0x3FF;
+break;
+case 2:
+/* 8-bit */
+s->adc_dr &= 0xFF;
+break;
+default:
+/* 6-bit */
+s->adc_dr &= 0x3F;
+}
+
+if (s->adc_cr2 & ADC_CR2_ALIGN) {
+return (s->adc_dr << 1) & 0xFFF0;
+} else {
+return s->adc_dr;
+}
+}
+
+static uint64_t stm32f2xx_adc_read(void *opaque, hwaddr addr,
+ 

[Qemu-devel] [PATCH v3 4/7] STM32F2xx: Add the SPI device

[Alistair Francis]

Add the STM32F2xx SPI device.

Signed-off-by: Alistair Francis 
---
V2:
 - Address Peter C's comments

 default-configs/arm-softmmu.mak |   1 +
 hw/ssi/Makefile.objs|   1 +
 hw/ssi/stm32f2xx_spi.c  | 205 
 include/hw/ssi/stm32f2xx_spi.h  |  72 ++
 4 files changed, 279 insertions(+)
 create mode 100644 hw/ssi/stm32f2xx_spi.c
 create mode 100644 include/hw/ssi/stm32f2xx_spi.h

diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index 520b209..af43eb1 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -86,6 +86,7 @@ CONFIG_STM32F2XX_TIMER=y
 CONFIG_STM32F2XX_USART=y
 CONFIG_STM32F2XX_SYSCFG=y
 CONFIG_STM32F2XX_ADC=y
+CONFIG_STM32F2XX_SPI=y
 CONFIG_STM32F205_SOC=y
 
 CONFIG_VERSATILE_PCI=y
diff --git a/hw/ssi/Makefile.objs b/hw/ssi/Makefile.objs
index 9555825..c674247 100644
--- a/hw/ssi/Makefile.objs
+++ b/hw/ssi/Makefile.objs
@@ -2,5 +2,6 @@ common-obj-$(CONFIG_PL022) += pl022.o
 common-obj-$(CONFIG_SSI) += ssi.o
 common-obj-$(CONFIG_XILINX_SPI) += xilinx_spi.o
 common-obj-$(CONFIG_XILINX_SPIPS) += xilinx_spips.o
+common-obj-$(CONFIG_STM32F2XX_SPI) += stm32f2xx_spi.o
 
 obj-$(CONFIG_OMAP) += omap_spi.o
diff --git a/hw/ssi/stm32f2xx_spi.c b/hw/ssi/stm32f2xx_spi.c
new file mode 100644
index 000..1ae88d7
--- /dev/null
+++ b/hw/ssi/stm32f2xx_spi.c
@@ -0,0 +1,205 @@
+/*
+ * STM32F405 SPI
+ *
+ * Copyright (c) 2014 Alistair Francis 
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "hw/ssi/stm32f2xx_spi.h"
+
+#ifndef STM_SPI_ERR_DEBUG
+#define STM_SPI_ERR_DEBUG 0
+#endif
+
+#define DB_PRINT_L(lvl, fmt, args...) do { \
+if (STM_SPI_ERR_DEBUG >= lvl) { \
+qemu_log("%s: " fmt, __func__, ## args); \
+} \
+} while (0);
+
+#define DB_PRINT(fmt, args...) DB_PRINT_L(1, fmt, ## args)
+
+static void stm32f2xx_spi_reset(DeviceState *dev)
+{
+STM32F2XXSPIState *s = STM32F2XX_SPI(dev);
+
+s->spi_cr1 = 0x;
+s->spi_cr2 = 0x;
+s->spi_sr = 0x000A;
+s->spi_dr = 0x000C;
+s->spi_crcpr = 0x0007;
+s->spi_rxcrcr = 0x;
+s->spi_txcrcr = 0x;
+s->spi_i2scfgr = 0x;
+s->spi_i2spr = 0x0002;
+}
+
+static void stm32f2xx_spi_transfer(STM32F2XXSPIState *s)
+{
+DB_PRINT("Data to send: 0x%x\n", s->spi_dr);
+
+s->spi_dr = ssi_transfer(s->ssi, s->spi_dr);
+s->spi_sr |= STM_SPI_SR_RXNE;
+
+DB_PRINT("Data received: 0x%x\n", s->spi_dr);
+}
+
+static uint64_t stm32f2xx_spi_read(void *opaque, hwaddr addr,
+ unsigned int size)
+{
+STM32F2XXSPIState *s = opaque;
+uint32_t retval;
+
+DB_PRINT("Address: 0x%"HWADDR_PRIx"\n", addr);
+
+switch (addr) {
+case STM_SPI_CR1:
+return s->spi_cr1;
+case STM_SPI_CR2:
+qemu_log_mask(LOG_UNIMP, "%s: Interrupts and DMA are not 
implemented\n",
+  __func__);
+return s->spi_cr2;
+case STM_SPI_SR:
+retval = s->spi_sr;
+return retval;
+case STM_SPI_DR:
+stm32f2xx_spi_transfer(s);
+s->spi_sr &= ~STM_SPI_SR_RXNE;
+return s->spi_dr;
+case STM_SPI_CRCPR:
+qemu_log_mask(LOG_UNIMP, "%s: CRC is not implemented, the registers " \
+  "are included for compatability\n", __func__);
+return s->spi_crcpr;
+case STM_SPI_RXCRCR:
+qemu_log_mask(LOG_UNIMP, "%s: CRC is not implemented, the registers " \
+  "are included for compatability\n", __func__);
+return s->spi_rxcrcr;
+case STM_SPI_TXCRCR:
+qemu_log_mask(LOG_UNIMP, "%s: CRC is not implemented, the registers " \
+  "are included for compatability\n", __func__);
+return s->spi_txcrcr;
+case STM_SPI_I2SCFGR:
+qemu_log_mask(LOG_UNIMP, "%s: I2S is not implemented, the registers " \
+  "are inc

[Qemu-devel] [PATCH v3 0/7] Update the Netduino 2 Machine

[Alistair Francis]

This patchset continues with the Netduino 2 and STM32F205 SoC
work.

This patch series makes a small change to the STM32F2xx
SoC to tidy up the code.

Next a feature is added to the STM32F2xx timer to display the
PWM duty cycle, when debugging is enabled.

Then the STM32F2xx SPI and ADC devices are added and connected
to the STM32F205 SoC.

Finally the maintainers file is updated to add myself as the
maintainer for the Netdunio 2 and STM32F2xx.

V3:
 - Rebase
V2:
 - Update based on Peter C's coments
 - Rebase
 - Create an ADC folder for the ADC device


Alistair Francis (7):
  STM32F205: Remove the individual device variables
  STM32F2xx: Display PWM duty cycle from timer
  STM32F2xx: Add the ADC device
  STM32F2xx: Add the SPI device
  STM32F205: Connect the ADC devices
  STM32F205: Connect the SPI devices
  MAINTAINERS: Add Alistair to the maintainers list

 MAINTAINERS |  15 +++
 default-configs/arm-softmmu.mak |   2 +
 hw/Makefile.objs|   1 +
 hw/adc/Makefile.objs|   1 +
 hw/adc/stm32f2xx_adc.c  | 283 
 hw/arm/stm32f205_soc.c  |  76 ---
 hw/ssi/Makefile.objs|   1 +
 hw/ssi/stm32f2xx_spi.c  | 205 +
 hw/timer/stm32f2xx_timer.c  |   9 ++
 include/hw/adc/stm32f2xx_adc.h  |  94 +
 include/hw/arm/stm32f205_soc.h  |   6 +
 include/hw/ssi/stm32f2xx_spi.h  |  72 ++
 12 files changed, 748 insertions(+), 17 deletions(-)
 create mode 100644 hw/adc/Makefile.objs
 create mode 100644 hw/adc/stm32f2xx_adc.c
 create mode 100644 hw/ssi/stm32f2xx_spi.c
 create mode 100644 include/hw/adc/stm32f2xx_adc.h
 create mode 100644 include/hw/ssi/stm32f2xx_spi.h

-- 
2.5.0

[Qemu-devel] [PATCH v3 1/7] STM32F205: Remove the individual device variables

[Alistair Francis]

Cleanup the individual DeviceState and SysBusDevice
variables to re-use the same variable for each
device.

Signed-off-by: Alistair Francis 
Reviewed-by: Peter Crosthwaite 
---

 hw/arm/stm32f205_soc.c | 32 +++-
 1 file changed, 15 insertions(+), 17 deletions(-)

diff --git a/hw/arm/stm32f205_soc.c b/hw/arm/stm32f205_soc.c
index 79bfe6d..a2bd970 100644
--- a/hw/arm/stm32f205_soc.c
+++ b/hw/arm/stm32f205_soc.c
@@ -60,8 +60,8 @@ static void stm32f205_soc_initfn(Object *obj)
 static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
 {
 STM32F205State *s = STM32F205_SOC(dev_soc);
-DeviceState *syscfgdev, *usartdev, *timerdev, *nvic;
-SysBusDevice *syscfgbusdev, *usartbusdev, *timerbusdev;
+DeviceState *dev, *nvic;
+SysBusDevice *busdev;
 Error *err = NULL;
 int i;
 
@@ -92,43 +92,41 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, 
Error **errp)
s->kernel_filename, s->cpu_model);
 
 /* System configuration controller */
-syscfgdev = DEVICE(&s->syscfg);
+dev = DEVICE(&s->syscfg);
 object_property_set_bool(OBJECT(&s->syscfg), true, "realized", &err);
 if (err != NULL) {
 error_propagate(errp, err);
 return;
 }
-syscfgbusdev = SYS_BUS_DEVICE(syscfgdev);
-sysbus_mmio_map(syscfgbusdev, 0, 0x40013800);
-sysbus_connect_irq(syscfgbusdev, 0, qdev_get_gpio_in(nvic, 71));
+busdev = SYS_BUS_DEVICE(dev);
+sysbus_mmio_map(busdev, 0, 0x40013800);
+sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(nvic, 71));
 
 /* Attach UART (uses USART registers) and USART controllers */
 for (i = 0; i < STM_NUM_USARTS; i++) {
-usartdev = DEVICE(&(s->usart[i]));
+dev = DEVICE(&(s->usart[i]));
 object_property_set_bool(OBJECT(&s->usart[i]), true, "realized", &err);
 if (err != NULL) {
 error_propagate(errp, err);
 return;
 }
-usartbusdev = SYS_BUS_DEVICE(usartdev);
-sysbus_mmio_map(usartbusdev, 0, usart_addr[i]);
-sysbus_connect_irq(usartbusdev, 0,
-   qdev_get_gpio_in(nvic, usart_irq[i]));
+busdev = SYS_BUS_DEVICE(dev);
+sysbus_mmio_map(busdev, 0, usart_addr[i]);
+sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(nvic, usart_irq[i]));
 }
 
 /* Timer 2 to 5 */
 for (i = 0; i < STM_NUM_TIMERS; i++) {
-timerdev = DEVICE(&(s->timer[i]));
-qdev_prop_set_uint64(timerdev, "clock-frequency", 10);
+dev = DEVICE(&(s->timer[i]));
+qdev_prop_set_uint64(dev, "clock-frequency", 10);
 object_property_set_bool(OBJECT(&s->timer[i]), true, "realized", &err);
 if (err != NULL) {
 error_propagate(errp, err);
 return;
 }
-timerbusdev = SYS_BUS_DEVICE(timerdev);
-sysbus_mmio_map(timerbusdev, 0, timer_addr[i]);
-sysbus_connect_irq(timerbusdev, 0,
-   qdev_get_gpio_in(nvic, timer_irq[i]));
+busdev = SYS_BUS_DEVICE(dev);
+sysbus_mmio_map(busdev, 0, timer_addr[i]);
+sysbus_connect_irq(busdev, 0, qdev_get_gpio_in(nvic, timer_irq[i]));
 }
 }
 
-- 
2.5.0

[Qemu-devel] [PATCH v3 2/7] STM32F2xx: Display PWM duty cycle from timer

[Alistair Francis]

If correctly configured allow the STM32F2xx timer to print
out the PWM duty cycle information.

Signed-off-by: Alistair Francis 
Reviewed-by: Peter Crosthwaite 
---
V3:
 - Use OR instead of + for masking
 - Improve clarity of print statement
V2:
 - Fix up if statement braces
 - Remove stm32f2xx_timer_set_alarm() call

 hw/timer/stm32f2xx_timer.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/hw/timer/stm32f2xx_timer.c b/hw/timer/stm32f2xx_timer.c
index ecadf9d..6e33a99 100644
--- a/hw/timer/stm32f2xx_timer.c
+++ b/hw/timer/stm32f2xx_timer.c
@@ -49,6 +49,15 @@ static void stm32f2xx_timer_interrupt(void *opaque)
 qemu_irq_pulse(s->irq);
 stm32f2xx_timer_set_alarm(s, s->hit_time);
 }
+
+if (s->tim_ccmr1 & (TIM_CCMR1_OC2M2 | TIM_CCMR1_OC2M1) &&
+!(s->tim_ccmr1 & TIM_CCMR1_OC2M0) &&
+s->tim_ccmr1 & TIM_CCMR1_OC2PE &&
+s->tim_ccer & TIM_CCER_CC2E) {
+/* PWM 2 - Mode 1 */
+DB_PRINT("PWM2 Duty Cycle: %d%%\n",
+s->tim_ccr2 / (100 * (s->tim_psc + 1)));
+}
 }
 
 static inline int64_t stm32f2xx_ns_to_ticks(STM32F2XXTimerState *s, int64_t t)
-- 
2.5.0

Re: [Qemu-devel] [PATCH v6 0/6] Xen PCI passthru: Convert to realize()

[Markus Armbruster]

Cao jin  writes:

> Seems I forgot to CC Eric, but I checked my command line history, I do
> include Eric...weird

Probably the broken duplicate suppression feature.  Eric, are you sure
the reduction in e-mail for you is worth the occasional confusion for
others?

Re: [Qemu-devel] [PATCH] .travis.yml: migrate to container builds

[Alex Bennée]

David Gibson  writes:

> On Fri, Jan 15, 2016 at 10:45:09AM +, Alex Bennée wrote:
>> This moves the Travis tests from their old legacy VM
>> infrastructure (which only seems to run 5-6 jobs at once) to their new
>> container based approach.
>>
>> The principle difference is there is no sudo in the containers so all
>> packages are installed using the apt add-on. This means one of the build
>> combinations can be dropped as it was only for checking the build with
>> additional packages.
>>
>> I've disabled the user-space tracing build until the dependant packages
>> go through the Travis package white-listing process.
>>
>> Signed-off-by: Alex Bennée 
>
> Tested-by: David Gibson 
>
> Is it safe to just drop that build combination, or should we instead
> be doing something else to test the build _without_ those extra
> packages?

Hmm, currently there doesn't seem to be a way to do this with the Travis
infrastructure as the packages are no longer part of a matrix.

>
>
> [snip]
>> @@ -86,10 +103,10 @@ matrix:
>>  - env: TARGETS=i386-softmmu,x86_64-softmmu
>> EXTRA_CONFIG="--enable-trace-backends=ftrace"
>>compiler: gcc
>> -- env: TARGETS=i386-softmmu,x86_64-softmmu
>> -  EXTRA_PKGS="liblttng-ust-dev liburcu-dev"
>> -  EXTRA_CONFIG="--enable-trace-backends=ust"
>> -  compiler: gcc
>> +# UST backend disabled until liblttng-ust-dev pkg white-listed
>> +#- env: TARGETS=i386-softmmu,x86_64-softmmu
>> +#   EXTRA_CONFIG="--enable-trace-backends=ust"
>> +#  compiler: gcc
>
> This comment is a bit confusing to me, since the apt addon package
> list already seems to include liblttng-ust-dev.

It's in the list but still awaiting whitelisting by Travis:

https://github.com/travis-ci/apt-package-whitelist/issues/2258

>
>>  - env: TARGETS=i386-softmmu,x86_64-softmmu
>> EXTRA_CONFIG="--enable-modules"
>>compiler: gcc


--
Alex Bennée

Re: [Qemu-devel] [PATCH v1 1/1] qom: Correct object_property_get_int() description

[Markus Armbruster]

Copying maintainer.  Always do that even for trivial patches.

Alistair Francis  writes:

> The description of object_property_get_int() stated that on an error
> it returns NULL. This is not the case and the function will return -1
> if an error occurs. Update the commented documentation accordingly.
>
> Reported-By: Christian Liebhardt 
> Signed-off-by: Christian Liebhardt 
> Signed-off-by: Alistair Francis 
> ---
>  include/qom/object.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/qom/object.h b/include/qom/object.h
> index 4509166..2848f5e 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -1107,7 +1107,7 @@ void object_property_set_int(Object *obj, int64_t value,
>   * @name: the name of the property
>   * @errp: returns an error if this function fails
>   *
> - * Returns: the value of the property, converted to an integer, or NULL if
> + * Returns: the value of the property, converted to an integer, or negative 
> if
>   * an error occurs (including when the property value is not an integer).
>   */
>  int64_t object_property_get_int(Object *obj, const char *name,

Re: [Qemu-devel] usb-storage assertions

[Gerd Hoffmann]

On Di, 2016-01-19 at 02:49 +0300, Andrey Korolyov wrote:
> On Mon, Jan 18, 2016 at 4:55 PM, Gerd Hoffmann  wrote:
> >   Hi,
> >
> >> > ok.  Had no trouble with freebsd, will go fetch netbsd images.  What
> >> > arch is this?  i386?  x86_64?
> >>
> >> i386 7.0 for the reference, but I`m sure that this wouldn`t matter in
> >> any way.
> >
> > 7.0 trace:
> 
> Whoops, sorry, should be 5.1/i386.

I'll check that too ...

>  On a 7.0 I observe same endless
> loop as you do.

... just wanted to start with that one.

[ trace snipped ]

> > So, to shutdown ehci netbsd clears the cmd register, then sets the reset
> > bit in the cmd register.  Fine.
> >
> > Then it goes read the status register, in a loop, forever.  No idea why,
> > and I also can't spot then place in the source code.  Hmm ...

/me was hoping anyone has an idea what is going on here.
Are you familiar with the netbsd kernel sources?

cheers,
  Gerd

Re: [Qemu-devel] [PATCH v6 00/15] qemu-img map: Allow driver to return file of the allocated block

[Fam Zheng]

On Fri, 01/08 10:07, Fam Zheng wrote:
> v6: Two more changes as suggested by Max:
> - Fix comments of BDRV_BLOCK_OFFSET_VALID and BDRV_BLOCK_DATA;
> - Really keep 102 working;
> - Add rev-by in patch 11.

Ping?

Re: [Qemu-devel] [PATCH] .travis.yml: migrate to container builds

[David Gibson]

On Fri, Jan 15, 2016 at 10:45:09AM +, Alex Bennée wrote:
> This moves the Travis tests from their old legacy VM
> infrastructure (which only seems to run 5-6 jobs at once) to their new
> container based approach.
> 
> The principle difference is there is no sudo in the containers so all
> packages are installed using the apt add-on. This means one of the build
> combinations can be dropped as it was only for checking the build with
> additional packages.
> 
> I've disabled the user-space tracing build until the dependant packages
> go through the Travis package white-listing process.
> 
> Signed-off-by: Alex Bennée 

Tested-by: David Gibson 

Is it safe to just drop that build combination, or should we instead
be doing something else to test the build _without_ those extra
packages?


[snip]
> @@ -86,10 +103,10 @@ matrix:
>  - env: TARGETS=i386-softmmu,x86_64-softmmu
> EXTRA_CONFIG="--enable-trace-backends=ftrace"
>compiler: gcc
> -- env: TARGETS=i386-softmmu,x86_64-softmmu
> -  EXTRA_PKGS="liblttng-ust-dev liburcu-dev"
> -  EXTRA_CONFIG="--enable-trace-backends=ust"
> -  compiler: gcc
> +# UST backend disabled until liblttng-ust-dev pkg white-listed
> +#- env: TARGETS=i386-softmmu,x86_64-softmmu
> +#   EXTRA_CONFIG="--enable-trace-backends=ust"
> +#  compiler: gcc

This comment is a bit confusing to me, since the apt addon package
list already seems to include liblttng-ust-dev.

>  - env: TARGETS=i386-softmmu,x86_64-softmmu
> EXTRA_CONFIG="--enable-modules"
>compiler: gcc

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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCHv2 1/3] spapr: Small fixes to rtas_ibm_get_system_parameter, remove rtas_st_buffer

[David Gibson]

On Tue, Jan 19, 2016 at 03:38:09PM +1100, Alexey Kardashevskiy wrote:
> On 01/19/2016 03:30 PM, David Gibson wrote:
> >rtas_st_buffer() appears in spapr.h as though it were a widely used helper,
> >but in fact it is only used for saving data in a format used by
> >rtas_ibm_get_system_parameter().  This changes it to a local helper more
> >specifically for that function.
> >
> >While we're there fix a couple of small defects in
> >rtas_ibm_get_system_parameter:
> >   - For the string value SPLPAR_CHARACTERISTICS, it wasn't including the
> > terminating \0 in the length which it should according to LoPAPR
> > 7.3.16.1
> >   - It now checks that the supplied buffer has at least enough space for
> > the length of the returned data, and returns an error if it does not.
> >
> >Signed-off-by: David Gibson 
> >---
> >  hw/ppc/spapr_rtas.c| 22 ++
> >  include/hw/ppc/spapr.h | 28 +---
> >  2 files changed, 27 insertions(+), 23 deletions(-)
> >
> >diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> >index 34b12a3..32cdd66 100644
> >--- a/hw/ppc/spapr_rtas.c
> >+++ b/hw/ppc/spapr_rtas.c
> >@@ -228,6 +228,20 @@ static void rtas_stop_self(PowerPCCPU *cpu, 
> >sPAPRMachineState *spapr,
> >  env->msr = 0;
> >  }
> >
> >+
> 
> 
> Nit: unneeded empty line. Besides that,

Ah, yes, fixed.

> Reviewed-by: Alexey Kardashevskiy 

Thanks, I've merged this series into ppc-for-2.6.

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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH] spapr: Don't create ibm, dynamic-reconfiguration-memory w/o DR LMBs

[David Gibson]

On Tue, Jan 19, 2016 at 10:09:21AM +0530, Bharata B Rao wrote:
> If guest doesn't have any dynamically reconfigurable (DR) logical memory
> blocks (LMB), then we shouldn't create ibm,dynamic-reconfiguration-memory
> device tree node.
> 
> Signed-off-by: Bharata B Rao 
> ---
> This applies against ppc-for-2.6 branch of David Gibson's tree.

Applied to ppc-for-2.6, thanks.

> 
>  hw/ppc/spapr.c | 9 -
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 50e5a26..86e5023 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -763,6 +763,13 @@ static int 
> spapr_populate_drconf_memory(sPAPRMachineState *spapr, void *fdt)
>  int nr_nodes = nb_numa_nodes ? nb_numa_nodes : 1;
>  
>  /*
> + * Don't create the node if there are no DR LMBs.
> + */
> +if (!nr_lmbs) {
> +return 0;
> +}
> +
> +/*
>   * Allocate enough buffer size to fit in ibm,dynamic-memory
>   * or ibm,associativity-lookup-arrays
>   */
> @@ -868,7 +875,7 @@ int spapr_h_cas_compose_response(sPAPRMachineState *spapr,
>  _FDT((spapr_fixup_cpu_dt(fdt, spapr)));
>  }
>  
> -/* Generate memory nodes or ibm,dynamic-reconfiguration-memory node */
> +/* Generate ibm,dynamic-reconfiguration-memory node if required */
>  if (memory_update && smc->dr_lmb_enabled) {
>  _FDT((spapr_populate_drconf_memory(spapr, fdt)));
>  }

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


signature.asc
Description: PGP signature

[Qemu-devel] [PATCH 3/5] ide: move buffered DMA cancel to core

[John Snow]

Buffered DMA cancellation was added to ATAPI devices and implemented
for the BMDMA HBA. Move the code over to common IDE code and allow
it to be used for any HBA.

Signed-off-by: John Snow 
---
 hw/ide/core.c | 45 +
 hw/ide/internal.h |  1 +
 hw/ide/pci.c  | 36 +---
 3 files changed, 47 insertions(+), 35 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 75486c2..5d81840 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -608,6 +608,51 @@ BlockAIOCB *ide_buffered_readv(IDEState *s, int64_t 
sector_num,
 return aioreq;
 }
 
+/**
+ * Cancel all pending DMA requests.
+ * Any buffered DMA requests are instantly canceled,
+ * but any pending unbuffered DMA requests must be waited on.
+ */
+void ide_cancel_dma_sync(IDEState *s)
+{
+IDEBufferedRequest *req;
+
+/* First invoke the callbacks of all buffered requests
+ * and flag those requests as orphaned. Ideally there
+ * are no unbuffered (Scatter Gather DMA Requests or
+ * write requests) pending and we can avoid to drain. */
+QLIST_FOREACH(req, &s->buffered_requests, list) {
+if (!req->orphaned) {
+#ifdef DEBUG_IDE
+printf("%s: invoking cb %p of buffered request %p with"
+   " -ECANCELED\n", __func__, req->original_cb, req);
+#endif
+req->original_cb(req->original_opaque, -ECANCELED);
+}
+req->orphaned = true;
+}
+
+/*
+ * We can't cancel Scatter Gather DMA in the middle of the
+ * operation or a partial (not full) DMA transfer would reach
+ * the storage so we wait for completion instead (we beahve
+ * like if the DMA was completed by the time the guest trying
+ * to cancel dma with bmdma_cmd_writeb with BM_CMD_START not
+ * set).
+ *
+ * In the future we'll be able to safely cancel the I/O if the
+ * whole DMA operation will be submitted to disk with a single
+ * aio operation with preadv/pwritev.
+ */
+if (s->bus->dma->aiocb) {
+#ifdef DEBUG_IDE
+printf("%s: draining all remaining requests", __func__);
+#endif
+blk_drain_all();
+assert(s->bus->dma->aiocb == NULL);
+}
+}
+
 static void ide_sector_read(IDEState *s);
 
 static void ide_sector_read_cb(void *opaque, int ret)
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 2d1e2d2..86bde26 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -586,6 +586,7 @@ BlockAIOCB *ide_issue_trim(BlockBackend *blk,
 BlockAIOCB *ide_buffered_readv(IDEState *s, int64_t sector_num,
QEMUIOVector *iov, int nb_sectors,
BlockCompletionFunc *cb, void *opaque);
+void ide_cancel_dma_sync(IDEState *s);
 
 /* hw/ide/atapi.c */
 void ide_atapi_cmd(IDEState *s);
diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index 37dbc29..6b780b8 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -233,41 +233,7 @@ void bmdma_cmd_writeb(BMDMAState *bm, uint32_t val)
 /* Ignore writes to SSBM if it keeps the old value */
 if ((val & BM_CMD_START) != (bm->cmd & BM_CMD_START)) {
 if (!(val & BM_CMD_START)) {
-/* First invoke the callbacks of all buffered requests
- * and flag those requests as orphaned. Ideally there
- * are no unbuffered (Scatter Gather DMA Requests or
- * write requests) pending and we can avoid to drain. */
-IDEBufferedRequest *req;
-IDEState *s = idebus_active_if(bm->bus);
-QLIST_FOREACH(req, &s->buffered_requests, list) {
-if (!req->orphaned) {
-#ifdef DEBUG_IDE
-printf("%s: invoking cb %p of buffered request %p with"
-   " -ECANCELED\n", __func__, req->original_cb, req);
-#endif
-req->original_cb(req->original_opaque, -ECANCELED);
-}
-req->orphaned = true;
-}
-/*
- * We can't cancel Scatter Gather DMA in the middle of the
- * operation or a partial (not full) DMA transfer would reach
- * the storage so we wait for completion instead (we beahve
- * like if the DMA was completed by the time the guest trying
- * to cancel dma with bmdma_cmd_writeb with BM_CMD_START not
- * set).
- *
- * In the future we'll be able to safely cancel the I/O if the
- * whole DMA operation will be submitted to disk with a single
- * aio operation with preadv/pwritev.
- */
-if (bm->bus->dma->aiocb) {
-#ifdef DEBUG_IDE
-printf("%s: draining all remaining requests", __func__);
-#endif
-blk_drain_all();
-assert(bm->bus->dma->aiocb == NULL);
-}
+ide_cancel_dma_sync(idebus_active_if(bm->bus));
 bm->status &= ~BM_STATUS_DMAING;
 } else {
 bm->cur_addr = bm->

[Qemu-devel] [PATCH 5/5] ide: fix device_reset to not ignore pending AIO

[John Snow]

Signed-off-by: John Snow 
---
 hw/ide/core.c | 27 +--
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 4b4bfe5..f728d43 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -502,7 +502,6 @@ void ide_transfer_stop(IDEState *s)
 ide_transfer_halt(s, ide_transfer_stop, true);
 }
 
-__attribute__((__unused__))
 static void ide_transfer_cancel(IDEState *s)
 {
 ide_transfer_halt(s, ide_transfer_cancel, false);
@@ -1295,6 +1294,23 @@ static bool cmd_nop(IDEState *s, uint8_t cmd)
 return true;
 }
 
+static bool cmd_device_reset(IDEState *s, uint8_t cmd)
+{
+/* Halt PIO (in the DRQ phase), then DMA */
+ide_transfer_cancel(s);
+ide_cancel_dma_sync(s);
+
+/* Reset any PIO commands, reset signature, etc */
+ide_reset(s);
+
+/* RESET: ATA8-ACS3 7.10.4 "Normal Outputs";
+ * ATA8-ACS3 Table 184 "Device Signatures for Normal Output" */
+s->status = 0x00;
+
+/* Do not overwrite status register */
+return false;
+}
+
 static bool cmd_data_set_management(IDEState *s, uint8_t cmd)
 {
 switch (s->feature) {
@@ -1611,15 +1627,6 @@ static bool cmd_exec_dev_diagnostic(IDEState *s, uint8_t 
cmd)
 return false;
 }
 
-static bool cmd_device_reset(IDEState *s, uint8_t cmd)
-{
-ide_set_signature(s);
-s->status = 0x00; /* NOTE: READY is _not_ set */
-s->error = 0x01;
-
-return false;
-}
-
 static bool cmd_packet(IDEState *s, uint8_t cmd)
 {
 /* overlapping commands not supported */
-- 
2.4.3

[Qemu-devel] [PATCH 0/5] ide: fix atapi software reset

[John Snow]

The ATAPI software reset function is implemented somewhat lackadaisically.

Firstly, it is valid only for ATAPI drives - not HDs. If a HD should
receive this command while BSY, it should be ignored like any other
command instead of aborted. A non-BSY HD is free to abort the command
in the usual fashion to indicate it doesn't understand or doesn't support
that command.

Second, for drives that should accept a software reset, they should not
"forget" about all pending AIO during the reset. Since a software reset
resets the DRQ and BSY flags, it is possible to 'stack' multiple
concurrent reads using DMA and alternately chaining software reset and
DMA reads. We mustn't reset BSY/DRQ until we are confident that we
have canceled existing AIO.

Third, the existing software reset routine does not perform a very
rigorous reset.

This series corrects this by:

(1) Correcting ide_exec_cmd to correctly ignore, not abort, software
reset commands for ide-hd devices that are busy executing a command.

(2) Improving the software reset routine to cancel buffered DMA, then
fall back to synchronously waiting for any pending DMA to finish
before returning, insuring that the reset completes sanely.

(3) Use existing reset routines to comprehensively reset the device.

Reported-by: Kevin Wolf 
Signed-off-by: John Snow 



For convenience, this branch is available at:
https://github.com/jnsnow/qemu.git branch ide-reset-fix
https://github.com/jnsnow/qemu/tree/ide-reset-fix

This version is tagged ide-reset-fix-v1:
https://github.com/jnsnow/qemu/releases/tag/ide-reset-fix-v1

John Snow (5):
  ide: Prohibit RESET on IDE drives
  ide: code motion
  ide: move buffered DMA cancel to core
  ide: Add silent DRQ cancellation
  ide: fix device_reset to not ignore pending AIO

 hw/ide/core.c | 214 +++---
 hw/ide/internal.h |   1 +
 hw/ide/pci.c  |  36 +
 3 files changed, 143 insertions(+), 108 deletions(-)

-- 
2.4.3

[Qemu-devel] [PATCH 2/5] ide: code motion

[John Snow]

Shuffle the reset function upwards.

Signed-off-by: John Snow 
---
 hw/ide/core.c | 116 +-
 1 file changed, 58 insertions(+), 58 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index ba33064..75486c2 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1174,6 +1174,64 @@ void ide_ioport_write(void *opaque, uint32_t addr, 
uint32_t val)
 }
 }
 
+static void ide_reset(IDEState *s)
+{
+#ifdef DEBUG_IDE
+printf("ide: reset\n");
+#endif
+
+if (s->pio_aiocb) {
+blk_aio_cancel(s->pio_aiocb);
+s->pio_aiocb = NULL;
+}
+
+if (s->drive_kind == IDE_CFATA)
+s->mult_sectors = 0;
+else
+s->mult_sectors = MAX_MULT_SECTORS;
+/* ide regs */
+s->feature = 0;
+s->error = 0;
+s->nsector = 0;
+s->sector = 0;
+s->lcyl = 0;
+s->hcyl = 0;
+
+/* lba48 */
+s->hob_feature = 0;
+s->hob_sector = 0;
+s->hob_nsector = 0;
+s->hob_lcyl = 0;
+s->hob_hcyl = 0;
+
+s->select = 0xa0;
+s->status = READY_STAT | SEEK_STAT;
+
+s->lba48 = 0;
+
+/* ATAPI specific */
+s->sense_key = 0;
+s->asc = 0;
+s->cdrom_changed = 0;
+s->packet_transfer_size = 0;
+s->elementary_transfer_size = 0;
+s->io_buffer_index = 0;
+s->cd_sector_size = 0;
+s->atapi_dma = 0;
+s->tray_locked = 0;
+s->tray_open = 0;
+/* ATA DMA state */
+s->io_buffer_size = 0;
+s->req_nb_sectors = 0;
+
+ide_set_signature(s);
+/* init the transfer handler so that 0x is returned on data
+   accesses */
+s->end_transfer_func = ide_dummy_transfer_stop;
+ide_dummy_transfer_stop(s);
+s->media_changed = 0;
+}
+
 static bool cmd_nop(IDEState *s, uint8_t cmd)
 {
 return true;
@@ -2181,64 +2239,6 @@ static void ide_dummy_transfer_stop(IDEState *s)
 s->io_buffer[3] = 0xff;
 }
 
-static void ide_reset(IDEState *s)
-{
-#ifdef DEBUG_IDE
-printf("ide: reset\n");
-#endif
-
-if (s->pio_aiocb) {
-blk_aio_cancel(s->pio_aiocb);
-s->pio_aiocb = NULL;
-}
-
-if (s->drive_kind == IDE_CFATA)
-s->mult_sectors = 0;
-else
-s->mult_sectors = MAX_MULT_SECTORS;
-/* ide regs */
-s->feature = 0;
-s->error = 0;
-s->nsector = 0;
-s->sector = 0;
-s->lcyl = 0;
-s->hcyl = 0;
-
-/* lba48 */
-s->hob_feature = 0;
-s->hob_sector = 0;
-s->hob_nsector = 0;
-s->hob_lcyl = 0;
-s->hob_hcyl = 0;
-
-s->select = 0xa0;
-s->status = READY_STAT | SEEK_STAT;
-
-s->lba48 = 0;
-
-/* ATAPI specific */
-s->sense_key = 0;
-s->asc = 0;
-s->cdrom_changed = 0;
-s->packet_transfer_size = 0;
-s->elementary_transfer_size = 0;
-s->io_buffer_index = 0;
-s->cd_sector_size = 0;
-s->atapi_dma = 0;
-s->tray_locked = 0;
-s->tray_open = 0;
-/* ATA DMA state */
-s->io_buffer_size = 0;
-s->req_nb_sectors = 0;
-
-ide_set_signature(s);
-/* init the transfer handler so that 0x is returned on data
-   accesses */
-s->end_transfer_func = ide_dummy_transfer_stop;
-ide_dummy_transfer_stop(s);
-s->media_changed = 0;
-}
-
 void ide_bus_reset(IDEBus *bus)
 {
 bus->unit = 0;
-- 
2.4.3

[Qemu-devel] [PATCH 1/5] ide: Prohibit RESET on IDE drives

[John Snow]

This command is meant for ATAPI devices only, prohibit acknowledging it with
a command aborted response when an IDE device is busy.

Signed-off-by: John Snow 
---
 hw/ide/core.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index da3baab..ba33064 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1876,9 +1876,12 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
 return;
 }
 
-/* Only DEVICE RESET is allowed while BSY or/and DRQ are set */
-if ((s->status & (BUSY_STAT|DRQ_STAT)) && val != WIN_DEVICE_RESET)
-return;
+/* Only RESET is allowed to an ATAPI device while BSY and/or DRQ are set. 
*/
+if (s->status & (BUSY_STAT|DRQ_STAT)) {
+if (!(val == WIN_DEVICE_RESET) && (s->drive_kind == IDE_CD)) {
+return;
+}
+}
 
 if (!ide_cmd_permitted(s, val)) {
 ide_abort_command(s);
-- 
2.4.3

[Qemu-devel] [PATCH 4/5] ide: Add silent DRQ cancellation

[John Snow]

Split apart the ide_transfer_stop function into two versions: one that
interrupts and one that doesn't. The one that doesn't can be used to
halt any PIO transfers that are in the DRQ phase. It will not halt
any PIO transfers that are currently in the process of buffering data
for the guest to read.

Signed-off-by: John Snow 
---
 hw/ide/core.c | 19 ---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 5d81840..4b4bfe5 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -486,13 +486,26 @@ static void ide_cmd_done(IDEState *s)
 }
 }
 
-void ide_transfer_stop(IDEState *s)
+static void ide_transfer_halt(IDEState *s, void(*etf)(IDEState *), bool notify)
 {
-s->end_transfer_func = ide_transfer_stop;
+s->end_transfer_func = etf;
 s->data_ptr = s->io_buffer;
 s->data_end = s->io_buffer;
 s->status &= ~DRQ_STAT;
-ide_cmd_done(s);
+if (notify) {
+ide_cmd_done(s);
+}
+}
+
+void ide_transfer_stop(IDEState *s)
+{
+ide_transfer_halt(s, ide_transfer_stop, true);
+}
+
+__attribute__((__unused__))
+static void ide_transfer_cancel(IDEState *s)
+{
+ide_transfer_halt(s, ide_transfer_cancel, false);
 }
 
 int64_t ide_get_sector(IDEState *s)
-- 
2.4.3

Re: [Qemu-devel] [PATCHv2 2/3] spapr: Remove rtas_st_buffer_direct()

[Alexey Kardashevskiy]

On 01/19/2016 03:30 PM, David Gibson wrote:

rtas_st_buffer_direct() is a not particularly useful wrapper around
cpu_physical_memory_write().  All the callers are in
rtas_ibm_configure_connector, where it's better handled by local helper.

Signed-off-by: David Gibson 


Reviewed-by: Alexey Kardashevskiy 


---
  hw/ppc/spapr_rtas.c| 17 ++---
  include/hw/ppc/spapr.h |  8 
  2 files changed, 10 insertions(+), 15 deletions(-)

diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index 32cdd66..96759be 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -506,6 +506,13 @@ out:
  #define CC_VAL_DATA_OFFSET ((CC_IDX_PROP_DATA_OFFSET + 1) * 4)
  #define CC_WA_LEN 4096

+static void configure_connector_st(target_ulong addr, target_ulong offset,
+   const void *buf, size_t len)
+{
+cpu_physical_memory_write(ppc64_phys_to_real(addr + offset),
+  buf, MIN(len, CC_WA_LEN - offset));
+}
+
  static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
   sPAPRMachineState *spapr,
   uint32_t token, uint32_t nargs,
@@ -571,8 +578,7 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
  /* provide the name of the next OF node */
  wa_offset = CC_VAL_DATA_OFFSET;
  rtas_st(wa_addr, CC_IDX_NODE_NAME_OFFSET, wa_offset);
-rtas_st_buffer_direct(wa_addr + wa_offset, CC_WA_LEN - wa_offset,
-  (uint8_t *)name, strlen(name) + 1);
+configure_connector_st(wa_addr, wa_offset, name, strlen(name) + 1);
  resp = SPAPR_DR_CC_RESPONSE_NEXT_CHILD;
  break;
  case FDT_END_NODE:
@@ -597,8 +603,7 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
  /* provide the name of the next OF property */
  wa_offset = CC_VAL_DATA_OFFSET;
  rtas_st(wa_addr, CC_IDX_PROP_NAME_OFFSET, wa_offset);
-rtas_st_buffer_direct(wa_addr + wa_offset, CC_WA_LEN - wa_offset,
-  (uint8_t *)name, strlen(name) + 1);
+configure_connector_st(wa_addr, wa_offset, name, strlen(name) + 1);

  /* provide the length and value of the OF property. data gets
   * placed immediately after NULL terminator of the OF property's
@@ -607,9 +612,7 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
  wa_offset += strlen(name) + 1,
  rtas_st(wa_addr, CC_IDX_PROP_LEN, prop_len);
  rtas_st(wa_addr, CC_IDX_PROP_DATA_OFFSET, wa_offset);
-rtas_st_buffer_direct(wa_addr + wa_offset, CC_WA_LEN - wa_offset,
-  (uint8_t *)((struct fdt_property 
*)prop)->data,
-  prop_len);
+configure_connector_st(wa_addr, wa_offset, prop->data, prop_len);
  resp = SPAPR_DR_CC_RESPONSE_NEXT_PROPERTY;
  break;
  case FDT_END:
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 1e10fc9..1f9e722 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -506,14 +506,6 @@ static inline void rtas_st(target_ulong phys, int n, 
uint32_t val)
  stl_be_phys(&address_space_memory, ppc64_phys_to_real(phys + 4*n), val);
  }

-static inline void rtas_st_buffer_direct(target_ulong phys,
- target_ulong phys_len,
- uint8_t *buffer, uint16_t buffer_len)
-{
-cpu_physical_memory_write(ppc64_phys_to_real(phys), buffer,
-  MIN(buffer_len, phys_len));
-}
-
  typedef void (*spapr_rtas_fn)(PowerPCCPU *cpu, sPAPRMachineState *sm,
uint32_t token,
uint32_t nargs, target_ulong args,




--
Alexey

[Qemu-devel] [PATCH] spapr: Don't create ibm, dynamic-reconfiguration-memory w/o DR LMBs

[Bharata B Rao]

If guest doesn't have any dynamically reconfigurable (DR) logical memory
blocks (LMB), then we shouldn't create ibm,dynamic-reconfiguration-memory
device tree node.

Signed-off-by: Bharata B Rao 
---
This applies against ppc-for-2.6 branch of David Gibson's tree.

 hw/ppc/spapr.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 50e5a26..86e5023 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -763,6 +763,13 @@ static int spapr_populate_drconf_memory(sPAPRMachineState 
*spapr, void *fdt)
 int nr_nodes = nb_numa_nodes ? nb_numa_nodes : 1;
 
 /*
+ * Don't create the node if there are no DR LMBs.
+ */
+if (!nr_lmbs) {
+return 0;
+}
+
+/*
  * Allocate enough buffer size to fit in ibm,dynamic-memory
  * or ibm,associativity-lookup-arrays
  */
@@ -868,7 +875,7 @@ int spapr_h_cas_compose_response(sPAPRMachineState *spapr,
 _FDT((spapr_fixup_cpu_dt(fdt, spapr)));
 }
 
-/* Generate memory nodes or ibm,dynamic-reconfiguration-memory node */
+/* Generate ibm,dynamic-reconfiguration-memory node if required */
 if (memory_update && smc->dr_lmb_enabled) {
 _FDT((spapr_populate_drconf_memory(spapr, fdt)));
 }
-- 
2.1.0

Re: [Qemu-devel] [PATCHv4 7/8] pseries: Clean up error reporting in ppc_spapr_init()

[Alexey Kardashevskiy]

On 01/19/2016 02:39 PM, David Gibson wrote:

This function includes a number of explicit fprintf()s for errors.
Change these to use error_report() instead.

Also replace the single exit(EXIT_FAILURE) with an explicit exit(1), since
the latter is the more usual idiom in qemu by a large margin.

Signed-off-by: David Gibson 


Reviewed-by: Alexey Kardashevskiy 



---
  hw/ppc/spapr.c | 23 ---
  1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index f45be7f..3cfacb9 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1781,8 +1781,8 @@ static void ppc_spapr_init(MachineState *machine)
  }

  if (spapr->rma_size > node0_size) {
-fprintf(stderr, "Error: Numa node 0 has to span the RMA 
(%#08"HWADDR_PRIx")\n",
-spapr->rma_size);
+error_report("Numa node 0 has to span the RMA (%#08"HWADDR_PRIx")",
+ spapr->rma_size);
  exit(1);
  }

@@ -1848,10 +1848,10 @@ static void ppc_spapr_init(MachineState *machine)
  ram_addr_t hotplug_mem_size = machine->maxram_size - 
machine->ram_size;

  if (machine->ram_slots > SPAPR_MAX_RAM_SLOTS) {
-error_report("Specified number of memory slots %" PRIu64
- " exceeds max supported %d",
+error_report("Specified number of memory slots %"
+ PRIu64" exceeds max supported %d",
   machine->ram_slots, SPAPR_MAX_RAM_SLOTS);
-exit(EXIT_FAILURE);
+exit(1);
  }

  spapr->hotplug_memory.base = ROUND_UP(machine->ram_size,
@@ -1947,8 +1947,9 @@ static void ppc_spapr_init(MachineState *machine)
  }

  if (spapr->rma_size < (MIN_RMA_SLOF << 20)) {
-fprintf(stderr, "qemu: pSeries SLOF firmware requires >= "
-"%ldM guest RMA (Real Mode Area memory)\n", MIN_RMA_SLOF);
+error_report(
+"pSeries SLOF firmware requires >= %ldM guest RMA (Real Mode Area 
memory)",
+MIN_RMA_SLOF);
  exit(1);
  }

@@ -1964,8 +1965,8 @@ static void ppc_spapr_init(MachineState *machine)
  kernel_le = kernel_size > 0;
  }
  if (kernel_size < 0) {
-fprintf(stderr, "qemu: error loading %s: %s\n",
-kernel_filename, load_elf_strerror(kernel_size));
+error_report("error loading %s: %s",
+ kernel_filename, load_elf_strerror(kernel_size));
  exit(1);
  }

@@ -1978,8 +1979,8 @@ static void ppc_spapr_init(MachineState *machine)
  initrd_size = load_image_targphys(initrd_filename, initrd_base,
load_limit - initrd_base);
  if (initrd_size < 0) {
-fprintf(stderr, "qemu: could not load initial ram disk '%s'\n",
-initrd_filename);
+error_report("could not load initial ram disk '%s'",
+ initrd_filename);
  exit(1);
  }
  } else {




--
Alexey

Re: [Qemu-devel] [PATCHv4 8/8] pseries: Clean up error reporting in htab migration functions

[Alexey Kardashevskiy]

On 01/19/2016 02:39 PM, David Gibson wrote:

The functions for migrating the hash page table on pseries machine type
(htab_save_setup() and htab_load()) can report some errors with an
explicit fprintf() before returning an appropriate error code.  Change these
to use error_report() instead.

Signed-off-by: David Gibson 
Reviewed-by: Thomas Huth 



Reviewed-by: Alexey Kardashevskiy 


---
  hw/ppc/spapr.c | 12 ++--
  1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 3cfacb9..1eb7d03 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1526,7 +1526,7 @@ static int htab_load(QEMUFile *f, void *opaque, int 
version_id)
  int fd = -1;

  if (version_id < 1 || version_id > 1) {
-fprintf(stderr, "htab_load() bad version\n");
+error_report("htab_load() bad version");
  return -EINVAL;
  }

@@ -1547,8 +1547,8 @@ static int htab_load(QEMUFile *f, void *opaque, int 
version_id)

  fd = kvmppc_get_htab_fd(true);
  if (fd < 0) {
-fprintf(stderr, "Unable to open fd to restore KVM hash table: 
%s\n",
-strerror(errno));
+error_report("Unable to open fd to restore KVM hash table: %s",
+ strerror(errno));
  }
  }

@@ -1568,9 +1568,9 @@ static int htab_load(QEMUFile *f, void *opaque, int 
version_id)
  if ((index + n_valid + n_invalid) >
  (HTAB_SIZE(spapr) / HASH_PTE_SIZE_64)) {
  /* Bad index in stream */
-fprintf(stderr, "htab_load() bad index %d (%hd+%hd entries) "
-"in htab stream (htab_shift=%d)\n", index, n_valid, 
n_invalid,
-spapr->htab_shift);
+error_report(
+"htab_load() bad index %d (%hd+%hd entries) in htab stream 
(htab_shift=%d)",
+index, n_valid, n_invalid, spapr->htab_shift);
  return -EINVAL;
  }





--
Alexey

Re: [Qemu-devel] [PATCHv4 5/8] pseries: Clean up error handling in spapr_rtas_register()

[Alexey Kardashevskiy]

On 01/19/2016 02:39 PM, David Gibson wrote:

The errors detected in this function necessarily indicate bugs in the rest
of the qemu code, rather than an external or configuration problem.

So, a simple assert() is more appropriate than any more complex error
reporting.

Signed-off-by: David Gibson 
Reviewed-by: Thomas Huth 



Reviewed-by: Alexey Kardashevskiy 


---
  hw/ppc/spapr_rtas.c | 12 +++-
  1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index 34b12a3..0be52ae 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -648,17 +648,11 @@ target_ulong spapr_rtas_call(PowerPCCPU *cpu, 
sPAPRMachineState *spapr,

  void spapr_rtas_register(int token, const char *name, spapr_rtas_fn fn)
  {
-if (!((token >= RTAS_TOKEN_BASE) && (token < RTAS_TOKEN_MAX))) {
-fprintf(stderr, "RTAS invalid token 0x%x\n", token);
-exit(1);
-}
+assert((token >= RTAS_TOKEN_BASE) && (token < RTAS_TOKEN_MAX));

  token -= RTAS_TOKEN_BASE;
-if (rtas_table[token].name) {
-fprintf(stderr, "RTAS call \"%s\" is registered already as 0x%x\n",
-rtas_table[token].name, token);
-exit(1);
-}
+
+assert(!rtas_table[token].name);

  rtas_table[token].name = name;
  rtas_table[token].fn = fn;




--
Alexey

Re: [Qemu-devel] [PATCHv4 2/8] pseries: Cleanup error handling of spapr_cpu_init()

[Alexey Kardashevskiy]

On 01/19/2016 02:39 PM, David Gibson wrote:

Currently spapr_cpu_init() is hardcoded to handle any errors as fatal.
That works for now, since it's only called from initial setup where an
error here means we really can't proceed.

However, we'll want to handle this more flexibly for cpu hotplug in future
so generalize this using the error reporting infrastructure.  While we're
at it make a small cleanup in a related part of ppc_spapr_init() to use
error_report() instead of an old-style explicit fprintf().

Signed-off-by: David Gibson 
Reviewed-by: Bharata B Rao 


Reviewed-by: Alexey Kardashevskiy 



---
  hw/ppc/spapr.c | 15 +++
  1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index fa7a7f4..b7fd09a 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1617,7 +1617,8 @@ static void spapr_boot_set(void *opaque, const char 
*boot_device,
  machine->boot_order = g_strdup(boot_device);
  }

-static void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu)
+static void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu,
+   Error **errp)
  {
  CPUPPCState *env = &cpu->env;

@@ -1635,7 +1636,13 @@ static void spapr_cpu_init(sPAPRMachineState *spapr, 
PowerPCCPU *cpu)
  }

  if (cpu->max_compat) {
-ppc_set_compat(cpu, cpu->max_compat, &error_fatal);
+Error *local_err = NULL;
+
+ppc_set_compat(cpu, cpu->max_compat, &local_err);
+if (local_err) {
+error_propagate(errp, local_err);
+return;
+}
  }

  xics_cpu_setup(spapr->icp, cpu);
@@ -1804,10 +1811,10 @@ static void ppc_spapr_init(MachineState *machine)
  for (i = 0; i < smp_cpus; i++) {
  cpu = cpu_ppc_init(machine->cpu_model);
  if (cpu == NULL) {
-fprintf(stderr, "Unable to find PowerPC CPU definition\n");
+error_report("Unable to find PowerPC CPU definition");
  exit(1);
  }
-spapr_cpu_init(spapr, cpu);
+spapr_cpu_init(spapr, cpu, &error_fatal);
  }

  if (kvm_enabled()) {




--
Alexey

[Qemu-devel] [PATCHv2 0/3] Reduce abuse of rtas_st / rtas_ld

[David Gibson]

The rtas_ld() and rtas_st() helpers were designed for loading RTAS
arguments and storing RTAS returns which are in a simple, common array
format.

However, a number of RTAS routines - and even non-RTAS routines - have
started using these for accessing other memory buffers, where the
normal qemu memory access routines would be more appropriate.

This series removes some of these abuses of the RTAS accessors.

Changes in v2:
 * Reworked 1/3 to use a local helper instead of open-coding
 * Assorted small cleanups suggesed by Alexey Kardashevskiy

David Gibson (3):
  spapr: Small fixes to rtas_ibm_get_system_parameter, remove
rtas_st_buffer
  spapr: Remove rtas_st_buffer_direct()
  spapr: Remove abuse of rtas_ld() in h_client_architecture_support

 hw/ppc/spapr_hcall.c   | 14 +++---
 hw/ppc/spapr_rtas.c| 39 ---
 include/hw/ppc/spapr.h | 36 +---
 3 files changed, 44 insertions(+), 45 deletions(-)

-- 
2.5.0

[Qemu-devel] [PATCHv2 1/3] spapr: Small fixes to rtas_ibm_get_system_parameter, remove rtas_st_buffer

[David Gibson]

rtas_st_buffer() appears in spapr.h as though it were a widely used helper,
but in fact it is only used for saving data in a format used by
rtas_ibm_get_system_parameter().  This changes it to a local helper more
specifically for that function.

While we're there fix a couple of small defects in
rtas_ibm_get_system_parameter:
  - For the string value SPLPAR_CHARACTERISTICS, it wasn't including the
terminating \0 in the length which it should according to LoPAPR
7.3.16.1
  - It now checks that the supplied buffer has at least enough space for
the length of the returned data, and returns an error if it does not.

Signed-off-by: David Gibson 
---
 hw/ppc/spapr_rtas.c| 22 ++
 include/hw/ppc/spapr.h | 28 +---
 2 files changed, 27 insertions(+), 23 deletions(-)

diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index 34b12a3..32cdd66 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -228,6 +228,20 @@ static void rtas_stop_self(PowerPCCPU *cpu, 
sPAPRMachineState *spapr,
 env->msr = 0;
 }
 
+
+static inline int sysparm_st(target_ulong addr, target_ulong len,
+ const void *val, uint16_t vallen)
+{
+hwaddr phys = ppc64_phys_to_real(addr);
+
+if (len < 2) {
+return RTAS_OUT_SYSPARM_PARAM_ERROR;
+}
+stw_be_phys(&address_space_memory, phys, vallen);
+cpu_physical_memory_write(phys + 2, val, MIN(len - 2, vallen));
+return RTAS_OUT_SUCCESS;
+}
+
 static void rtas_ibm_get_system_parameter(PowerPCCPU *cpu,
   sPAPRMachineState *spapr,
   uint32_t token, uint32_t nargs,
@@ -237,7 +251,7 @@ static void rtas_ibm_get_system_parameter(PowerPCCPU *cpu,
 target_ulong parameter = rtas_ld(args, 0);
 target_ulong buffer = rtas_ld(args, 1);
 target_ulong length = rtas_ld(args, 2);
-target_ulong ret = RTAS_OUT_SUCCESS;
+target_ulong ret;
 
 switch (parameter) {
 case RTAS_SYSPARM_SPLPAR_CHARACTERISTICS: {
@@ -249,18 +263,18 @@ static void rtas_ibm_get_system_parameter(PowerPCCPU *cpu,
   current_machine->ram_size / M_BYTE,
   smp_cpus,
   max_cpus);
-rtas_st_buffer(buffer, length, (uint8_t *)param_val, 
strlen(param_val));
+ret = sysparm_st(buffer, length, param_val, strlen(param_val) + 1);
 g_free(param_val);
 break;
 }
 case RTAS_SYSPARM_DIAGNOSTICS_RUN_MODE: {
 uint8_t param_val = DIAGNOSTICS_RUN_MODE_DISABLED;
 
-rtas_st_buffer(buffer, length, ¶m_val, sizeof(param_val));
+ret = sysparm_st(buffer, length, ¶m_val, sizeof(param_val));
 break;
 }
 case RTAS_SYSPARM_UUID:
-rtas_st_buffer(buffer, length, qemu_uuid, (qemu_uuid_set ? 16 : 0));
+ret = sysparm_st(buffer, length, qemu_uuid, (qemu_uuid_set ? 16 : 0));
 break;
 default:
 ret = RTAS_OUT_NOT_SUPPORTED;
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 53af76a..1e10fc9 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -408,14 +408,15 @@ int spapr_allocate_irq_block(int num, bool lsi, bool msi);
 #define RTAS_SLOT_PERM_ERR_LOG   2
 
 /* RTAS return codes */
-#define RTAS_OUT_SUCCESS0
-#define RTAS_OUT_NO_ERRORS_FOUND1
-#define RTAS_OUT_HW_ERROR   -1
-#define RTAS_OUT_BUSY   -2
-#define RTAS_OUT_PARAM_ERROR-3
-#define RTAS_OUT_NOT_SUPPORTED  -3
-#define RTAS_OUT_NO_SUCH_INDICATOR  -3
-#define RTAS_OUT_NOT_AUTHORIZED -9002
+#define RTAS_OUT_SUCCESS0
+#define RTAS_OUT_NO_ERRORS_FOUND1
+#define RTAS_OUT_HW_ERROR   -1
+#define RTAS_OUT_BUSY   -2
+#define RTAS_OUT_PARAM_ERROR-3
+#define RTAS_OUT_NOT_SUPPORTED  -3
+#define RTAS_OUT_NO_SUCH_INDICATOR  -3
+#define RTAS_OUT_NOT_AUTHORIZED -9002
+#define RTAS_OUT_SYSPARM_PARAM_ERROR-
 
 /* RTAS tokens */
 #define RTAS_TOKEN_BASE  0x2000
@@ -513,17 +514,6 @@ static inline void rtas_st_buffer_direct(target_ulong phys,
   MIN(buffer_len, phys_len));
 }
 
-static inline void rtas_st_buffer(target_ulong phys, target_ulong phys_len,
-  uint8_t *buffer, uint16_t buffer_len)
-{
-if (phys_len < 2) {
-return;
-}
-stw_be_phys(&address_space_memory,
-ppc64_phys_to_real(phys), buffer_len);
-rtas_st_buffer_direct(phys + 2, phys_len - 2, buffer, buffer_len);
-}
-
 typedef void (*spapr_rtas_fn)(PowerPCCPU *cpu, sPAPRMachineState *sm,
   uint32_t token,
   uint32_t nargs, target_ulong args,
-- 
2.5.0

Re: [Qemu-devel] [PATCHv4 6/8] pseries: Clean up error handling in xics_system_init()

[Alexey Kardashevskiy]

On 01/19/2016 02:39 PM, David Gibson wrote:

Use the error handling infrastructure to pass an error out from
try_create_xics() instead of assuming &error_abort - the caller is in a
better position to decide on error handling policy.

Also change the error handling from an &error_abort to &error_fatal, since
this occurs during the initial machine construction and could be triggered
by bad configuration rather than a program error.

Signed-off-by: David Gibson 
Reviewed-by: Thomas Huth 


Reviewed-by: Alexey Kardashevskiy 



---
  hw/ppc/spapr.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 1ce9b27..f45be7f 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -111,7 +111,7 @@ static XICSState *try_create_xics(const char *type, int 
nr_servers,
  }

  static XICSState *xics_system_init(MachineState *machine,
-   int nr_servers, int nr_irqs)
+   int nr_servers, int nr_irqs, Error **errp)
  {
  XICSState *icp = NULL;

@@ -130,7 +130,7 @@ static XICSState *xics_system_init(MachineState *machine,
  }

  if (!icp) {
-icp = try_create_xics(TYPE_XICS, nr_servers, nr_irqs, &error_abort);
+icp = try_create_xics(TYPE_XICS, nr_servers, nr_irqs, errp);
  }

  return icp;
@@ -1805,7 +1805,7 @@ static void ppc_spapr_init(MachineState *machine)
  spapr->icp = xics_system_init(machine,
DIV_ROUND_UP(max_cpus * 
kvmppc_smt_threads(),
 smp_threads),
-  XICS_IRQS);
+  XICS_IRQS, &error_fatal);

  if (smc->dr_lmb_enabled) {
  spapr_validate_node_memory(machine, &error_fatal);




--
Alexey

Re: [Qemu-devel] [PATCHv4 3/8] pseries: Clean up error handling in spapr_validate_node_memory()

[Alexey Kardashevskiy]

On 01/19/2016 02:39 PM, David Gibson wrote:

Use error_setg() and return an error, rather than using an explicit exit().

Also improve messages, and be more explicit about which constraint failed.

Signed-off-by: David Gibson 
Reviewed-by: Bharata B Rao 
Reviewed-by: Thomas Huth 



Reviewed-by: Alexey Kardashevskiy 


---
  hw/ppc/spapr.c | 37 ++---
  1 file changed, 22 insertions(+), 15 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index b7fd09a..fb0e254 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1691,27 +1691,34 @@ static void 
spapr_create_lmb_dr_connectors(sPAPRMachineState *spapr)
   * to SPAPR_MEMORY_BLOCK_SIZE(256MB), then refuse to start the guest
   * since we can't support such unaligned sizes with DRCONF_MEMORY.
   */
-static void spapr_validate_node_memory(MachineState *machine)
+static void spapr_validate_node_memory(MachineState *machine, Error **errp)
  {
  int i;

-if (machine->maxram_size % SPAPR_MEMORY_BLOCK_SIZE ||
-machine->ram_size % SPAPR_MEMORY_BLOCK_SIZE) {
-error_report("Can't support memory configuration where RAM size "
- "0x" RAM_ADDR_FMT " or maxmem size "
- "0x" RAM_ADDR_FMT " isn't aligned to %llu MB",
- machine->ram_size, machine->maxram_size,
- SPAPR_MEMORY_BLOCK_SIZE/M_BYTE);
-exit(EXIT_FAILURE);
+if (machine->ram_size % SPAPR_MEMORY_BLOCK_SIZE) {
+error_setg(errp, "Memory size 0x" RAM_ADDR_FMT
+   " is not aligned to %llu MiB",
+   machine->ram_size,
+   SPAPR_MEMORY_BLOCK_SIZE / M_BYTE);
+return;
+}
+
+if (machine->maxram_size % SPAPR_MEMORY_BLOCK_SIZE) {
+error_setg(errp, "Maximum memory size 0x" RAM_ADDR_FMT
+   " is not aligned to %llu MiB",
+   machine->ram_size,
+   SPAPR_MEMORY_BLOCK_SIZE / M_BYTE);
+return;
  }

  for (i = 0; i < nb_numa_nodes; i++) {
  if (numa_info[i].node_mem % SPAPR_MEMORY_BLOCK_SIZE) {
-error_report("Can't support memory configuration where memory size"
- " %" PRIx64 " of node %d isn't aligned to %llu MB",
- numa_info[i].node_mem, i,
- SPAPR_MEMORY_BLOCK_SIZE/M_BYTE);
-exit(EXIT_FAILURE);
+error_setg(errp,
+   "Node %d memory size 0x" RAM_ADDR_FMT
+   " is not aligned to %llu MiB",
+   i, numa_info[i].node_mem,
+   SPAPR_MEMORY_BLOCK_SIZE / M_BYTE);
+return;
  }
  }
  }
@@ -1801,7 +1808,7 @@ static void ppc_spapr_init(MachineState *machine)
XICS_IRQS);

  if (smc->dr_lmb_enabled) {
-spapr_validate_node_memory(machine);
+spapr_validate_node_memory(machine, &error_fatal);
  }

  /* init CPUs */




--
Alexey

Re: [Qemu-devel] [PATCHv4 4/8] pseries: Cleanup error handling in spapr_vga_init()

[Alexey Kardashevskiy]

On 01/19/2016 02:39 PM, David Gibson wrote:

Use error_setg() to return an error rather than an explicit exit().
Previously it was an exit(0) instead of a non-zero exit code, which was
simply a bug.  Also improve the error message.

While we're at it change the type of spapr_vga_init() to bool since that's
how we're using it anyway.

Signed-off-by: David Gibson 
Reviewed-by: Thomas Huth 


Reviewed-by: Alexey Kardashevskiy 



---
  hw/ppc/spapr.c | 10 +-
  1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index fb0e254..1ce9b27 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1238,7 +1238,7 @@ static void spapr_rtc_create(sPAPRMachineState *spapr)
  }

  /* Returns whether we want to use VGA or not */
-static int spapr_vga_init(PCIBus *pci_bus)
+static bool spapr_vga_init(PCIBus *pci_bus, Error **errp)
  {
  switch (vga_interface_type) {
  case VGA_NONE:
@@ -1249,9 +1249,9 @@ static int spapr_vga_init(PCIBus *pci_bus)
  case VGA_VIRTIO:
  return pci_vga_init(pci_bus) != NULL;
  default:
-fprintf(stderr, "This vga model is not supported,"
-"currently it only supports -vga std\n");
-exit(0);
+error_setg(errp,
+   "Unsupported VGA mode, only -vga std or -vga virtio is 
supported");
+return false;
  }
  }

@@ -1926,7 +1926,7 @@ static void ppc_spapr_init(MachineState *machine)
  }

  /* Graphics */
-if (spapr_vga_init(phb->bus)) {
+if (spapr_vga_init(phb->bus, &error_fatal)) {
  spapr->has_graphics = true;
  machine->usb |= defaults_enabled() && !machine->usb_disabled;
  }




--
Alexey

Re: [Qemu-devel] [PATCHv4 1/8] ppc: Cleanup error handling in ppc_set_compat()

[Alexey Kardashevskiy]

On 01/19/2016 02:39 PM, David Gibson wrote:

Current ppc_set_compat() returns -1 for errors, and also (unconditionally)
reports an error message.  The caller in h_client_architecture_support()
may then report it again using an outdated fprintf().

Clean this up by using the modern error reporting mechanisms.  Also add
strerror(errno) to the error message.

Signed-off-by: David Gibson 
Reviewed-by: Thomas Huth 


Reviewed-by: Alexey Kardashevskiy 


---
  hw/ppc/spapr.c  |  4 +---
  hw/ppc/spapr_hcall.c| 10 +-
  target-ppc/cpu.h|  2 +-
  target-ppc/translate_init.c | 13 +++--
  4 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 50e5a26..fa7a7f4 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1635,9 +1635,7 @@ static void spapr_cpu_init(sPAPRMachineState *spapr, 
PowerPCCPU *cpu)
  }

  if (cpu->max_compat) {
-if (ppc_set_compat(cpu, cpu->max_compat) < 0) {
-exit(1);
-}
+ppc_set_compat(cpu, cpu->max_compat, &error_fatal);
  }

  xics_cpu_setup(spapr->icp, cpu);
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index cebceea..8b0fcb3 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -837,7 +837,7 @@ static target_ulong cas_get_option_vector(int vector, 
target_ulong table)
  typedef struct {
  PowerPCCPU *cpu;
  uint32_t cpu_version;
-int ret;
+Error *err;
  } SetCompatState;

  static void do_set_compat(void *arg)
@@ -845,7 +845,7 @@ static void do_set_compat(void *arg)
  SetCompatState *s = arg;

  cpu_synchronize_state(CPU(s->cpu));
-s->ret = ppc_set_compat(s->cpu, s->cpu_version);
+ppc_set_compat(s->cpu, s->cpu_version, &s->err);
  }

  #define get_compat_level(cpuver) ( \
@@ -929,13 +929,13 @@ static target_ulong 
h_client_architecture_support(PowerPCCPU *cpu_,
  SetCompatState s = {
  .cpu = POWERPC_CPU(cs),
  .cpu_version = cpu_version,
-.ret = 0
+.err = NULL,
  };

  run_on_cpu(cs, do_set_compat, &s);

-if (s.ret < 0) {
-fprintf(stderr, "Unable to set compatibility mode\n");
+if (s.err) {
+error_report_err(s.err);
  return H_HARDWARE;
  }
  }
diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 9706000..b3b89e6 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -1210,7 +1210,7 @@ void ppc_store_msr (CPUPPCState *env, target_ulong value);

  void ppc_cpu_list (FILE *f, fprintf_function cpu_fprintf);
  int ppc_get_compat_smt_threads(PowerPCCPU *cpu);
-int ppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version);
+void ppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version, Error **errp);

  /* Time-base and decrementer management */
  #ifndef NO_CPU_IO_DEFS
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 4ab2d92..678957a 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -9186,7 +9186,7 @@ int ppc_get_compat_smt_threads(PowerPCCPU *cpu)
  return ret;
  }

-int ppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version)
+void ppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version, Error **errp)
  {
  int ret = 0;
  CPUPPCState *env = &cpu->env;
@@ -9208,12 +9208,13 @@ int ppc_set_compat(PowerPCCPU *cpu, uint32_t 
cpu_version)
  break;
  }

-if (kvm_enabled() && kvmppc_set_compat(cpu, cpu->cpu_version) < 0) {
-error_report("Unable to set compatibility mode in KVM");
-ret = -1;
+if (kvm_enabled()) {
+ret = kvmppc_set_compat(cpu, cpu->cpu_version);
+if (ret < 0) {
+error_setg_errno(errp, -ret,
+ "Unable to set CPU compatibility mode in KVM");
+}
  }
-
-return ret;
  }

  static gint ppc_cpu_compare_class_pvr(gconstpointer a, gconstpointer b)




--
Alexey

[Qemu-devel] [PATCHv2 2/3] spapr: Remove rtas_st_buffer_direct()

[David Gibson]

rtas_st_buffer_direct() is a not particularly useful wrapper around
cpu_physical_memory_write().  All the callers are in
rtas_ibm_configure_connector, where it's better handled by local helper.

Signed-off-by: David Gibson 
---
 hw/ppc/spapr_rtas.c| 17 ++---
 include/hw/ppc/spapr.h |  8 
 2 files changed, 10 insertions(+), 15 deletions(-)

diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index 32cdd66..96759be 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -506,6 +506,13 @@ out:
 #define CC_VAL_DATA_OFFSET ((CC_IDX_PROP_DATA_OFFSET + 1) * 4)
 #define CC_WA_LEN 4096
 
+static void configure_connector_st(target_ulong addr, target_ulong offset,
+   const void *buf, size_t len)
+{
+cpu_physical_memory_write(ppc64_phys_to_real(addr + offset),
+  buf, MIN(len, CC_WA_LEN - offset));
+}
+
 static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
  sPAPRMachineState *spapr,
  uint32_t token, uint32_t nargs,
@@ -571,8 +578,7 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
 /* provide the name of the next OF node */
 wa_offset = CC_VAL_DATA_OFFSET;
 rtas_st(wa_addr, CC_IDX_NODE_NAME_OFFSET, wa_offset);
-rtas_st_buffer_direct(wa_addr + wa_offset, CC_WA_LEN - wa_offset,
-  (uint8_t *)name, strlen(name) + 1);
+configure_connector_st(wa_addr, wa_offset, name, strlen(name) + 1);
 resp = SPAPR_DR_CC_RESPONSE_NEXT_CHILD;
 break;
 case FDT_END_NODE:
@@ -597,8 +603,7 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
 /* provide the name of the next OF property */
 wa_offset = CC_VAL_DATA_OFFSET;
 rtas_st(wa_addr, CC_IDX_PROP_NAME_OFFSET, wa_offset);
-rtas_st_buffer_direct(wa_addr + wa_offset, CC_WA_LEN - wa_offset,
-  (uint8_t *)name, strlen(name) + 1);
+configure_connector_st(wa_addr, wa_offset, name, strlen(name) + 1);
 
 /* provide the length and value of the OF property. data gets
  * placed immediately after NULL terminator of the OF property's
@@ -607,9 +612,7 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
 wa_offset += strlen(name) + 1,
 rtas_st(wa_addr, CC_IDX_PROP_LEN, prop_len);
 rtas_st(wa_addr, CC_IDX_PROP_DATA_OFFSET, wa_offset);
-rtas_st_buffer_direct(wa_addr + wa_offset, CC_WA_LEN - wa_offset,
-  (uint8_t *)((struct fdt_property 
*)prop)->data,
-  prop_len);
+configure_connector_st(wa_addr, wa_offset, prop->data, prop_len);
 resp = SPAPR_DR_CC_RESPONSE_NEXT_PROPERTY;
 break;
 case FDT_END:
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 1e10fc9..1f9e722 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -506,14 +506,6 @@ static inline void rtas_st(target_ulong phys, int n, 
uint32_t val)
 stl_be_phys(&address_space_memory, ppc64_phys_to_real(phys + 4*n), val);
 }
 
-static inline void rtas_st_buffer_direct(target_ulong phys,
- target_ulong phys_len,
- uint8_t *buffer, uint16_t buffer_len)
-{
-cpu_physical_memory_write(ppc64_phys_to_real(phys), buffer,
-  MIN(buffer_len, phys_len));
-}
-
 typedef void (*spapr_rtas_fn)(PowerPCCPU *cpu, sPAPRMachineState *sm,
   uint32_t token,
   uint32_t nargs, target_ulong args,
-- 
2.5.0

Re: [Qemu-devel] [PATCH 1/3] spapr: Small fixes to rtas_ibm_get_system_parameter, remove rtas_st_buffer

[David Gibson]

On Tue, Jan 19, 2016 at 01:00:50PM +1100, Alexey Kardashevskiy wrote:
> On 01/16/2016 12:14 PM, David Gibson wrote:
> >rtas_st_buffer() appears in spapr.h as though it were a widely used helper,
> >but in fact it is only used for saving data in a format used by
> >rtas_ibm_get_system_parameter().  We can fold it into that caller just as
> >simply.
> >
> >While we're there fix a couple of small defects in
> >rtas_ibm_get_system_parameter:
> >   - For the string value SPLPAR_CHARACTERISTICS, it wasn't including the
> > terminating \0 in the length which it should according to LoPAPR
> > 7.3.16.1
> >   - It now checks that the supplied buffer has at least enough space for
> > the length of the returned data, and returns an error if it does not.
> >
> >Signed-off-by: David Gibson 
> >---
> >  hw/ppc/spapr_rtas.c| 28 
> >  include/hw/ppc/spapr.h | 11 ---
> >  2 files changed, 20 insertions(+), 19 deletions(-)
> >
> >diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> >index 34b12a3..f4fb9ba 100644
> >--- a/hw/ppc/spapr_rtas.c
> >+++ b/hw/ppc/spapr_rtas.c
> >@@ -235,9 +235,15 @@ static void rtas_ibm_get_system_parameter(PowerPCCPU 
> >*cpu,
> >uint32_t nret, target_ulong rets)
> >  {
> >  target_ulong parameter = rtas_ld(args, 0);
> >-target_ulong buffer = rtas_ld(args, 1);
> >+target_ulong buffer = ppc64_phys_to_real(rtas_ld(args, 1));
> >  target_ulong length = rtas_ld(args, 2);
> >-target_ulong ret = RTAS_OUT_SUCCESS;
> >+void *val;
> >+size_t vallen;
> 
> Having such temporary variables (val/vallen) tells me that a helper is a
> better solution as this assumes that one word and one buffer will be ever
> enough for all cases but there is already "7.3.16.17 Processor Module
> Information" coming (even though I have not heard from Sukadev for quite
> some time) which is more complicated that this.

Actually latest discussions with Sukadev and the RH licensing people
suggest we won't need that after all.  But still, point taken.

> May be rtas_st_buffer() is not the best name and it does not have to be
> global so I'd propose making it static and calling it syspar_st().

Ok, that works for me.

> 
> 
> >+
> >+if (length < 2) {
> >+rtas_st(rets, 0, -); /* Parameter error */
> 
> 
> #define RTAS_OUT_SYSPAR_PARAM_ERROR -
> and then
> rtas_st(rets, 0, RTAS_OUT_SYSPAR_PARAM_ERROR);
> 
> ?

Hm, ok.  I'm not entirely sold on the need, since most RTAS return
values are specific to individual RTAS calls, and they're not given
symbolic names by PAPR.  Still, why not.

> 
> In all other places ("rtas_st(\S\+, 0") a macro is used (except just one
> case in spapr_pci.c which needs a fix).
> 
> 
> >+return;
> >+}
> >
> >  switch (parameter) {
> >  case RTAS_SYSPARM_SPLPAR_CHARACTERISTICS: {
> >@@ -249,24 +255,30 @@ static void rtas_ibm_get_system_parameter(PowerPCCPU 
> >*cpu,
> >current_machine->ram_size / 
> > M_BYTE,
> >smp_cpus,
> >max_cpus);
> >-rtas_st_buffer(buffer, length, (uint8_t *)param_val, 
> >strlen(param_val));
> >+val = param_val;
> >+vallen = strlen(param_val) + 1;
> >  g_free(param_val);
> >  break;
> >  }
> >  case RTAS_SYSPARM_DIAGNOSTICS_RUN_MODE: {
> >-uint8_t param_val = DIAGNOSTICS_RUN_MODE_DISABLED;
> >+uint8_t diagnostics_run_mode = DIAGNOSTICS_RUN_MODE_DISABLED;
> >
> >-rtas_st_buffer(buffer, length, ¶m_val, sizeof(param_val));
> >+val = &diagnostics_run_mode;
> 
> 
> I know that any reasonable compiler will keep @diagnostics_run_mode on stack
> till return but it is still not clean...

For a minute I didn't see what you were getting at, then realized that
diagnostics_run_mode is in a different scope from the final store.

Yeah, that's not ok.

> 
> 
> 
> >+vallen = sizeof(diagnostics_run_mode);
> >  break;
> >  }
> >  case RTAS_SYSPARM_UUID:
> >-rtas_st_buffer(buffer, length, qemu_uuid, (qemu_uuid_set ? 16 : 0));
> >+val = qemu_uuid;
> >+vallen = qemu_uuid_set ? 16 : 0;
> >  break;
> >  default:
> >-ret = RTAS_OUT_NOT_SUPPORTED;
> >+rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
> >+return;
> >  }
> >
> >-rtas_st(rets, 0, ret);
> >+stw_be_phys(&address_space_memory, buffer, vallen);
> >+cpu_physical_memory_write(buffer + 2, val, MIN(vallen, length - 2));
> >+rtas_st(rets, 0, 0); /* Success */
> 
> 
> rtas_st(rets, 0, RTAS_OUT_SUCCESS);
> 
> 
> 
> >  }
> >
> >  static void rtas_ibm_set_system_parameter(PowerPCCPU *cpu,
> >diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> >index 53af76a..ec9e7ea 100644
> >--- a/include/hw/ppc/spapr.h
> >+++ b/include/hw/ppc/spapr.h
> >@@ -513,17 +513,6 @@ static inline void rtas_st_b

[Qemu-devel] [PATCHv2 3/3] spapr: Remove abuse of rtas_ld() in h_client_architecture_support

[David Gibson]

h_client_architecture_support() uses rtas_ld() for general purpose memory
access, despite the fact that it's not an RTAS routine at all and rtas_ld
makes things more awkward.

Clean this up by replacing rtas_ld() calls with appropriate ldXX_phys()
calls.

Signed-off-by: David Gibson 
Reviewed-by: Alexey Kardashevskiy 
---
 hw/ppc/spapr_hcall.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index cebceea..9dbdba9 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -861,7 +861,8 @@ static target_ulong 
h_client_architecture_support(PowerPCCPU *cpu_,
   target_ulong opcode,
   target_ulong *args)
 {
-target_ulong list = args[0], ov_table;
+target_ulong list = ppc64_phys_to_real(args[0]);
+target_ulong ov_table, ov5;
 PowerPCCPUClass *pcc_ = POWERPC_CPU_GET_CLASS(cpu_);
 CPUState *cs;
 bool cpu_match = false, cpu_update = true, memory_update = false;
@@ -875,9 +876,9 @@ static target_ulong 
h_client_architecture_support(PowerPCCPU *cpu_,
 for (counter = 0; counter < 512; ++counter) {
 uint32_t pvr, pvr_mask;
 
-pvr_mask = rtas_ld(list, 0);
+pvr_mask = ldl_be_phys(&address_space_memory, list);
 list += 4;
-pvr = rtas_ld(list, 0);
+pvr = ldl_be_phys(&address_space_memory, list);
 list += 4;
 
 trace_spapr_cas_pvr_try(pvr);
@@ -948,14 +949,13 @@ static target_ulong 
h_client_architecture_support(PowerPCCPU *cpu_,
 /* For the future use: here @ov_table points to the first option vector */
 ov_table = list;
 
-list = cas_get_option_vector(5, ov_table);
-if (!list) {
+ov5 = cas_get_option_vector(5, ov_table);
+if (!ov5) {
 return H_SUCCESS;
 }
 
 /* @list now points to OV 5 */
-list += 2;
-ov5_byte2 = rtas_ld(list, 0) >> 24;
+ov5_byte2 = ldub_phys(&address_space_memory, ov5 + 2);
 if (ov5_byte2 & OV5_DRCONF_MEMORY) {
 memory_update = true;
 }
-- 
2.5.0

Re: [Qemu-devel] [PATCH 2/3] spapr: Remove rtas_st_buffer_direct()

[David Gibson]

On Tue, Jan 19, 2016 at 01:07:23PM +1100, Alexey Kardashevskiy wrote:
> On 01/16/2016 12:14 PM, David Gibson wrote:
> >rtas_st_buffer_direct() is a not particularly useful wrapper around
> >cpu_physical_memory_write().  All the callers are in
> >rtas_ibm_configure_connector, where it's better handled by local helper.
> >
> >Signed-off-by: David Gibson 
> >---
> >  hw/ppc/spapr_rtas.c| 19 ---
> >  include/hw/ppc/spapr.h |  8 
> >  2 files changed, 12 insertions(+), 15 deletions(-)
> >
> >diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> >index f4fb9ba..940509e 100644
> >--- a/hw/ppc/spapr_rtas.c
> >+++ b/hw/ppc/spapr_rtas.c
> >@@ -504,6 +504,13 @@ out:
> >  #define CC_VAL_DATA_OFFSET ((CC_IDX_PROP_DATA_OFFSET + 1) * 4)
> >  #define CC_WA_LEN 4096
> >
> >+static void st_cc_buf(target_ulong addr, target_ulong offset,
> >+  void *buf, size_t len)
> 
> 
> Can we please call it cc_st_buf() to make it clear that "cc" is a prefix and
> not about functionality?

Yeah, st_cc_buf() is not a good name.  I've gone with
"configure_connector_st()" which I hope will be better.

> 
> 
> 
> >+{
> >+cpu_physical_memory_write(ppc64_phys_to_real(addr + offset),
> >+  buf, MIN(len, CC_WA_LEN - offset));
> >+}
> >+
> >  static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
> >   sPAPRMachineState *spapr,
> >   uint32_t token, uint32_t nargs,
> >@@ -526,6 +533,7 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
> >  }
> >
> >  wa_addr = ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 0);
> >+wa_addr = ppc64_phys_to_real(wa_addr);
> 
> 
> Not needed here, st_cc_buf() also does ppc64_phys_to_real().

Good point, corrected.

> >
> >  drc_index = rtas_ld(wa_addr, 0);
> >  drc = spapr_dr_connector_by_index(drc_index);
> >@@ -569,8 +577,7 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
> >  /* provide the name of the next OF node */
> >  wa_offset = CC_VAL_DATA_OFFSET;
> >  rtas_st(wa_addr, CC_IDX_NODE_NAME_OFFSET, wa_offset);
> >-rtas_st_buffer_direct(wa_addr + wa_offset, CC_WA_LEN - 
> >wa_offset,
> >-  (uint8_t *)name, strlen(name) + 1);
> >+st_cc_buf(wa_addr, wa_offset, (uint8_t *)name, strlen(name) + 
> >1);
> 
> 
> Since you made @buf a void* in st_cc_buf(), you do not need (uint8_t*) here
> and below.

Ah, yes good point also.

> 
> 
> 
> >  resp = SPAPR_DR_CC_RESPONSE_NEXT_CHILD;
> >  break;
> >  case FDT_END_NODE:
> >@@ -595,8 +602,7 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
> >  /* provide the name of the next OF property */
> >  wa_offset = CC_VAL_DATA_OFFSET;
> >  rtas_st(wa_addr, CC_IDX_PROP_NAME_OFFSET, wa_offset);
> >-rtas_st_buffer_direct(wa_addr + wa_offset, CC_WA_LEN - 
> >wa_offset,
> >-  (uint8_t *)name, strlen(name) + 1);
> >+st_cc_buf(wa_addr, wa_offset, (uint8_t *)name, strlen(name) + 
> >1);
> >
> >  /* provide the length and value of the OF property. data gets
> >   * placed immediately after NULL terminator of the OF 
> > property's
> >@@ -605,9 +611,8 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
> >  wa_offset += strlen(name) + 1,
> >  rtas_st(wa_addr, CC_IDX_PROP_LEN, prop_len);
> >  rtas_st(wa_addr, CC_IDX_PROP_DATA_OFFSET, wa_offset);
> >-rtas_st_buffer_direct(wa_addr + wa_offset, CC_WA_LEN - 
> >wa_offset,
> >-  (uint8_t *)((struct fdt_property 
> >*)prop)->data,
> >-  prop_len);
> >+st_cc_buf(wa_addr, wa_offset,
> >+  (uint8_t *)((struct fdt_property *)prop)->data, 
> >prop_len);
> >  resp = SPAPR_DR_CC_RESPONSE_NEXT_PROPERTY;
> >  break;
> >  case FDT_END:
> >diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> >index ec9e7ea..d6f4eb4 100644
> >--- a/include/hw/ppc/spapr.h
> >+++ b/include/hw/ppc/spapr.h
> >@@ -505,14 +505,6 @@ static inline void rtas_st(target_ulong phys, int n, 
> >uint32_t val)
> >  stl_be_phys(&address_space_memory, ppc64_phys_to_real(phys + 4*n), 
> > val);
> >  }
> >
> >-static inline void rtas_st_buffer_direct(target_ulong phys,
> >- target_ulong phys_len,
> >- uint8_t *buffer, uint16_t 
> >buffer_len)
> >-{
> >-cpu_physical_memory_write(ppc64_phys_to_real(phys), buffer,
> >-  MIN(buffer_len, phys_len));
> >-}
> >-
> >  typedef void (*spapr_rtas_fn)(PowerPCCPU *cpu, sPAPRMachineState *sm,
> >uint32_t token,
> >uint32_t nargs, target_ulon

Re: [Qemu-devel] [PATCH COLO-Frame v13 38/39] colo: Use default buffer-filter to buffer and release packets

[Jason Wang]

On 12/29/2015 03:09 PM, zhanghailiang wrote:
> Enable default filter to buffer packets and release the
> packets after a checkpoint.
>
> Signed-off-by: zhanghailiang 
> Cc: Jason Wang 
> Cc: Yang Hongyang 
> ---
> v12:
> - Add a helper function to check if all netdev supports buffer packets.
> - Flush buffered packets when do failover.
> v11:
> - Use new helper functions to buffer and release packets.
> ---
>  include/net/net.h |  1 +
>  migration/colo.c  | 25 -
>  net/net.c | 17 +
>  3 files changed, 42 insertions(+), 1 deletion(-)
>
> diff --git a/include/net/net.h b/include/net/net.h
> index 5c65c45..2eb9451 100644
> --- a/include/net/net.h
> +++ b/include/net/net.h
> @@ -129,6 +129,7 @@ typedef void (*qemu_netfilter_foreach)(NetFilterState 
> *nf, void *opaque,
> Error **errp);
>  void qemu_foreach_netfilter(qemu_netfilter_foreach func, void *opaque,
>  Error **errp);
> +bool qemu_netdev_support_netfilter(void);
>  int qemu_can_send_packet(NetClientState *nc);
>  ssize_t qemu_sendv_packet(NetClientState *nc, const struct iovec *iov,
>int iovcnt);
> diff --git a/migration/colo.c b/migration/colo.c
> index bd68e86..23e1131 100644
> --- a/migration/colo.c
> +++ b/migration/colo.c
> @@ -18,6 +18,8 @@
>  #include "qemu/error-report.h"
>  #include "migration/failover.h"
>  #include "qapi-event.h"
> +#include "net/filter.h"
> +#include "net/net.h"
>  
>  static bool vmstate_loading;
>  
> @@ -129,6 +131,11 @@ static void primary_vm_do_failover(void)
>   old_state);
>  return;
>  }
> +/* Don't buffer any packets while exited COLO */
> +qemu_set_default_filters_status(false);
> +/* Flush the residuary buffered packts */
> +qemu_release_default_filters_packets();

It's ok to depend on buffer filter explicitly. But the name of above two
functions need to be renamed (since it was rather generic).

> +
>  /* Notify COLO thread that failover work is finished */
>  qemu_sem_post(&s->colo_sem);
>  }
> @@ -335,6 +342,8 @@ static int colo_do_checkpoint_transaction(MigrationState 
> *s,
>  goto out;
>  }
>  
> +qemu_release_default_filters_packets();
> +
>  if (colo_shutdown) {
>  colo_put_cmd(s->to_dst_file, COLO_COMMAND_GUEST_SHUTDOWN, 
> &local_err);
>  if (local_err) {
> @@ -379,6 +388,17 @@ static int colo_prepare_before_save(MigrationState *s)
>  return ret;
>  }
>  
> +static int colo_init_buffer_filters(void)
> +{
> +if (!qemu_netdev_support_netfilter()) {
> +return -EPERM;
> +}

I think colo should fail to be started if any netdev doesn't support
filter. Instead of failing in the middle.

> +/* Begin to buffer packets that sent by VM */
> +qemu_set_default_filters_status(true);
> +
> +return 0;
> +}
> +
>  static void colo_process_checkpoint(MigrationState *s)
>  {
>  QEMUSizedBuffer *buffer = NULL;
> @@ -387,7 +407,10 @@ static void colo_process_checkpoint(MigrationState *s)
>  int ret;
>  
>  failover_init_state();
> -
> +ret = colo_init_buffer_filters();
> +if (ret < 0) {
> +goto out;
> +}
>  s->rp_state.from_dst_file = qemu_file_get_return_path(s->to_dst_file);
>  if (!s->rp_state.from_dst_file) {
>  error_report("Open QEMUFile from_dst_file failed");
> diff --git a/net/net.c b/net/net.c
> index 30946c5..4678a6e 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -288,6 +288,23 @@ void qemu_foreach_netfilter(qemu_netfilter_foreach func, 
> void *opaque,
>  }
>  }
>  
> +bool qemu_netdev_support_netfilter(void)
> +{
> +NetClientState *nc;
> +
> +QTAILQ_FOREACH(nc, &net_clients, next) {
> +if (nc->info->type == NET_CLIENT_OPTIONS_KIND_NIC) {
> +continue;
> +}
> +if (QTAILQ_EMPTY(&nc->filters)) {
> +error_report("netdev (%s) does not support filter", nc->name);
> +return false;
> +}
> +}
> +
> +return true;
> +}
> +
>  static void qemu_net_client_destructor(NetClientState *nc)
>  {
>  g_free(nc);

Re: [Qemu-devel] [RFC PATCH v2 00/10] Introduce Intel 82574 GbE Controller Emulation (e1000e)

[Jason Wang]

On 01/19/2016 01:35 AM, Leonid Bloch wrote:
> Hello All,
>
> This series is the latest code of the e1000e device emulation being developed.
>
> Changes since v1:
>
> 1. Added support for all the device features:
>   - Interrupt moderation.
>   - RSS.
>   - Multiqueue.
> 2. Simulated exact PCI/PCIe configuration space layout.
> 3. Made fixes needed to pass Microsoft's HW certification tests (HCK).
>
> This series is still an RFC, because the following tasks are not done yet:
>
> 1. See which code can be shared between this device and the existing e1000 
> device.
> 2. Rebase patches to the latest master (current base is v2.3.0).
>
> Please share your thoughts,
> Thanks, Dmitry.

Hi:

Do you have a public git tree for easier reviewing?

Thanks

>
> ===
>
> Hello qemu-devel,
>
> This patch series is an RFC for the new networking device emulation
> we're developing for QEMU.
>
> This new device emulates the Intel 82574 GbE Controller and works
> with unmodified Intel e1000e drivers from the Linux/Windows kernels.
>
> The status of the current series is "Functional Device Ready, work
> on Extended Features in Progress".
>
> More precisely, these patches represent a functional device, which
> is recognized by the standard Intel drivers, and is able to transfer
> TX/RX packets with CSO/TSO offloads, according to the spec.
>
> Extended features not supported yet (work in progress):
>   1. TX/RX Interrupt moderation mechanisms
>   2. RSS
>   3. Full-featured multi-queue (use of multiqueued network backend)
>
> Also, there will be some code refactoring and performance
> optimization efforts.
>
> This series was tested on Linux (Fedora 22) and Windows (2012R2)
> guests, using Iperf, with TX/RX and TCP/UDP streams, and various
> packet sizes.
>
> More thorough testing, including data streams with different MTU
> sizes, and Microsoft Certification (HLK) tests, are pending missing
> features' development.
>
> See commit messages (esp. "net: Introduce e1000e device emulation")
> for more information about the development approaches and the
> architecture options chosen for this device.
>
> This series is based upon v2.3.0 tag of the upstream QEMU repository,
> and it will be rebased to latest before the final submission.
>
> Please share your thoughts - any feedback is highly welcomed :)
>
> Best Regards,
> Dmitry Fleytman.
>
> Dmitry Fleytman (10):
>   msix: make msix_clr_pending() visible for clients
>   pci: Introduce function for PCI PM capability creation
>   pcie: Add support for PCIe CAP v1
>   pcie: Introduce function for DSN capability creation
>   net: Introduce Toeplitz hash calculator
>   net: Add macros for ETH address tracing
>   net_pkt: Name vmxnet3 packet abstractions more generic
>   net_pkt: Extend packet abstraction as requied by e1000e functionality
>   e1000_regs: Add definitions for Intel 82574-specific bits
>   net: Introduce e1000e device emulation
>
>  MAINTAINERS|   14 +
>  default-configs/pci.mak|1 +
>  hw/net/Makefile.objs   |5 +-
>  hw/net/e1000_regs.h|  353 -
>  hw/net/e1000e.c|  700 +
>  hw/net/e1000e_core.c   | 3453 
> 
>  hw/net/e1000e_core.h   |  230 +++
>  hw/net/net_rx_pkt.c|  536 +++
>  hw/net/net_rx_pkt.h|  353 +
>  hw/net/net_tx_pkt.c|  627 
>  hw/net/net_tx_pkt.h|  191 +++
>  hw/net/vmxnet3.c   |   80 +-
>  hw/net/vmxnet_rx_pkt.c |  187 ---
>  hw/net/vmxnet_rx_pkt.h |  174 ---
>  hw/net/vmxnet_tx_pkt.c |  567 
>  hw/net/vmxnet_tx_pkt.h |  148 --
>  hw/pci/msix.c  |2 +-
>  hw/pci/pci.c   |   21 +
>  hw/pci/pcie.c  |   96 +-
>  include/hw/pci/msix.h  |1 +
>  include/hw/pci/pci.h   |2 +
>  include/hw/pci/pci_regs.h  |4 +
>  include/hw/pci/pcie.h  |5 +
>  include/hw/pci/pcie_regs.h |8 +-
>  include/net/checksum.h |   49 +-
>  include/net/eth.h  |  161 ++-
>  include/net/net.h  |5 +
>  net/checksum.c |7 +-
>  net/eth.c  |  410 +-
>  tests/Makefile |4 +-
>  trace-events   |  195 +++
>  31 files changed, 7350 insertions(+), 1239 deletions(-)
>  create mode 100644 hw/net/e1000e.c
>  create mode 100644 hw/net/e1000e_core.c
>  create mode 100644 hw/net/e1000e_core.h
>  create mode 100644 hw/net/net_rx_pkt.c
>  create mode 100644 hw/net/net_rx_pkt.h
>  create mode 100644 hw/net/net_tx_pkt.c
>  create mode 100644 hw/net/net_tx_pkt.h
>  delete mode 100644 hw/net/vmxnet_rx_pkt.c
>  delete mode 100644 hw/net/vmxnet_rx_pkt.h
>  delete mode 100644 hw/net/vmxnet_tx_pkt.c
>  delete mode 100644 hw/net/vmxnet_tx_pkt.h
>

[Qemu-devel] [PATCHv4 1/8] ppc: Cleanup error handling in ppc_set_compat()

[David Gibson]

Current ppc_set_compat() returns -1 for errors, and also (unconditionally)
reports an error message.  The caller in h_client_architecture_support()
may then report it again using an outdated fprintf().

Clean this up by using the modern error reporting mechanisms.  Also add
strerror(errno) to the error message.

Signed-off-by: David Gibson 
Reviewed-by: Thomas Huth 
---
 hw/ppc/spapr.c  |  4 +---
 hw/ppc/spapr_hcall.c| 10 +-
 target-ppc/cpu.h|  2 +-
 target-ppc/translate_init.c | 13 +++--
 4 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 50e5a26..fa7a7f4 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1635,9 +1635,7 @@ static void spapr_cpu_init(sPAPRMachineState *spapr, 
PowerPCCPU *cpu)
 }
 
 if (cpu->max_compat) {
-if (ppc_set_compat(cpu, cpu->max_compat) < 0) {
-exit(1);
-}
+ppc_set_compat(cpu, cpu->max_compat, &error_fatal);
 }
 
 xics_cpu_setup(spapr->icp, cpu);
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index cebceea..8b0fcb3 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -837,7 +837,7 @@ static target_ulong cas_get_option_vector(int vector, 
target_ulong table)
 typedef struct {
 PowerPCCPU *cpu;
 uint32_t cpu_version;
-int ret;
+Error *err;
 } SetCompatState;
 
 static void do_set_compat(void *arg)
@@ -845,7 +845,7 @@ static void do_set_compat(void *arg)
 SetCompatState *s = arg;
 
 cpu_synchronize_state(CPU(s->cpu));
-s->ret = ppc_set_compat(s->cpu, s->cpu_version);
+ppc_set_compat(s->cpu, s->cpu_version, &s->err);
 }
 
 #define get_compat_level(cpuver) ( \
@@ -929,13 +929,13 @@ static target_ulong 
h_client_architecture_support(PowerPCCPU *cpu_,
 SetCompatState s = {
 .cpu = POWERPC_CPU(cs),
 .cpu_version = cpu_version,
-.ret = 0
+.err = NULL,
 };
 
 run_on_cpu(cs, do_set_compat, &s);
 
-if (s.ret < 0) {
-fprintf(stderr, "Unable to set compatibility mode\n");
+if (s.err) {
+error_report_err(s.err);
 return H_HARDWARE;
 }
 }
diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 9706000..b3b89e6 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -1210,7 +1210,7 @@ void ppc_store_msr (CPUPPCState *env, target_ulong value);
 
 void ppc_cpu_list (FILE *f, fprintf_function cpu_fprintf);
 int ppc_get_compat_smt_threads(PowerPCCPU *cpu);
-int ppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version);
+void ppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version, Error **errp);
 
 /* Time-base and decrementer management */
 #ifndef NO_CPU_IO_DEFS
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 4ab2d92..678957a 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -9186,7 +9186,7 @@ int ppc_get_compat_smt_threads(PowerPCCPU *cpu)
 return ret;
 }
 
-int ppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version)
+void ppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version, Error **errp)
 {
 int ret = 0;
 CPUPPCState *env = &cpu->env;
@@ -9208,12 +9208,13 @@ int ppc_set_compat(PowerPCCPU *cpu, uint32_t 
cpu_version)
 break;
 }
 
-if (kvm_enabled() && kvmppc_set_compat(cpu, cpu->cpu_version) < 0) {
-error_report("Unable to set compatibility mode in KVM");
-ret = -1;
+if (kvm_enabled()) {
+ret = kvmppc_set_compat(cpu, cpu->cpu_version);
+if (ret < 0) {
+error_setg_errno(errp, -ret,
+ "Unable to set CPU compatibility mode in KVM");
+}
 }
-
-return ret;
 }
 
 static gint ppc_cpu_compare_class_pvr(gconstpointer a, gconstpointer b)
-- 
2.5.0

[Qemu-devel] [PATCHv4 7/8] pseries: Clean up error reporting in ppc_spapr_init()

[David Gibson]

This function includes a number of explicit fprintf()s for errors.
Change these to use error_report() instead.

Also replace the single exit(EXIT_FAILURE) with an explicit exit(1), since
the latter is the more usual idiom in qemu by a large margin.

Signed-off-by: David Gibson 
---
 hw/ppc/spapr.c | 23 ---
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index f45be7f..3cfacb9 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1781,8 +1781,8 @@ static void ppc_spapr_init(MachineState *machine)
 }
 
 if (spapr->rma_size > node0_size) {
-fprintf(stderr, "Error: Numa node 0 has to span the RMA 
(%#08"HWADDR_PRIx")\n",
-spapr->rma_size);
+error_report("Numa node 0 has to span the RMA (%#08"HWADDR_PRIx")",
+ spapr->rma_size);
 exit(1);
 }
 
@@ -1848,10 +1848,10 @@ static void ppc_spapr_init(MachineState *machine)
 ram_addr_t hotplug_mem_size = machine->maxram_size - machine->ram_size;
 
 if (machine->ram_slots > SPAPR_MAX_RAM_SLOTS) {
-error_report("Specified number of memory slots %" PRIu64
- " exceeds max supported %d",
+error_report("Specified number of memory slots %"
+ PRIu64" exceeds max supported %d",
  machine->ram_slots, SPAPR_MAX_RAM_SLOTS);
-exit(EXIT_FAILURE);
+exit(1);
 }
 
 spapr->hotplug_memory.base = ROUND_UP(machine->ram_size,
@@ -1947,8 +1947,9 @@ static void ppc_spapr_init(MachineState *machine)
 }
 
 if (spapr->rma_size < (MIN_RMA_SLOF << 20)) {
-fprintf(stderr, "qemu: pSeries SLOF firmware requires >= "
-"%ldM guest RMA (Real Mode Area memory)\n", MIN_RMA_SLOF);
+error_report(
+"pSeries SLOF firmware requires >= %ldM guest RMA (Real Mode Area 
memory)",
+MIN_RMA_SLOF);
 exit(1);
 }
 
@@ -1964,8 +1965,8 @@ static void ppc_spapr_init(MachineState *machine)
 kernel_le = kernel_size > 0;
 }
 if (kernel_size < 0) {
-fprintf(stderr, "qemu: error loading %s: %s\n",
-kernel_filename, load_elf_strerror(kernel_size));
+error_report("error loading %s: %s",
+ kernel_filename, load_elf_strerror(kernel_size));
 exit(1);
 }
 
@@ -1978,8 +1979,8 @@ static void ppc_spapr_init(MachineState *machine)
 initrd_size = load_image_targphys(initrd_filename, initrd_base,
   load_limit - initrd_base);
 if (initrd_size < 0) {
-fprintf(stderr, "qemu: could not load initial ram disk '%s'\n",
-initrd_filename);
+error_report("could not load initial ram disk '%s'",
+ initrd_filename);
 exit(1);
 }
 } else {
-- 
2.5.0

[Qemu-devel] [PATCHv4 6/8] pseries: Clean up error handling in xics_system_init()

[David Gibson]

Use the error handling infrastructure to pass an error out from
try_create_xics() instead of assuming &error_abort - the caller is in a
better position to decide on error handling policy.

Also change the error handling from an &error_abort to &error_fatal, since
this occurs during the initial machine construction and could be triggered
by bad configuration rather than a program error.

Signed-off-by: David Gibson 
Reviewed-by: Thomas Huth 
---
 hw/ppc/spapr.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 1ce9b27..f45be7f 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -111,7 +111,7 @@ static XICSState *try_create_xics(const char *type, int 
nr_servers,
 }
 
 static XICSState *xics_system_init(MachineState *machine,
-   int nr_servers, int nr_irqs)
+   int nr_servers, int nr_irqs, Error **errp)
 {
 XICSState *icp = NULL;
 
@@ -130,7 +130,7 @@ static XICSState *xics_system_init(MachineState *machine,
 }
 
 if (!icp) {
-icp = try_create_xics(TYPE_XICS, nr_servers, nr_irqs, &error_abort);
+icp = try_create_xics(TYPE_XICS, nr_servers, nr_irqs, errp);
 }
 
 return icp;
@@ -1805,7 +1805,7 @@ static void ppc_spapr_init(MachineState *machine)
 spapr->icp = xics_system_init(machine,
   DIV_ROUND_UP(max_cpus * kvmppc_smt_threads(),
smp_threads),
-  XICS_IRQS);
+  XICS_IRQS, &error_fatal);
 
 if (smc->dr_lmb_enabled) {
 spapr_validate_node_memory(machine, &error_fatal);
-- 
2.5.0

[Qemu-devel] [PATCHv4 2/8] pseries: Cleanup error handling of spapr_cpu_init()

[David Gibson]

Currently spapr_cpu_init() is hardcoded to handle any errors as fatal.
That works for now, since it's only called from initial setup where an
error here means we really can't proceed.

However, we'll want to handle this more flexibly for cpu hotplug in future
so generalize this using the error reporting infrastructure.  While we're
at it make a small cleanup in a related part of ppc_spapr_init() to use
error_report() instead of an old-style explicit fprintf().

Signed-off-by: David Gibson 
Reviewed-by: Bharata B Rao 
---
 hw/ppc/spapr.c | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index fa7a7f4..b7fd09a 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1617,7 +1617,8 @@ static void spapr_boot_set(void *opaque, const char 
*boot_device,
 machine->boot_order = g_strdup(boot_device);
 }
 
-static void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu)
+static void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu,
+   Error **errp)
 {
 CPUPPCState *env = &cpu->env;
 
@@ -1635,7 +1636,13 @@ static void spapr_cpu_init(sPAPRMachineState *spapr, 
PowerPCCPU *cpu)
 }
 
 if (cpu->max_compat) {
-ppc_set_compat(cpu, cpu->max_compat, &error_fatal);
+Error *local_err = NULL;
+
+ppc_set_compat(cpu, cpu->max_compat, &local_err);
+if (local_err) {
+error_propagate(errp, local_err);
+return;
+}
 }
 
 xics_cpu_setup(spapr->icp, cpu);
@@ -1804,10 +1811,10 @@ static void ppc_spapr_init(MachineState *machine)
 for (i = 0; i < smp_cpus; i++) {
 cpu = cpu_ppc_init(machine->cpu_model);
 if (cpu == NULL) {
-fprintf(stderr, "Unable to find PowerPC CPU definition\n");
+error_report("Unable to find PowerPC CPU definition");
 exit(1);
 }
-spapr_cpu_init(spapr, cpu);
+spapr_cpu_init(spapr, cpu, &error_fatal);
 }
 
 if (kvm_enabled()) {
-- 
2.5.0

[Qemu-devel] [PATCHv4 0/8] Cleanups to error reporting on ppc and spapr

[David Gibson]

Another spin of my patches to clean up a bunch of error reporting in
the pseries machine type and target-ppc code, to better use the error
API.

Once reviewed, I hope to merge this into ppc-for-2.6 shortly.

Changes in v4:
 * Corrected a place I'd accidentally messed up the indenting
 * Dropped the patch changing the htab allocation path and some hunks
   of the migration path cleanups, since they will be obsoleted by
   other things I'm working on
Changes in v3:
 * Adjusted a commit message for accuracy (suggest by Markus)
 * Dropped a patch which relied on a wrong guess about the behaviour
   of foreach_dynamic_sysbus_device().
Changes in v2:
 * Assorted minor tweaks based on review
  
David Gibson (8):
  ppc: Cleanup error handling in ppc_set_compat()
  pseries: Cleanup error handling of spapr_cpu_init()
  pseries: Clean up error handling in spapr_validate_node_memory()
  pseries: Cleanup error handling in spapr_vga_init()
  pseries: Clean up error handling in spapr_rtas_register()
  pseries: Clean up error handling in xics_system_init()
  pseries: Clean up error reporting in ppc_spapr_init()
  pseries: Clean up error reporting in htab migration functions

 hw/ppc/spapr.c  | 103 +---
 hw/ppc/spapr_hcall.c|  10 ++---
 hw/ppc/spapr_rtas.c |  12 ++
 target-ppc/cpu.h|   2 +-
 target-ppc/translate_init.c |  13 +++---
 5 files changed, 74 insertions(+), 66 deletions(-)

-- 
2.5.0

[Qemu-devel] [PATCHv4 3/8] pseries: Clean up error handling in spapr_validate_node_memory()

[David Gibson]

Use error_setg() and return an error, rather than using an explicit exit().

Also improve messages, and be more explicit about which constraint failed.

Signed-off-by: David Gibson 
Reviewed-by: Bharata B Rao 
Reviewed-by: Thomas Huth 
---
 hw/ppc/spapr.c | 37 ++---
 1 file changed, 22 insertions(+), 15 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index b7fd09a..fb0e254 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1691,27 +1691,34 @@ static void 
spapr_create_lmb_dr_connectors(sPAPRMachineState *spapr)
  * to SPAPR_MEMORY_BLOCK_SIZE(256MB), then refuse to start the guest
  * since we can't support such unaligned sizes with DRCONF_MEMORY.
  */
-static void spapr_validate_node_memory(MachineState *machine)
+static void spapr_validate_node_memory(MachineState *machine, Error **errp)
 {
 int i;
 
-if (machine->maxram_size % SPAPR_MEMORY_BLOCK_SIZE ||
-machine->ram_size % SPAPR_MEMORY_BLOCK_SIZE) {
-error_report("Can't support memory configuration where RAM size "
- "0x" RAM_ADDR_FMT " or maxmem size "
- "0x" RAM_ADDR_FMT " isn't aligned to %llu MB",
- machine->ram_size, machine->maxram_size,
- SPAPR_MEMORY_BLOCK_SIZE/M_BYTE);
-exit(EXIT_FAILURE);
+if (machine->ram_size % SPAPR_MEMORY_BLOCK_SIZE) {
+error_setg(errp, "Memory size 0x" RAM_ADDR_FMT
+   " is not aligned to %llu MiB",
+   machine->ram_size,
+   SPAPR_MEMORY_BLOCK_SIZE / M_BYTE);
+return;
+}
+
+if (machine->maxram_size % SPAPR_MEMORY_BLOCK_SIZE) {
+error_setg(errp, "Maximum memory size 0x" RAM_ADDR_FMT
+   " is not aligned to %llu MiB",
+   machine->ram_size,
+   SPAPR_MEMORY_BLOCK_SIZE / M_BYTE);
+return;
 }
 
 for (i = 0; i < nb_numa_nodes; i++) {
 if (numa_info[i].node_mem % SPAPR_MEMORY_BLOCK_SIZE) {
-error_report("Can't support memory configuration where memory size"
- " %" PRIx64 " of node %d isn't aligned to %llu MB",
- numa_info[i].node_mem, i,
- SPAPR_MEMORY_BLOCK_SIZE/M_BYTE);
-exit(EXIT_FAILURE);
+error_setg(errp,
+   "Node %d memory size 0x" RAM_ADDR_FMT
+   " is not aligned to %llu MiB",
+   i, numa_info[i].node_mem,
+   SPAPR_MEMORY_BLOCK_SIZE / M_BYTE);
+return;
 }
 }
 }
@@ -1801,7 +1808,7 @@ static void ppc_spapr_init(MachineState *machine)
   XICS_IRQS);
 
 if (smc->dr_lmb_enabled) {
-spapr_validate_node_memory(machine);
+spapr_validate_node_memory(machine, &error_fatal);
 }
 
 /* init CPUs */
-- 
2.5.0

[Qemu-devel] [PATCHv4 8/8] pseries: Clean up error reporting in htab migration functions

[David Gibson]

The functions for migrating the hash page table on pseries machine type
(htab_save_setup() and htab_load()) can report some errors with an
explicit fprintf() before returning an appropriate error code.  Change these
to use error_report() instead.

Signed-off-by: David Gibson 
Reviewed-by: Thomas Huth 
---
 hw/ppc/spapr.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 3cfacb9..1eb7d03 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1526,7 +1526,7 @@ static int htab_load(QEMUFile *f, void *opaque, int 
version_id)
 int fd = -1;
 
 if (version_id < 1 || version_id > 1) {
-fprintf(stderr, "htab_load() bad version\n");
+error_report("htab_load() bad version");
 return -EINVAL;
 }
 
@@ -1547,8 +1547,8 @@ static int htab_load(QEMUFile *f, void *opaque, int 
version_id)
 
 fd = kvmppc_get_htab_fd(true);
 if (fd < 0) {
-fprintf(stderr, "Unable to open fd to restore KVM hash table: 
%s\n",
-strerror(errno));
+error_report("Unable to open fd to restore KVM hash table: %s",
+ strerror(errno));
 }
 }
 
@@ -1568,9 +1568,9 @@ static int htab_load(QEMUFile *f, void *opaque, int 
version_id)
 if ((index + n_valid + n_invalid) >
 (HTAB_SIZE(spapr) / HASH_PTE_SIZE_64)) {
 /* Bad index in stream */
-fprintf(stderr, "htab_load() bad index %d (%hd+%hd entries) "
-"in htab stream (htab_shift=%d)\n", index, n_valid, 
n_invalid,
-spapr->htab_shift);
+error_report(
+"htab_load() bad index %d (%hd+%hd entries) in htab stream 
(htab_shift=%d)",
+index, n_valid, n_invalid, spapr->htab_shift);
 return -EINVAL;
 }
 
-- 
2.5.0

[Qemu-devel] [PATCHv4 4/8] pseries: Cleanup error handling in spapr_vga_init()

[David Gibson]

Use error_setg() to return an error rather than an explicit exit().
Previously it was an exit(0) instead of a non-zero exit code, which was
simply a bug.  Also improve the error message.

While we're at it change the type of spapr_vga_init() to bool since that's
how we're using it anyway.

Signed-off-by: David Gibson 
Reviewed-by: Thomas Huth 
---
 hw/ppc/spapr.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index fb0e254..1ce9b27 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1238,7 +1238,7 @@ static void spapr_rtc_create(sPAPRMachineState *spapr)
 }
 
 /* Returns whether we want to use VGA or not */
-static int spapr_vga_init(PCIBus *pci_bus)
+static bool spapr_vga_init(PCIBus *pci_bus, Error **errp)
 {
 switch (vga_interface_type) {
 case VGA_NONE:
@@ -1249,9 +1249,9 @@ static int spapr_vga_init(PCIBus *pci_bus)
 case VGA_VIRTIO:
 return pci_vga_init(pci_bus) != NULL;
 default:
-fprintf(stderr, "This vga model is not supported,"
-"currently it only supports -vga std\n");
-exit(0);
+error_setg(errp,
+   "Unsupported VGA mode, only -vga std or -vga virtio is 
supported");
+return false;
 }
 }
 
@@ -1926,7 +1926,7 @@ static void ppc_spapr_init(MachineState *machine)
 }
 
 /* Graphics */
-if (spapr_vga_init(phb->bus)) {
+if (spapr_vga_init(phb->bus, &error_fatal)) {
 spapr->has_graphics = true;
 machine->usb |= defaults_enabled() && !machine->usb_disabled;
 }
-- 
2.5.0

[Qemu-devel] [PATCHv4 5/8] pseries: Clean up error handling in spapr_rtas_register()

[David Gibson]

The errors detected in this function necessarily indicate bugs in the rest
of the qemu code, rather than an external or configuration problem.

So, a simple assert() is more appropriate than any more complex error
reporting.

Signed-off-by: David Gibson 
Reviewed-by: Thomas Huth 
---
 hw/ppc/spapr_rtas.c | 12 +++-
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index 34b12a3..0be52ae 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -648,17 +648,11 @@ target_ulong spapr_rtas_call(PowerPCCPU *cpu, 
sPAPRMachineState *spapr,
 
 void spapr_rtas_register(int token, const char *name, spapr_rtas_fn fn)
 {
-if (!((token >= RTAS_TOKEN_BASE) && (token < RTAS_TOKEN_MAX))) {
-fprintf(stderr, "RTAS invalid token 0x%x\n", token);
-exit(1);
-}
+assert((token >= RTAS_TOKEN_BASE) && (token < RTAS_TOKEN_MAX));
 
 token -= RTAS_TOKEN_BASE;
-if (rtas_table[token].name) {
-fprintf(stderr, "RTAS call \"%s\" is registered already as 0x%x\n",
-rtas_table[token].name, token);
-exit(1);
-}
+
+assert(!rtas_table[token].name);
 
 rtas_table[token].name = name;
 rtas_table[token].fn = fn;
-- 
2.5.0

Re: [Qemu-devel] [PATCH] migration: not send zero page header in ram bulk stage

[Li, Liang Z]

> Actually, someone has done like that before and cause a migration bug, See
> commit f1c72795af573b24a7da5eb52375c9aba8a37972, and the fixing patch is
> commit 9ef051e5536b6368a1076046ec6c4ec4ac12b5c6
> Revert "migration: do not sent zero pages in bulk stage"

Thanks for your information, I didn't notice that before. May be there is a 
workaround solution instead of reverting, I need more investigation.

Liang

Re: [Qemu-devel] [PATCH COLO-Frame v13 36/39] filter-buffer: Introduce a helper function to enable/disable default filter

[Jason Wang]

On 12/29/2015 03:09 PM, zhanghailiang wrote:
> The default buffer filter doesn't buffer packets in default,
> but we need to buffer packets for COLO or Micro-checkpoint,
> Here we add a helper function to enable/disable filter's buffer
> capability.
>
> Signed-off-by: zhanghailiang 
> Cc: Jason Wang 
> Cc: Yang Hongyang 
> ---
> v12:
> - Rename the heler function to qemu_set_default_filters_status()
> v11:
> - New patch
> ---
>  include/net/filter.h |  1 +
>  include/net/net.h|  4 
>  net/filter-buffer.c  | 19 +++
>  net/net.c| 29 +
>  4 files changed, 53 insertions(+)
>
> diff --git a/include/net/filter.h b/include/net/filter.h
> index 40aa38c..08aa604 100644
> --- a/include/net/filter.h
> +++ b/include/net/filter.h
> @@ -84,4 +84,5 @@ static inline bool qemu_need_skip_netfilter(NetFilterState 
> *nf)
>  void netdev_add_default_filter_buffer(const char *netdev_id,
>NetFilterDirection direction,
>Error **errp);
> +void qemu_set_default_filters_status(bool enable);
>  #endif /* QEMU_NET_FILTER_H */
> diff --git a/include/net/net.h b/include/net/net.h
> index 7af3e15..5c65c45 100644
> --- a/include/net/net.h
> +++ b/include/net/net.h
> @@ -125,6 +125,10 @@ NetClientState *qemu_find_vlan_client_by_name(Monitor 
> *mon, int vlan_id,
>const char *client_str);
>  typedef void (*qemu_nic_foreach)(NICState *nic, void *opaque);
>  void qemu_foreach_nic(qemu_nic_foreach func, void *opaque);
> +typedef void (*qemu_netfilter_foreach)(NetFilterState *nf, void *opaque,
> +   Error **errp);
> +void qemu_foreach_netfilter(qemu_netfilter_foreach func, void *opaque,
> +Error **errp);
>  int qemu_can_send_packet(NetClientState *nc);
>  ssize_t qemu_sendv_packet(NetClientState *nc, const struct iovec *iov,
>int iovcnt);
> diff --git a/net/filter-buffer.c b/net/filter-buffer.c
> index 8abac94..90a50cc 100644
> --- a/net/filter-buffer.c
> +++ b/net/filter-buffer.c
> @@ -169,6 +169,25 @@ out:
>  error_propagate(errp, local_err);
>  }
>  
> +static void set_default_filter_status(NetFilterState *nf,
> +  void *opaque,
> +  Error **errp)
> +{
> +if (!strcmp(object_get_typename(OBJECT(nf)), TYPE_FILTER_BUFFER)) {
> +bool *status = opaque;
> +
> +if (nf->is_default) {
> +nf->enabled = *status;
> +}
> +}
> +}
> +
> +void qemu_set_default_filters_status(bool enable)
> +{
> +qemu_foreach_netfilter(set_default_filter_status,
> +   &enable, NULL);
> +}

The name of the function sounds a generic helper but it in fact pass a
type specific function. Consider enable is a generic property of
netfilter, we want a more generic code here.

> +
>  /*
>  * This will be used by COLO or MC FT, for which they will need
>  * to buffer the packets of VM's net devices, Here we add a default
> diff --git a/net/net.c b/net/net.c
> index fd53cfc..30946c5 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -259,6 +259,35 @@ static char *assign_name(NetClientState *nc1, const char 
> *model)
>  return g_strdup_printf("%s.%d", model, id);
>  }
>  
> +void qemu_foreach_netfilter(qemu_netfilter_foreach func, void *opaque,
> +Error **errp)
> +{
> +NetClientState *nc;
> +NetFilterState *nf;
> +
> +QTAILQ_FOREACH(nc, &net_clients, next) {
> +if (nc->info->type == NET_CLIENT_OPTIONS_KIND_NIC) {
> +continue;
> +}
> +/* FIXME: Not support multiqueue */
> +if (nc->queue_index > 1) {
> +error_setg(errp, "%s: multiqueue is not supported", __func__);
> +return;
> +}

Do we really need this? Looks like netfilter_complete() has already
checked this.

> +QTAILQ_FOREACH(nf, &nc->filters, next) {
> +if (func) {
> +Error *local_err = NULL;
> +
> +func(nf, opaque, &local_err);
> +if (local_err) {
> +error_propagate(errp, local_err);
> +return;
> +}
> +}
> +}
> +}
> +}

Need a separate patch for this helper.

> +
>  static void qemu_net_client_destructor(NetClientState *nc)
>  {
>  g_free(nc);

Re: [Qemu-devel] virtio-blk-dataplane performance numbers

[Fam Zheng]

On Fri, 01/15 12:35, Diana Madalina Craciun wrote:
> Hi,
> 
> I made some measurements guest vs bare metal (using virtio-data-plane)
> and got some results I cannot fully explain.
> 
> First some details about the setup:
> 
> - I have an ARM v8 hardware + 1 SSD connected to SATA.
> 
> I have run FIO using multiple block sizes and IO depths:
> 
> for i in 1 2 4 8 16 32
> do
> for j in 4 8 16 32 64 128 256 512
> do
> echo "Test ${i}_${j}"
> fio -filename=/dev/sda1 -direct=1 -iodepth $i  -rw=write 
> -ioengine=libaio -bs=${j}k -size=8G -numjobs=4 -group_reporting
> -name=mytest_write_${i}_${j} > /dev/out_write_${i}_${j}
> done
> done
> 
> 
> I run the same script for both baremetal and guest.
> 
> QEMU (QEMU 2.4 and kernel 4.1) command line is the following:
> 
> qemu-system-aarch64 -enable-kvm -nographic -machine type=virt -cpu host
> -kernel /boot/Image -append "root=/dev/ram rw console=ttyAMA0,115200
> ramdisk_size=100" -serial tcp::,server,telnet -initrd
> /boot/rootfs.ext2.gz -m 1024 -mem-path /var/lib/hugetlbfs/pagesize-1GB
> -object iothread,id=iothread0 -drive
> if=none,id=drive0,cache=none,format=raw,file=/dev/sda,aio=native -device
> virtio-blk-pci,drive=drive0,scsi=off,iothread=iothread0
> 
> I have pinned the I/O thread to physical CPU 0 and the VCPU thread to
> physical CPU 1.
> 
> When comparing bare metal vs guest I have noticed that in some
> situations I get better results in guest. The table contains the results
> for sequential read (lines: io depth and columns: block sizes, the
> numbers are guest vs bare metal degradation percentage). For random read
> and write the results do not show large variations, but for sequential
> read and write I see important variations.
> 
>4k  8K  16K 32K 64K128K256K 512K
> 1  50.28   37.19   36.08  -0.44.09   5.183.22 1.71
> 2  46.22   22.63   24.41  -0.45   1.72   2.172.37-4.64
> 4 -10.82   15.60   11.64   5.21   0.09   2.86   -3.52 6.71
> 8 -18.05   5.968.820.26   0.95   4.30   -13.5317.9  
> 16 12.78   11.76   6.293.42   7.00   18.14  -0.4  5.59
> 32 16.99   7.984.707.67  -9.78   3.669.48-3.55   
> 
> The negative numbers may come from the benchmark variation, probably I
> would have to run the benchmark multiple times to see the variation even
> in host.
> 
> However if somebody have an explanation of why I might get better
> results in guest vs bare metal or at least in what direction to investigate.

It's probably due to request merges in virtio-blk-pci. You can collect blktrace
at host side to see - the I/O size sent to host device would be larger due to
merges.

Fam

Re: [Qemu-devel] [PATCH v2 2/4] acpi: expose oem_id and oem_table_id in build_rsdt()

[Shannon Zhao]

On 2016/1/18 22:12, Laszlo Ersek wrote:
> Since build_rsdt() is implemented as common utility code (in
> "hw/acpi/aml-build.c"), it should expose -- and forward -- the oem_id and
> oem_table_id parameters between board code and the generic build_header()
> function.
> 
> Cc: "Michael S. Tsirkin"  (supporter:ACPI/SMBIOS)
> Cc: Igor Mammedov  (supporter:ACPI/SMBIOS)
> Cc: Shannon Zhao  (maintainer:ARM ACPI Subsystem)
> Cc: Paolo Bonzini  (maintainer:X86)
> Cc: Richard W.M. Jones 
> Cc: Aleksei Kovura 
> Cc: Michael Tokarev 
> Cc: Steven Newbury 
> RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1248758
> LP: https://bugs.launchpad.net/qemu/+bug/1533848
> Signed-off-by: Laszlo Ersek 

Reviewed-by: Shannon Zhao 
> ---
> 
> Notes:
> v2:
> - no change
> 
>  include/hw/acpi/aml-build.h | 3 ++-
>  hw/acpi/aml-build.c | 5 +++--
>  hw/arm/virt-acpi-build.c| 2 +-
>  hw/i386/acpi-build.c| 2 +-
>  4 files changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
> index c460bdd..aa29d30 100644
> --- a/include/hw/acpi/aml-build.h
> +++ b/include/hw/acpi/aml-build.h
> @@ -364,6 +364,7 @@ void acpi_add_table(GArray *table_offsets, GArray 
> *table_data);
>  void acpi_build_tables_init(AcpiBuildTables *tables);
>  void acpi_build_tables_cleanup(AcpiBuildTables *tables, bool mfre);
>  void
> -build_rsdt(GArray *table_data, GArray *linker, GArray *table_offsets);
> +build_rsdt(GArray *table_data, GArray *linker, GArray *table_offsets,
> +   const char *oem_id, const char *oem_table_id);
>  
>  #endif
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index 05b8bd0..ce7fe81 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -1496,7 +1496,8 @@ void acpi_build_tables_cleanup(AcpiBuildTables *tables, 
> bool mfre)
>  
>  /* Build rsdt table */
>  void
> -build_rsdt(GArray *table_data, GArray *linker, GArray *table_offsets)
> +build_rsdt(GArray *table_data, GArray *linker, GArray *table_offsets,
> +   const char *oem_id, const char *oem_table_id)
>  {
>  AcpiRsdtDescriptorRev1 *rsdt;
>  size_t rsdt_len;
> @@ -1515,5 +1516,5 @@ build_rsdt(GArray *table_data, GArray *linker, GArray 
> *table_offsets)
> sizeof(uint32_t));
>  }
>  build_header(linker, table_data,
> - (void *)rsdt, "RSDT", rsdt_len, 1, NULL, NULL);
> + (void *)rsdt, "RSDT", rsdt_len, 1, oem_id, oem_table_id);
>  }
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 985fca4..2e90515 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -642,7 +642,7 @@ void virt_acpi_build(VirtGuestInfo *guest_info, 
> AcpiBuildTables *tables)
>  
>  /* RSDT is pointed to by RSDP */
>  rsdt = tables_blob->len;
> -build_rsdt(tables_blob, tables->linker, table_offsets);
> +build_rsdt(tables_blob, tables->linker, table_offsets, NULL, NULL);
>  
>  /* RSDP is in FSEG memory, so allocate it separately */
>  build_rsdp(tables->rsdp, tables->linker, rsdt);
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index e1ebd07..6408362 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -2705,7 +2705,7 @@ void acpi_build(PcGuestInfo *guest_info, 
> AcpiBuildTables *tables)
>  
>  /* RSDT is pointed to by RSDP */
>  rsdt = tables_blob->len;
> -build_rsdt(tables_blob, tables->linker, table_offsets);
> +build_rsdt(tables_blob, tables->linker, table_offsets, NULL, NULL);
>  
>  /* RSDP is in FSEG memory, so allocate it separately */
>  build_rsdp(tables->rsdp, tables->linker, rsdt);
> 

-- 
Shannon

Re: [Qemu-devel] [PATCH v2 1/4] acpi: take oem_id in build_header(), optionally

[Shannon Zhao]

On 2016/1/18 22:12, Laszlo Ersek wrote:
> This patch is the continuation of commit 8870ca0e94f2 ("acpi: support
> specified oem table id for build_header"). It will allow us to control the
> OEM ID field too in the SDT header.
> 
> Cc: "Michael S. Tsirkin"  (supporter:ACPI/SMBIOS)
> Cc: Igor Mammedov  (supporter:ACPI/SMBIOS)
> Cc: Xiao Guangrong  (maintainer:NVDIMM)
> Cc: Shannon Zhao  (maintainer:ARM ACPI Subsystem)
> Cc: Paolo Bonzini  (maintainer:X86)
> Cc: Richard W.M. Jones 
> Cc: Aleksei Kovura 
> Cc: Michael Tokarev 
> Cc: Steven Newbury 
> RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1248758
> LP: https://bugs.launchpad.net/qemu/+bug/1533848
> Signed-off-by: Laszlo Ersek 

Reviewed-by: Shannon Zhao 
> ---
> 
> Notes:
> v2:
> - no change
> 
>  include/hw/acpi/aml-build.h |  2 +-
>  hw/acpi/aml-build.c | 11 ---
>  hw/acpi/nvdimm.c|  4 ++--
>  hw/arm/virt-acpi-build.c| 12 ++--
>  hw/i386/acpi-build.c| 20 ++--
>  5 files changed, 27 insertions(+), 22 deletions(-)
> 
> diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
> index 6d6f705..c460bdd 100644
> --- a/include/hw/acpi/aml-build.h
> +++ b/include/hw/acpi/aml-build.h
> @@ -357,7 +357,7 @@ Aml *aml_sizeof(Aml *arg);
>  void
>  build_header(GArray *linker, GArray *table_data,
>   AcpiTableHeader *h, const char *sig, int len, uint8_t rev,
> - const char *oem_table_id);
> + const char *oem_id, const char *oem_table_id);
>  void *acpi_data_push(GArray *table_data, unsigned size);
>  unsigned acpi_data_len(GArray *table);
>  void acpi_add_table(GArray *table_offsets, GArray *table_data);
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index 78e1290..05b8bd0 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -1430,12 +1430,17 @@ Aml *aml_alias(const char *source_object, const char 
> *alias_object)
>  void
>  build_header(GArray *linker, GArray *table_data,
>   AcpiTableHeader *h, const char *sig, int len, uint8_t rev,
> - const char *oem_table_id)
> + const char *oem_id, const char *oem_table_id)
>  {
>  memcpy(&h->signature, sig, 4);
>  h->length = cpu_to_le32(len);
>  h->revision = rev;
> -memcpy(h->oem_id, ACPI_BUILD_APPNAME6, 6);
> +
> +if (oem_id) {
> +strncpy((char *)h->oem_id, oem_id, sizeof h->oem_id);
> +} else {
> +memcpy(h->oem_id, ACPI_BUILD_APPNAME6, 6);
> +}
>  
>  if (oem_table_id) {
>  strncpy((char *)h->oem_table_id, oem_table_id, 
> sizeof(h->oem_table_id));
> @@ -1510,5 +1515,5 @@ build_rsdt(GArray *table_data, GArray *linker, GArray 
> *table_offsets)
> sizeof(uint32_t));
>  }
>  build_header(linker, table_data,
> - (void *)rsdt, "RSDT", rsdt_len, 1, NULL);
> + (void *)rsdt, "RSDT", rsdt_len, 1, NULL, NULL);
>  }
> diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
> index df1b176..73749aa 100644
> --- a/hw/acpi/nvdimm.c
> +++ b/hw/acpi/nvdimm.c
> @@ -365,7 +365,7 @@ static void nvdimm_build_nfit(GSList *device_list, GArray 
> *table_offsets,
>  
>  build_header(linker, table_data,
>   (void *)(table_data->data + header), "NFIT",
> - sizeof(NvdimmNfitHeader) + structures->len, 1, NULL);
> + sizeof(NvdimmNfitHeader) + structures->len, 1, NULL, NULL);
>  g_array_free(structures, true);
>  }
>  
> @@ -470,7 +470,7 @@ static void nvdimm_build_ssdt(GSList *device_list, GArray 
> *table_offsets,
>  g_array_append_vals(table_data, ssdt->buf->data, ssdt->buf->len);
>  build_header(linker, table_data,
>  (void *)(table_data->data + table_data->len - ssdt->buf->len),
> -"SSDT", ssdt->buf->len, 1, "NVDIMM");
> +"SSDT", ssdt->buf->len, 1, NULL, "NVDIMM");
>  free_aml_allocator();
>  }
>  
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 0d5c635..985fca4 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -407,7 +407,7 @@ build_spcr(GArray *table_data, GArray *linker, 
> VirtGuestInfo *guest_info)
>  spcr->pci_vendor_id = 0x;  /* PCI Vendor ID: not a PCI device */
>  
>  build_header(linker, table_data, (void *)spcr, "SPCR", sizeof(*spcr), 2,
> - NULL);
> + NULL, NULL);
>  }
>  
>  static void
> @@ -426,7 +426,7 @@ build_mcfg(GArray *table_data, GArray *linker, 
> VirtGuestInfo *guest_info)
>  mcfg->allocation[0].end_bus_number = (memmap[VIRT_PCIE_ECAM].size
>/ PCIE_MMCFG_SIZE_MIN) - 1;
>  
> -build_header(linker, table_data, (void *)mcfg, "MCFG", len, 1, NULL);
> +build_header(linker, table_data, (void *)mcfg, "MCFG", len, 1, NULL, 
> NULL);
>  }
>  
>  /* GTDT */
> @@ -452,7 +452,7 @@ build_gtdt(GArray *table_data, GArray *linker)
>  
>  build_header(linker, table_data,
> 

Re: [Qemu-devel] [PATCH] migration: not send zero page header in ram bulk stage

[Hailiang Zhang]

On 2016/1/19 11:11, Hailiang Zhang wrote:

On 2016/1/19 9:26, Li, Liang Z wrote:

On 2016/1/15 18:24, Li, Liang Z wrote:

It seems that this patch is incorrect, if the no-zero pages are
zeroed again during !ram_bulk_stage, we didn't send the new zeroed
page, there will be an error.



If not in ram_bulk_stage, still send the header, could you explain why it's

wrong?


Liang



I have made a mistake, and yes, this patch can speed up the live migration
time, especially when there are many zero pages, it will be more obvious.
I like this idea. Did you test it with postcopy ? Does it break postcopy ?



Not yet, I saw Dave's comment's, it will beak post copy, it's not hard to fix 
this.
A more important thing is Paolo's comments, I don't know in which case this 
patch will break LM. Do you have any idea about this?
Hope that QEMU don't write data to the block 'pc.ram'.



Paolo is right, for VM in destination, QEMU may write VM's memory before VM 
starts.
So your assumption that "VM's RAM pages are initialized to zero" is incorrect.
This patch will break LM.



Actually, someone has done like that before and cause a migration bug,
See commit f1c72795af573b24a7da5eb52375c9aba8a37972, and
the fixing patch is
commit 9ef051e5536b6368a1076046ec6c4ec4ac12b5c6
Revert "migration: do not sent zero pages in bulk stage"


Liang


Thanks,
zhanghailiang



.

Re: [Qemu-devel] [PATCH COLO-Frame v13 35/39] filter-buffer: Accept zero interval

[Jason Wang]

On 12/29/2015 03:09 PM, zhanghailiang wrote:
> For default buffer filter, its 'interval' value is zero,
> so here we should accept zero interval.
>
> Signed-off-by: zhanghailiang 
> Reviewed-by: Yang Hongyang 
> Cc: Jason Wang 
> ---
> v12:
> - Add Reviewed-by tag
> v11:
> - Add comment
> v10:
> - new patch
> ---
>  net/filter-buffer.c | 10 --
>  1 file changed, 10 deletions(-)
>
> diff --git a/net/filter-buffer.c b/net/filter-buffer.c
> index 9cf3544..8abac94 100644
> --- a/net/filter-buffer.c
> +++ b/net/filter-buffer.c
> @@ -111,16 +111,6 @@ static void filter_buffer_setup(NetFilterState *nf, 
> Error **errp)
>  FilterBufferState *s = FILTER_BUFFER(nf);
>  char *path = object_get_canonical_path_component(OBJECT(nf));
>  
> -/*
> - * We may want to accept zero interval when VM FT solutions like MC
> - * or COLO use this filter to release packets on demand.
> - */

You'd better move this to the commit log for a better rationale of the
patch.

> -if (!s->interval) {
> -error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "interval",
> -   "a non-zero interval");
> -return;
> -}
> -
>  s->incoming_queue = qemu_new_net_queue(qemu_netfilter_pass_to_next, nf);
>  nf->is_default = !strcmp(path, "nop");
>  /*

Re: [Qemu-devel] [PATCH COLO-Frame v13 34/39] net/filter-buffer: Add default filter-buffer for each netdev

[Jason Wang]

On 12/29/2015 03:09 PM, zhanghailiang wrote:
> We add each netdev (except vhost-net) a default filter-buffer,
> which will be used for COLO or Micro-checkpoint to buffer VM's packets.
> The name of default filter-buffer is 'nop'.
> For the default filter-buffer, it will not buffer any packets in default.
> So it has no side effect for the netdev.
>
> Signed-off-by: zhanghailiang 
> Cc: Jason Wang 
> Cc: Yang Hongyang 

This patch did three things:

1) the ability to enable or disable a netfilter
2) the ability to add a default filter
3) default filter attaching for filter-buffer

Better to split them into separate small patches.

And several questions:

For 1), I'm not sure this is real needed, we can in fact disable a
filter by removing it.
For 2), Instead of a specific code just for filter buffer, I think we
need a generic method for an arbitrary filter to be used as default.
And if we can achieve 2), 3) is not needed any more.

> ---
> v12:
> - Skip vhost-net when add default filter
> - Don't go through filter layer if the filter is disabled.
> v11:
> - New patch
> ---
>  include/net/filter.h | 10 +++
>  net/filter-buffer.c  | 82 
> 
>  net/filter.c |  6 +++-
>  net/net.c| 12 
>  4 files changed, 109 insertions(+), 1 deletion(-)
>
> diff --git a/include/net/filter.h b/include/net/filter.h
> index 2deda36..40aa38c 100644
> --- a/include/net/filter.h
> +++ b/include/net/filter.h
> @@ -56,6 +56,8 @@ struct NetFilterState {
>  NetClientState *netdev;
>  NetFilterDirection direction;
>  char info_str[256];
> +bool is_default;
> +bool enabled;
>  QTAILQ_ENTRY(NetFilterState) next;
>  };
>  
> @@ -74,4 +76,12 @@ ssize_t qemu_netfilter_pass_to_next(NetClientState *sender,
>  int iovcnt,
>  void *opaque);
>  
> +static inline bool qemu_need_skip_netfilter(NetFilterState *nf)
> +{
> +return nf->enabled ? false : true;
> +}
> +
> +void netdev_add_default_filter_buffer(const char *netdev_id,
> +  NetFilterDirection direction,
> +  Error **errp);
>  #endif /* QEMU_NET_FILTER_H */
> diff --git a/net/filter-buffer.c b/net/filter-buffer.c
> index 57be149..9cf3544 100644
> --- a/net/filter-buffer.c
> +++ b/net/filter-buffer.c
> @@ -14,6 +14,13 @@
>  #include "qapi/qmp/qerror.h"
>  #include "qapi-visit.h"
>  #include "qom/object.h"
> +#include "net/net.h"
> +#include "qapi/qmp/qdict.h"
> +#include "qapi/qmp-output-visitor.h"
> +#include "qapi/qmp-input-visitor.h"
> +#include "monitor/monitor.h"
> +#include "qmp-commands.h"
> +#include "net/vhost_net.h"
>  
>  #define TYPE_FILTER_BUFFER "filter-buffer"
>  
> @@ -102,6 +109,7 @@ static void filter_buffer_cleanup(NetFilterState *nf)
>  static void filter_buffer_setup(NetFilterState *nf, Error **errp)
>  {
>  FilterBufferState *s = FILTER_BUFFER(nf);
> +char *path = object_get_canonical_path_component(OBJECT(nf));
>  
>  /*
>   * We may want to accept zero interval when VM FT solutions like MC
> @@ -114,6 +122,14 @@ static void filter_buffer_setup(NetFilterState *nf, 
> Error **errp)
>  }
>  
>  s->incoming_queue = qemu_new_net_queue(qemu_netfilter_pass_to_next, nf);
> +nf->is_default = !strcmp(path, "nop");
> +/*
> +* For the default buffer filter, it will be disabled by default,
> +* So it will not buffer any packets.
> +*/
> +if (nf->is_default) {
> +nf->enabled = false;
> +}
>  if (s->interval) {
>  timer_init_us(&s->release_timer, QEMU_CLOCK_VIRTUAL,
>filter_buffer_release_timer, nf);
> @@ -163,6 +179,72 @@ out:
>  error_propagate(errp, local_err);
>  }
>  
> +/*
> +* This will be used by COLO or MC FT, for which they will need
> +* to buffer the packets of VM's net devices, Here we add a default
> +* buffer filter for each netdev. The name of default buffer filter is
> +* 'nop'
> +*/
> +void netdev_add_default_filter_buffer(const char *netdev_id,
> +  NetFilterDirection direction,
> +  Error **errp)
> +{

Need a more generic way to add an arbitrary filter as default. E.g
during netdev init, query if there's a default and do the initialization
there.

> +QmpOutputVisitor *qov;
> +QmpInputVisitor *qiv;
> +Visitor *ov, *iv;
> +QObject *obj = NULL;
> +QDict *qdict;
> +void *dummy = NULL;
> +const char *id = "nop";
> +char *queue = g_strdup(NetFilterDirection_lookup[direction]);
> +NetClientState *nc = qemu_find_netdev(netdev_id);
> +Error *err = NULL;
> +
> +/* FIXME: Not support multiple queues */
> +if (!nc || nc->queue_index > 1) {
> +g_free(queue);
> +return;
> +}
> +/* Not support vhost-net */
> +if (get_vhost_net(nc)) {
> +g_free(queue);
> +return;
> +   

Re: [Qemu-devel] [PATCH] migration: not send zero page header in ram bulk stage

[Li, Liang Z]

> > Not yet, I saw Dave's comment's, it will beak post copy, it's not hard to 
> > fix
> this.
> > A more important thing is Paolo's comments, I don't know in which case
> this patch will break LM. Do you have any idea about this?
> > Hope that QEMU don't write data to the block 'pc.ram'.
> >
> 
> Paolo is right, for VM in destination, QEMU may write VM's memory before
> VM starts.
> So your assumption that "VM's RAM pages are initialized to zero" is incorrect.
> This patch will break LM.
> 

Which portion of the VM's RAM pages will be written by QEMU? Do you know some 
exact information?
I can't wait for Paolo's response.

Liang

Re: [Qemu-devel] [PATCH] migration: not send zero page header in ram bulk stage

[Hailiang Zhang]

On 2016/1/19 9:26, Li, Liang Z wrote:

On 2016/1/15 18:24, Li, Liang Z wrote:

It seems that this patch is incorrect, if the no-zero pages are
zeroed again during !ram_bulk_stage, we didn't send the new zeroed
page, there will be an error.



If not in ram_bulk_stage, still send the header, could you explain why it's

wrong?


Liang



I have made a mistake, and yes, this patch can speed up the live migration
time, especially when there are many zero pages, it will be more obvious.
I like this idea. Did you test it with postcopy ? Does it break postcopy ?



Not yet, I saw Dave's comment's, it will beak post copy, it's not hard to fix 
this.
A more important thing is Paolo's comments, I don't know in which case this 
patch will break LM. Do you have any idea about this?
Hope that QEMU don't write data to the block 'pc.ram'.



Paolo is right, for VM in destination, QEMU may write VM's memory before VM 
starts.
So your assumption that "VM's RAM pages are initialized to zero" is incorrect.
This patch will break LM.


Liang


Thanks,
zhanghailiang



.

Re: [Qemu-devel] qcow2 snapshot + resize

[lihuiba]

>I guess the only thing that would need to implement something new is
>qcow2_snapshot_goto(), which currently refuses to load a snapshot that
>has a different disk size.
>Once this is done, just removing the check in qcow2_truncate() should be
>okay.
Thanks! I'll see what I can do, later.

Re: [Qemu-devel] [Qemu-arm] [PATCH] cadence_gem: fix buffer overflow

[Jason Wang]

On 01/19/2016 12:54 AM, Alistair Francis wrote:
> On Mon, Jan 18, 2016 at 2:06 AM, Peter Maydell  
> wrote:
>> On 18 January 2016 at 09:57, Jason Wang  wrote:
>>> Thanks for the pointer.
>>>
>>> In section 16.1.5, it said
>>>
>>> "Jumbo frames are not supported."
>>>
>>> So it was in fact not an unimplemented feature?
> I'd say this should be a guest error then. Unless anyone who knows
> Ethernet well thinks otherwise?
>
> Thanks,
>
> Alistair

Yes, agree.

>
>> I have a vague feeling from the last time I looked at something like
>> this that what often happens is the hardware happily goes on dumping
>> data out on the wire but this is a violation of the ethernet protocol,
>> ie a guest error. But I'm no ethernet expert...
>>
>> thanks
>> -- PMM
>>

Re: [Qemu-devel] [Qemu-arm] [PATCH] cadence_gem: fix buffer overflow

[Jason Wang]

On 01/18/2016 06:06 PM, Peter Maydell wrote:
> On 18 January 2016 at 09:57, Jason Wang  wrote:
>> Thanks for the pointer.
>>
>> In section 16.1.5, it said
>>
>> "Jumbo frames are not supported."
>>
>> So it was in fact not an unimplemented feature?
> I have a vague feeling from the last time I looked at something like
> this that what often happens is the hardware happily goes on dumping
> data out on the wire but this is a violation of the ethernet protocol,
> ie a guest error. But I'm no ethernet expert...
>
> thanks
> -- PMM
>

I think it's a guest error (unless we find a guest/driver that depends
on this). Anyway, fixing the buffer overflow should be fine.

[Qemu-devel] [Bug 1535497] [NEW] Guest can not boot up when assigned more than 20 vcpus with option "-no-acpi"

[Robert Hu]

Public bug reported:

Environment:
 ---
 KVM commit/branch: da3f7ca3/next
 Qemu commit/branch: 7b8a354d/master
 Host OS: RHEL7.2 ia32e
 Host Kernel: 4.4-rc2
 Guest OS: RHEL7.2 ia32e

Description:
 
When assign more than 20 vpcus with option "-no-acpi", guest can not boot up.

Reproduce:
 
 1.qemu-system-x86_64 --enable-kvm -m 1024 -smp 20 -no-acpi -device 
virtio-net-pci,netdev=nic0,mac=00:16:3e:17:9b:4c -netdev 
tap,id=nic0,script=/etc/kvm/qemu-ifup -drive 
file=/root/7u2.qcow2,if=none,id=virtio-disk0 -device 
virtio-blk-pci,drive=virtio-disk0


Bisect show the bad commit is 9ee2e2625 of seabios.

** Affects: qemu
 Importance: Undecided
 Status: New

** Attachment added: "host dmesg log"
   https://bugs.launchpad.net/bugs/1535497/+attachment/4552327/+files/dmesg.log

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

Title:
  Guest can not boot up when assigned more than 20 vcpus with option
  "-no-acpi"

Status in QEMU:
  New

Bug description:
  Environment:
   ---
   KVM commit/branch: da3f7ca3/next
   Qemu commit/branch: 7b8a354d/master
   Host OS: RHEL7.2 ia32e
   Host Kernel: 4.4-rc2
   Guest OS: RHEL7.2 ia32e

  Description:
   
  When assign more than 20 vpcus with option "-no-acpi", guest can not boot up.

  Reproduce:
   
   1.qemu-system-x86_64 --enable-kvm -m 1024 -smp 20 -no-acpi -device 
virtio-net-pci,netdev=nic0,mac=00:16:3e:17:9b:4c -netdev 
tap,id=nic0,script=/etc/kvm/qemu-ifup -drive 
file=/root/7u2.qcow2,if=none,id=virtio-disk0 -device 
virtio-blk-pci,drive=virtio-disk0

  
  Bisect show the bad commit is 9ee2e2625 of seabios.

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

[Qemu-devel] [Bug 1535497] Re: Guest can not boot up when assigned more than 20 vcpus with option "-no-acpi"

[Robert Hu]

** Attachment added: "guest log"
   
https://bugs.launchpad.net/qemu/+bug/1535497/+attachment/4552328/+files/guest.log

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

Title:
  Guest can not boot up when assigned more than 20 vcpus with option
  "-no-acpi"

Status in QEMU:
  New

Bug description:
  Environment:
   ---
   KVM commit/branch: da3f7ca3/next
   Qemu commit/branch: 7b8a354d/master
   Host OS: RHEL7.2 ia32e
   Host Kernel: 4.4-rc2
   Guest OS: RHEL7.2 ia32e

  Description:
   
  When assign more than 20 vpcus with option "-no-acpi", guest can not boot up.

  Reproduce:
   
   1.qemu-system-x86_64 --enable-kvm -m 1024 -smp 20 -no-acpi -device 
virtio-net-pci,netdev=nic0,mac=00:16:3e:17:9b:4c -netdev 
tap,id=nic0,script=/etc/kvm/qemu-ifup -drive 
file=/root/7u2.qcow2,if=none,id=virtio-disk0 -device 
virtio-blk-pci,drive=virtio-disk0

  
  Bisect show the bad commit is 9ee2e2625 of seabios.

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

Re: [Qemu-devel] [PATCH 3/3] spapr: Remove abuse of rtas_ld() in h_client_architecture_support

[Alexey Kardashevskiy]

On 01/16/2016 12:14 PM, David Gibson wrote:

h_client_architecture_support() uses rtas_ld() for general purpose memory
access, despite the fact that it's not an RTAS routine at all and rtas_ld
makes things more awkward.

Clean this up by replacing rtas_ld() calls with appropriate ldXX_phys()
calls.

Signed-off-by: David Gibson 



Reviewed-by: Alexey Kardashevskiy 


---
  hw/ppc/spapr_hcall.c | 14 +++---
  1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index cebceea..9dbdba9 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -861,7 +861,8 @@ static target_ulong 
h_client_architecture_support(PowerPCCPU *cpu_,
target_ulong opcode,
target_ulong *args)
  {
-target_ulong list = args[0], ov_table;
+target_ulong list = ppc64_phys_to_real(args[0]);
+target_ulong ov_table, ov5;
  PowerPCCPUClass *pcc_ = POWERPC_CPU_GET_CLASS(cpu_);
  CPUState *cs;
  bool cpu_match = false, cpu_update = true, memory_update = false;
@@ -875,9 +876,9 @@ static target_ulong 
h_client_architecture_support(PowerPCCPU *cpu_,
  for (counter = 0; counter < 512; ++counter) {
  uint32_t pvr, pvr_mask;

-pvr_mask = rtas_ld(list, 0);
+pvr_mask = ldl_be_phys(&address_space_memory, list);
  list += 4;
-pvr = rtas_ld(list, 0);
+pvr = ldl_be_phys(&address_space_memory, list);
  list += 4;

  trace_spapr_cas_pvr_try(pvr);
@@ -948,14 +949,13 @@ static target_ulong 
h_client_architecture_support(PowerPCCPU *cpu_,
  /* For the future use: here @ov_table points to the first option vector */
  ov_table = list;

-list = cas_get_option_vector(5, ov_table);
-if (!list) {
+ov5 = cas_get_option_vector(5, ov_table);
+if (!ov5) {
  return H_SUCCESS;
  }

  /* @list now points to OV 5 */
-list += 2;
-ov5_byte2 = rtas_ld(list, 0) >> 24;
+ov5_byte2 = ldub_phys(&address_space_memory, ov5 + 2);
  if (ov5_byte2 & OV5_DRCONF_MEMORY) {
  memory_update = true;
  }




--
Alexey

Re: [Qemu-devel] [PATCH 2/3] spapr: Remove rtas_st_buffer_direct()

[Alexey Kardashevskiy]

On 01/16/2016 12:14 PM, David Gibson wrote:

rtas_st_buffer_direct() is a not particularly useful wrapper around
cpu_physical_memory_write().  All the callers are in
rtas_ibm_configure_connector, where it's better handled by local helper.

Signed-off-by: David Gibson 
---
  hw/ppc/spapr_rtas.c| 19 ---
  include/hw/ppc/spapr.h |  8 
  2 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index f4fb9ba..940509e 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -504,6 +504,13 @@ out:
  #define CC_VAL_DATA_OFFSET ((CC_IDX_PROP_DATA_OFFSET + 1) * 4)
  #define CC_WA_LEN 4096

+static void st_cc_buf(target_ulong addr, target_ulong offset,
+  void *buf, size_t len)



Can we please call it cc_st_buf() to make it clear that "cc" is a prefix 
and not about functionality?





+{
+cpu_physical_memory_write(ppc64_phys_to_real(addr + offset),
+  buf, MIN(len, CC_WA_LEN - offset));
+}
+
  static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
   sPAPRMachineState *spapr,
   uint32_t token, uint32_t nargs,
@@ -526,6 +533,7 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
  }

  wa_addr = ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 0);
+wa_addr = ppc64_phys_to_real(wa_addr);



Not needed here, st_cc_buf() also does ppc64_phys_to_real().




  drc_index = rtas_ld(wa_addr, 0);
  drc = spapr_dr_connector_by_index(drc_index);
@@ -569,8 +577,7 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
  /* provide the name of the next OF node */
  wa_offset = CC_VAL_DATA_OFFSET;
  rtas_st(wa_addr, CC_IDX_NODE_NAME_OFFSET, wa_offset);
-rtas_st_buffer_direct(wa_addr + wa_offset, CC_WA_LEN - wa_offset,
-  (uint8_t *)name, strlen(name) + 1);
+st_cc_buf(wa_addr, wa_offset, (uint8_t *)name, strlen(name) + 1);



Since you made @buf a void* in st_cc_buf(), you do not need (uint8_t*) here 
and below.





  resp = SPAPR_DR_CC_RESPONSE_NEXT_CHILD;
  break;
  case FDT_END_NODE:
@@ -595,8 +602,7 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
  /* provide the name of the next OF property */
  wa_offset = CC_VAL_DATA_OFFSET;
  rtas_st(wa_addr, CC_IDX_PROP_NAME_OFFSET, wa_offset);
-rtas_st_buffer_direct(wa_addr + wa_offset, CC_WA_LEN - wa_offset,
-  (uint8_t *)name, strlen(name) + 1);
+st_cc_buf(wa_addr, wa_offset, (uint8_t *)name, strlen(name) + 1);

  /* provide the length and value of the OF property. data gets
   * placed immediately after NULL terminator of the OF property's
@@ -605,9 +611,8 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
  wa_offset += strlen(name) + 1,
  rtas_st(wa_addr, CC_IDX_PROP_LEN, prop_len);
  rtas_st(wa_addr, CC_IDX_PROP_DATA_OFFSET, wa_offset);
-rtas_st_buffer_direct(wa_addr + wa_offset, CC_WA_LEN - wa_offset,
-  (uint8_t *)((struct fdt_property 
*)prop)->data,
-  prop_len);
+st_cc_buf(wa_addr, wa_offset,
+  (uint8_t *)((struct fdt_property *)prop)->data, 
prop_len);
  resp = SPAPR_DR_CC_RESPONSE_NEXT_PROPERTY;
  break;
  case FDT_END:
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index ec9e7ea..d6f4eb4 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -505,14 +505,6 @@ static inline void rtas_st(target_ulong phys, int n, 
uint32_t val)
  stl_be_phys(&address_space_memory, ppc64_phys_to_real(phys + 4*n), val);
  }

-static inline void rtas_st_buffer_direct(target_ulong phys,
- target_ulong phys_len,
- uint8_t *buffer, uint16_t buffer_len)
-{
-cpu_physical_memory_write(ppc64_phys_to_real(phys), buffer,
-  MIN(buffer_len, phys_len));
-}
-
  typedef void (*spapr_rtas_fn)(PowerPCCPU *cpu, sPAPRMachineState *sm,
uint32_t token,
uint32_t nargs, target_ulong args,




--
Alexey

Re: [Qemu-devel] [PATCH 1/3] spapr: Small fixes to rtas_ibm_get_system_parameter, remove rtas_st_buffer

[Alexey Kardashevskiy]

On 01/16/2016 12:14 PM, David Gibson wrote:

rtas_st_buffer() appears in spapr.h as though it were a widely used helper,
but in fact it is only used for saving data in a format used by
rtas_ibm_get_system_parameter().  We can fold it into that caller just as
simply.

While we're there fix a couple of small defects in
rtas_ibm_get_system_parameter:
   - For the string value SPLPAR_CHARACTERISTICS, it wasn't including the
 terminating \0 in the length which it should according to LoPAPR
 7.3.16.1
   - It now checks that the supplied buffer has at least enough space for
 the length of the returned data, and returns an error if it does not.

Signed-off-by: David Gibson 
---
  hw/ppc/spapr_rtas.c| 28 
  include/hw/ppc/spapr.h | 11 ---
  2 files changed, 20 insertions(+), 19 deletions(-)

diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index 34b12a3..f4fb9ba 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -235,9 +235,15 @@ static void rtas_ibm_get_system_parameter(PowerPCCPU *cpu,
uint32_t nret, target_ulong rets)
  {
  target_ulong parameter = rtas_ld(args, 0);
-target_ulong buffer = rtas_ld(args, 1);
+target_ulong buffer = ppc64_phys_to_real(rtas_ld(args, 1));
  target_ulong length = rtas_ld(args, 2);
-target_ulong ret = RTAS_OUT_SUCCESS;
+void *val;
+size_t vallen;


Having such temporary variables (val/vallen) tells me that a helper is a 
better solution as this assumes that one word and one buffer will be ever 
enough for all cases but there is already "7.3.16.17 Processor Module 
Information" coming (even though I have not heard from Sukadev for quite 
some time) which is more complicated that this.


May be rtas_st_buffer() is not the best name and it does not have to be 
global so I'd propose making it static and calling it syspar_st().




+
+if (length < 2) {
+rtas_st(rets, 0, -); /* Parameter error */



#define RTAS_OUT_SYSPAR_PARAM_ERROR -
and then
rtas_st(rets, 0, RTAS_OUT_SYSPAR_PARAM_ERROR);

?

In all other places ("rtas_st(\S\+, 0") a macro is used (except just one 
case in spapr_pci.c which needs a fix).




+return;
+}

  switch (parameter) {
  case RTAS_SYSPARM_SPLPAR_CHARACTERISTICS: {
@@ -249,24 +255,30 @@ static void rtas_ibm_get_system_parameter(PowerPCCPU *cpu,
current_machine->ram_size / M_BYTE,
smp_cpus,
max_cpus);
-rtas_st_buffer(buffer, length, (uint8_t *)param_val, 
strlen(param_val));
+val = param_val;
+vallen = strlen(param_val) + 1;
  g_free(param_val);
  break;
  }
  case RTAS_SYSPARM_DIAGNOSTICS_RUN_MODE: {
-uint8_t param_val = DIAGNOSTICS_RUN_MODE_DISABLED;
+uint8_t diagnostics_run_mode = DIAGNOSTICS_RUN_MODE_DISABLED;

-rtas_st_buffer(buffer, length, ¶m_val, sizeof(param_val));
+val = &diagnostics_run_mode;



I know that any reasonable compiler will keep @diagnostics_run_mode on 
stack till return but it is still not clean...





+vallen = sizeof(diagnostics_run_mode);
  break;
  }
  case RTAS_SYSPARM_UUID:
-rtas_st_buffer(buffer, length, qemu_uuid, (qemu_uuid_set ? 16 : 0));
+val = qemu_uuid;
+vallen = qemu_uuid_set ? 16 : 0;
  break;
  default:
-ret = RTAS_OUT_NOT_SUPPORTED;
+rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
+return;
  }

-rtas_st(rets, 0, ret);
+stw_be_phys(&address_space_memory, buffer, vallen);
+cpu_physical_memory_write(buffer + 2, val, MIN(vallen, length - 2));
+rtas_st(rets, 0, 0); /* Success */



rtas_st(rets, 0, RTAS_OUT_SUCCESS);




  }

  static void rtas_ibm_set_system_parameter(PowerPCCPU *cpu,
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 53af76a..ec9e7ea 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -513,17 +513,6 @@ static inline void rtas_st_buffer_direct(target_ulong phys,
MIN(buffer_len, phys_len));
  }

-static inline void rtas_st_buffer(target_ulong phys, target_ulong phys_len,
-  uint8_t *buffer, uint16_t buffer_len)
-{
-if (phys_len < 2) {
-return;
-}
-stw_be_phys(&address_space_memory,
-ppc64_phys_to_real(phys), buffer_len);
-rtas_st_buffer_direct(phys + 2, phys_len - 2, buffer, buffer_len);
-}
-
  typedef void (*spapr_rtas_fn)(PowerPCCPU *cpu, sPAPRMachineState *sm,
uint32_t token,
uint32_t nargs, target_ulong args,




--
Alexey

Re: [Qemu-devel] [PATCH COLO-Frame v13 34/39] net/filter-buffer: Add default filter-buffer for each netdev

[Hailiang Zhang]

ping?

The other part of this series have been fully reviewed except this net related 
part.
We hope COLO frame could be merged in qemu 2.6, so please help us.
Any comments will be welcomed, thanks.

zhanghailiang

On 2016/1/11 9:26, Hailiang Zhang wrote:

Hi Jason,

Could you please help reviewing the filter part of this series ?

Thanks,
Hailiang

On 2015/12/29 15:09, zhanghailiang wrote:

We add each netdev (except vhost-net) a default filter-buffer,
which will be used for COLO or Micro-checkpoint to buffer VM's packets.
The name of default filter-buffer is 'nop'.
For the default filter-buffer, it will not buffer any packets in default.
So it has no side effect for the netdev.

Signed-off-by: zhanghailiang 
Cc: Jason Wang 
Cc: Yang Hongyang 
---
v12:
- Skip vhost-net when add default filter
- Don't go through filter layer if the filter is disabled.
v11:
- New patch
---
  include/net/filter.h | 10 +++
  net/filter-buffer.c  | 82 
  net/filter.c |  6 +++-
  net/net.c| 12 
  4 files changed, 109 insertions(+), 1 deletion(-)

diff --git a/include/net/filter.h b/include/net/filter.h
index 2deda36..40aa38c 100644
--- a/include/net/filter.h
+++ b/include/net/filter.h
@@ -56,6 +56,8 @@ struct NetFilterState {
  NetClientState *netdev;
  NetFilterDirection direction;
  char info_str[256];
+bool is_default;
+bool enabled;
  QTAILQ_ENTRY(NetFilterState) next;
  };

@@ -74,4 +76,12 @@ ssize_t qemu_netfilter_pass_to_next(NetClientState *sender,
  int iovcnt,
  void *opaque);

+static inline bool qemu_need_skip_netfilter(NetFilterState *nf)
+{
+return nf->enabled ? false : true;
+}
+
+void netdev_add_default_filter_buffer(const char *netdev_id,
+  NetFilterDirection direction,
+  Error **errp);
  #endif /* QEMU_NET_FILTER_H */
diff --git a/net/filter-buffer.c b/net/filter-buffer.c
index 57be149..9cf3544 100644
--- a/net/filter-buffer.c
+++ b/net/filter-buffer.c
@@ -14,6 +14,13 @@
  #include "qapi/qmp/qerror.h"
  #include "qapi-visit.h"
  #include "qom/object.h"
+#include "net/net.h"
+#include "qapi/qmp/qdict.h"
+#include "qapi/qmp-output-visitor.h"
+#include "qapi/qmp-input-visitor.h"
+#include "monitor/monitor.h"
+#include "qmp-commands.h"
+#include "net/vhost_net.h"

  #define TYPE_FILTER_BUFFER "filter-buffer"

@@ -102,6 +109,7 @@ static void filter_buffer_cleanup(NetFilterState *nf)
  static void filter_buffer_setup(NetFilterState *nf, Error **errp)
  {
  FilterBufferState *s = FILTER_BUFFER(nf);
+char *path = object_get_canonical_path_component(OBJECT(nf));

  /*
   * We may want to accept zero interval when VM FT solutions like MC
@@ -114,6 +122,14 @@ static void filter_buffer_setup(NetFilterState *nf, Error 
**errp)
  }

  s->incoming_queue = qemu_new_net_queue(qemu_netfilter_pass_to_next, nf);
+nf->is_default = !strcmp(path, "nop");
+/*
+* For the default buffer filter, it will be disabled by default,
+* So it will not buffer any packets.
+*/
+if (nf->is_default) {
+nf->enabled = false;
+}
  if (s->interval) {
  timer_init_us(&s->release_timer, QEMU_CLOCK_VIRTUAL,
filter_buffer_release_timer, nf);
@@ -163,6 +179,72 @@ out:
  error_propagate(errp, local_err);
  }

+/*
+* This will be used by COLO or MC FT, for which they will need
+* to buffer the packets of VM's net devices, Here we add a default
+* buffer filter for each netdev. The name of default buffer filter is
+* 'nop'
+*/
+void netdev_add_default_filter_buffer(const char *netdev_id,
+  NetFilterDirection direction,
+  Error **errp)
+{
+QmpOutputVisitor *qov;
+QmpInputVisitor *qiv;
+Visitor *ov, *iv;
+QObject *obj = NULL;
+QDict *qdict;
+void *dummy = NULL;
+const char *id = "nop";
+char *queue = g_strdup(NetFilterDirection_lookup[direction]);
+NetClientState *nc = qemu_find_netdev(netdev_id);
+Error *err = NULL;
+
+/* FIXME: Not support multiple queues */
+if (!nc || nc->queue_index > 1) {
+g_free(queue);
+return;
+}
+/* Not support vhost-net */
+if (get_vhost_net(nc)) {
+g_free(queue);
+return;
+}
+qov = qmp_output_visitor_new();
+ov = qmp_output_get_visitor(qov);
+visit_start_struct(ov,  &dummy, NULL, NULL, 0, &err);
+if (err) {
+goto out;
+}
+visit_type_str(ov, &nc->name, "netdev", &err);
+if (err) {
+goto out;
+}
+visit_type_str(ov, &queue, "queue", &err);
+if (err) {
+goto out;
+}
+visit_end_struct(ov, &err);
+if (err) {
+goto out;
+}
+obj = qmp_output_get_qobject(qov);
+g_assert(obj != NULL);
+qdict = qobject_to_qdict(obj);
+qmp_

Re: [Qemu-devel] [PATCHv3 3/9] pseries: Clean up hash page table allocation error handling

[David Gibson]

On Mon, Jan 18, 2016 at 11:21:08AM +0100, Markus Armbruster wrote:
> Thomas Huth  writes:
> 
> > On 18.01.2016 05:24, David Gibson wrote:
> >> The spapr_alloc_htab() and spapr_reset_htab() functions currently handle
> >> all errors with error_setg(&error_abort, ...).
> >> 
> >> But really, the callers are really better placed to decide on the error
> >> handling.  So, instead make the functions use the error propagation
> >> infrastructure.
> >> 
> >> In the callers we change to &error_fatal instead of &error_abort, since
> >> this can be triggered by a bad configuration or kernel error rather than
> >> indicating a programming error in qemu.
> >> 
> >> While we're at it improve the messages themselves a bit, and clean up the
> >> indentation a little.
> >> 
> >> Signed-off-by: David Gibson 
> >> ---
> >>  hw/ppc/spapr.c | 24 
> >>  1 file changed, 16 insertions(+), 8 deletions(-)
> >> 
> >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> >> index b7fd09a..d28e349 100644
> >> --- a/hw/ppc/spapr.c
> >> +++ b/hw/ppc/spapr.c
> >> @@ -1016,7 +1016,7 @@ static void emulate_spapr_hypercall(PowerPCCPU *cpu)
> >>  #define CLEAN_HPTE(_hpte)  ((*(uint64_t *)(_hpte)) &= 
> >> tswap64(~HPTE64_V_HPTE_DIRTY))
> >>  #define DIRTY_HPTE(_hpte)  ((*(uint64_t *)(_hpte)) |= 
> >> tswap64(HPTE64_V_HPTE_DIRTY))
> >>  
> >> -static void spapr_alloc_htab(sPAPRMachineState *spapr)
> >> +static void spapr_alloc_htab(sPAPRMachineState *spapr, Error **errp)
> >>  {
> >>  long shift;
> >>  int index;
> >> @@ -1031,7 +1031,8 @@ static void spapr_alloc_htab(sPAPRMachineState 
> >> *spapr)
> >>   * For HV KVM, host kernel will return -ENOMEM when requested
> >>   * HTAB size can't be allocated.
> >>   */
> >> -error_setg(&error_abort, "Failed to allocate HTAB of requested 
> >> size, try with smaller maxmem");
> >> +error_setg_errno(errp, -shift,
> >> + "Error allocating KVM hash page table, try 
> >> smaller maxmem");
> >>  } else if (shift > 0) {
> >>  /*
> >>   * Kernel handles htab, we don't need to allocate one
> >> @@ -1040,7 +1041,10 @@ static void spapr_alloc_htab(sPAPRMachineState 
> >> *spapr)
> >>   * but we don't allow booting of such guests.
> >>   */
> >>  if (shift != spapr->htab_shift) {
> >> -error_setg(&error_abort, "Failed to allocate HTAB of 
> >> requested size, try with smaller maxmem");
> >> +error_setg(errp,
> >> +"Small allocation for KVM hash page table (%ld < %"
> >> +PRIu32 "), try smaller maxmem",
> >> +shift, spapr->htab_shift);
> >
> > Maybe you should add an "return" statement here - theoretically you do
> > not want to continue with "kvmppc_kern_htab = true" in case of errors.
> > (practically this does not happen because errp = error_fatal, but in
> > case the caller gets changed, this might introduce subtle errors otherwise)
> 
> Good point.
> 
> With abort() / exit(), we don't have to worry about recovery.  In
> particular, we don't have to revert half-done changes.
> 
> Conversions away from abort() / exit() need to consider error recovery.
> We have to make sure the function leaves things in a sane state on
> error.  This normally means taking an early return, and often means
> reverting some state changes.

That's true, but Thomas is mistaken about what error recovery is
needed here.

However, I'm going to drop this patch from the series anyway - I've
realised I need to rework the htab allocation substantially for other
reasons, so it would be better to not have that conflict with this
series.

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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCHv3 8/9] pseries: Clean up error reporting in ppc_spapr_init()

[David Gibson]

On Mon, Jan 18, 2016 at 10:31:42AM +0100, Thomas Huth wrote:
> On 18.01.2016 05:24, David Gibson wrote:
> > This function includes a number of explicit fprintf()s for errors.
> > Change these to use error_report() instead.
> > 
> > Also replace the single exit(EXIT_FAILURE) with an explicit exit(1), since
> > the latter is the more usual idiom in qemu by a large margin.
> > 
> > Signed-off-by: David Gibson 
> > ---
> >  hw/ppc/spapr.c | 25 +
> >  1 file changed, 13 insertions(+), 12 deletions(-)
> > 
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 148ca5a..58f26cd 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -1789,8 +1789,8 @@ static void ppc_spapr_init(MachineState *machine)
> >  }
> >  
> >  if (spapr->rma_size > node0_size) {
> > -fprintf(stderr, "Error: Numa node 0 has to span the RMA 
> > (%#08"HWADDR_PRIx")\n",
> > -spapr->rma_size);
> > +error_report("Numa node 0 has to span the RMA (%#08"HWADDR_PRIx")",
> > + spapr->rma_size);
> >  exit(1);
> >  }
> >  
> > @@ -1856,10 +1856,10 @@ static void ppc_spapr_init(MachineState *machine)
> >  ram_addr_t hotplug_mem_size = machine->maxram_size - 
> > machine->ram_size;
> >  
> >  if (machine->ram_slots > SPAPR_MAX_RAM_SLOTS) {
> > -error_report("Specified number of memory slots %" PRIu64
> > - " exceeds max supported %d",
> > - machine->ram_slots, SPAPR_MAX_RAM_SLOTS);
> > -exit(EXIT_FAILURE);
> > +error_report("Specified number of memory slots %"
> > + PRIu64" exceeds max supported %d",
> > +machine->ram_slots, SPAPR_MAX_RAM_SLOTS);
> 
> Why did you change the indentation of the "machine->ram_slots, ..." line
> here? The original looked better to me.

I don't know.  Probably just a mistake on my part, I'll fix it.

> > +exit(1);
> 
> EXIT_FAILURE still seems to be used quite often in the QEMU sources...
> All in all, this hunk does not really change anything from a functional
> point of view, so I'd like to suggest to omit this hunk completely
> instead to avoid code churn here.

This I'll keep, as Markus says for local consistency.

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


signature.asc
Description: PGP signature

[Qemu-devel] [PATCHv2] sysbus: Remove ignored return value of FindSysbusDeviceFunc

[David Gibson]

Functions of type FindSysbusDeviceFunc currently return an integer.
However, this return value is always ignored by the caller in
find_sysbus_device().

This changes the function type to return void, to avoid confusion over
the function semantics.

Signed-off-by: David Gibson 
---

I recently screwed up a patch because the return value led me to a
wrong guess on the semantics; let's avoid others making the same error.

Changes in v2:
 * Reworded commit message to read better without context
 * Resending with correct qemu-devel address

 hw/arm/sysbus-fdt.c| 4 ++--
 hw/core/machine.c  | 2 +-
 hw/core/platform-bus.c | 8 ++--
 hw/ppc/e500.c  | 4 +---
 hw/ppc/spapr.c | 4 +---
 include/hw/sysbus.h| 2 +-
 6 files changed, 8 insertions(+), 16 deletions(-)

diff --git a/hw/arm/sysbus-fdt.c b/hw/arm/sysbus-fdt.c
index 68a3de5..1fc03fd 100644
--- a/hw/arm/sysbus-fdt.c
+++ b/hw/arm/sysbus-fdt.c
@@ -142,7 +142,7 @@ static const NodeCreationPair add_fdt_node_functions[] = {
  * are dynamically instantiable and if so call the node creation
  * function.
  */
-static int add_fdt_node(SysBusDevice *sbdev, void *opaque)
+static void add_fdt_node(SysBusDevice *sbdev, void *opaque)
 {
 int i, ret;
 
@@ -151,7 +151,7 @@ static int add_fdt_node(SysBusDevice *sbdev, void *opaque)
 add_fdt_node_functions[i].typename)) {
 ret = add_fdt_node_functions[i].add_fdt_node_fn(sbdev, opaque);
 assert(!ret);
-return 0;
+return;
 }
 }
 error_report("Device %s can not be dynamically instantiated",
diff --git a/hw/core/machine.c b/hw/core/machine.c
index c46ddc7..792b471 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -311,7 +311,7 @@ static bool machine_get_suppress_vmdesc(Object *obj, Error 
**errp)
 return ms->suppress_vmdesc;
 }
 
-static int error_on_sysbus_device(SysBusDevice *sbdev, void *opaque)
+static void error_on_sysbus_device(SysBusDevice *sbdev, void *opaque)
 {
 error_report("Option '-device %s' cannot be handled by this machine",
  object_class_get_name(object_get_class(OBJECT(sbdev;
diff --git a/hw/core/platform-bus.c b/hw/core/platform-bus.c
index aa55d01..2578471 100644
--- a/hw/core/platform-bus.c
+++ b/hw/core/platform-bus.c
@@ -73,7 +73,7 @@ hwaddr platform_bus_get_mmio_addr(PlatformBusDevice *pbus, 
SysBusDevice *sbdev,
 return object_property_get_int(OBJECT(sbdev_mr), "addr", NULL);
 }
 
-static int platform_bus_count_irqs(SysBusDevice *sbdev, void *opaque)
+static void platform_bus_count_irqs(SysBusDevice *sbdev, void *opaque)
 {
 PlatformBusDevice *pbus = opaque;
 qemu_irq sbirq;
@@ -92,8 +92,6 @@ static int platform_bus_count_irqs(SysBusDevice *sbdev, void 
*opaque)
 }
 }
 }
-
-return 0;
 }
 
 /*
@@ -167,7 +165,7 @@ static void platform_bus_map_mmio(PlatformBusDevice *pbus, 
SysBusDevice *sbdev,
  * For each sysbus device, look for unassigned IRQ lines as well as
  * unassociated MMIO regions. Connect them to the platform bus if available.
  */
-static int link_sysbus_device(SysBusDevice *sbdev, void *opaque)
+static void link_sysbus_device(SysBusDevice *sbdev, void *opaque)
 {
 PlatformBusDevice *pbus = opaque;
 int i;
@@ -179,8 +177,6 @@ static int link_sysbus_device(SysBusDevice *sbdev, void 
*opaque)
 for (i = 0; sysbus_has_mmio(sbdev, i); i++) {
 platform_bus_map_mmio(pbus, sbdev, i);
 }
-
-return 0;
 }
 
 static void platform_bus_init_notify(Notifier *notifier, void *data)
diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index bd7da47..37d9bc1 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -195,7 +195,7 @@ static int create_devtree_etsec(SysBusDevice *sbdev, 
PlatformDevtreeData *data)
 return 0;
 }
 
-static int sysbus_device_create_devtree(SysBusDevice *sbdev, void *opaque)
+static void sysbus_device_create_devtree(SysBusDevice *sbdev, void *opaque)
 {
 PlatformDevtreeData *data = opaque;
 bool matched = false;
@@ -210,8 +210,6 @@ static int sysbus_device_create_devtree(SysBusDevice 
*sbdev, void *opaque)
  qdev_fw_name(DEVICE(sbdev)));
 exit(1);
 }
-
-return 0;
 }
 
 static void platform_bus_create_devtree(PPCE500Params *params, void *fdt,
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 50e5a26..fbd4239 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1096,7 +1096,7 @@ static void spapr_reset_htab(sPAPRMachineState *spapr)
 }
 }
 
-static int find_unknown_sysbus_device(SysBusDevice *sbdev, void *opaque)
+static void find_unknown_sysbus_device(SysBusDevice *sbdev, void *opaque)
 {
 bool matched = false;
 
@@ -1109,8 +1109,6 @@ static int find_unknown_sysbus_device(SysBusDevice 
*sbdev, void *opaque)
  qdev_fw_name(DEVICE(sbdev)));
 exit(1);
 }
-
-return 0;
 }
 
 /*
diff --git a/include/hw/sysbus.h b/include/hw/sysbus.h
index cc1dba4..263f740 100644
--- a/include/hw/sysbus.h
+++ b/include/hw/

Re: [Qemu-devel] [PATCH v16 09/14] add check reset mechanism when hotplug vfio device

[Cao jin]

On 01/18/2016 07:03 PM, Marcel Apfelbaum wrote:

On 01/12/2016 04:43 AM, Cao jin wrote:

From: Chen Fan 

Since we support multi-function hotplug. the function 0 indicate


I think you wanted , instead of ., also I would suggest
"..., function 0 indicates..."


the closure of the slot, so we have the chance to do the check.




+static int pci_bus_hotplug_notifier(PCIBus *bus, void *opaque)


Maybe  pci_bus_hotplug_notify instead of pci_bus_hotplug_notifier


Thanks,
Marcel



Sure, thanks for all the correction, will update it

--
Yours Sincerely,

Cao jin

[Qemu-devel] [PATCH v1 1/1] arm_gic: Update ID registers based on revision

[Alistair Francis]

Update the GIC ID registers (registers above 0xfe0) based on the GIC
revision instead of using the sames values for all GIC implementations.

Signed-off-by: Alistair Francis 
Tested-by: Sören Brinkmann 
---

 hw/intc/arm_gic.c | 29 ++---
 1 file changed, 26 insertions(+), 3 deletions(-)

diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
index 13e297d..f6bfa53 100644
--- a/hw/intc/arm_gic.c
+++ b/hw/intc/arm_gic.c
@@ -31,8 +31,16 @@ do { fprintf(stderr, "arm_gic: " fmt , ## __VA_ARGS__); } 
while (0)
 #define DPRINTF(fmt, ...) do {} while(0)
 #endif
 
-static const uint8_t gic_id[] = {
-0x90, 0x13, 0x04, 0x00, 0x0d, 0xf0, 0x05, 0xb1
+static const uint8_t gic_id_11mpcore[] = {
+0x00, 0x00, 0x00, 0x00, 0x90, 0x13, 0x04, 0x00, 0x0d, 0xf0, 0x05, 0xb1
+};
+
+static const uint8_t gic_id_gicv1[] = {
+0x04, 0x00, 0x00, 0x00, 0x90, 0xb3, 0x1b, 0x00, 0x0d, 0xf0, 0x05, 0xb1
+};
+
+static const uint8_t gic_id_gicv2[] = {
+0x04, 0x00, 0x00, 0x00, 0x90, 0xb4, 0x2b, 0x00, 0x0d, 0xf0, 0x05, 0xb1
 };
 
 static inline int gic_get_current_cpu(GICState *s)
@@ -689,7 +697,22 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr 
offset, MemTxAttrs attrs)
 if (offset & 3) {
 res = 0;
 } else {
-res = gic_id[(offset - 0xfe0) >> 2];
+switch (s->revision) {
+case REV_11MPCORE:
+res = gic_id_11mpcore[(offset - 0xfe0) >> 2];
+break;
+case 1:
+res = gic_id_gicv1[(offset - 0xfe0) >> 2];
+break;
+case 2:
+res = gic_id_gicv2[(offset - 0xfe0) >> 2];
+break;
+case REV_NVIC:
+/* Shouldn't be able to get here */
+abort();
+default:
+res = 0;
+}
 }
 }
 return res;
-- 
2.5.0

Re: [Qemu-devel] [PATCH v6 0/6] Xen PCI passthru: Convert to realize()

[Cao jin]

Seems I forgot to CC Eric, but I checked my command line history, I do 
include Eric...weird


On 01/17/2016 08:13 PM, Cao jin wrote:

v6 changelog:
1. split modification of xen_host_pci_sysfs_path() into a separate new patch
as 1/6 shows.
2. 'bug' fix of qemu_strtoul(), in patch 2/6 & 3/6
3. Grammar fix in patch 4/6
4. 'msg' --> 'message' in commit message.

Cao jin (6):
   Change xen_host_pci_sysfs_path() to return void
   Xen: use qemu_strtoul instead of strtol
   Add Error **errp for xen_host_pci_device_get()
   Add Error **errp for xen_pt_setup_vga()
   Add Error **errp for xen_pt_config_init()
   Xen PCI passthru: convert to realize()

  hw/xen/xen-host-pci-device.c | 149 +--
  hw/xen/xen-host-pci-device.h |   5 +-
  hw/xen/xen_pt.c  |  77 --
  hw/xen/xen_pt.h  |   5 +-
  hw/xen/xen_pt_config_init.c  |  51 ---
  hw/xen/xen_pt_graphics.c |  11 ++--
  6 files changed, 155 insertions(+), 143 deletions(-)



--
Yours Sincerely,

Cao jin

Re: [Qemu-devel] [PATCH v1 1/1] arm_gic: Include the GIC ArchRev in the ICPIDR2 register

[Alistair Francis]

On Mon, Jan 18, 2016 at 2:13 PM, Peter Maydell  wrote:
> On 18 January 2016 at 21:41, Alistair Francis
>  wrote:
>> On Mon, Jan 11, 2016 at 5:58 AM, Peter Maydell  
>> wrote:
>>> The current implementation of the ID registers seems to be
>>> simply "like the 11MPCore interrupt controller". I think we
>>> should get them right more generally if we're going to fix them:
>>>
>>>fd0 .. fdc fe0 .. fec  ff0 ... ffc
>>> 11MPCore   reserved   90 13 04 00 0d f0 05 b1
>>> ARM GICv1 (eg A9)  04 00 00 0090 b3 1b 00 0d f0 05 b1
>>> ARM GICv2 (eg A15) 04 00 00 0090 b4 2b 00 0d f0 05 b1
>>>
>>> Your patch doesn't account for ICPIDR1 also having a field that
>>> changes between GICv1 and GICv2 (for ARM implementations), and
>>> we're missing the fd0..fdc bytes.
>>>
>>> I think this is probably simplest modelled with several
>>> gic_id arrays and using the appropriate one for 11MPCore/GICv1/GICv2,
>>> rather than trying to OR extra values into the 11MPCore ID values.
>>
>> Ok, just to make sure I am reading this right. You think I should just
>> create three arrays and then if() the revision to determine which one
>> to use?
>
> Yes, something like that. The fd0..fdc being reserved for 11MPCore
> is documented to mean RAZ/WI, so you can just make those array elements
> zeroes.

Ok, sounds good to me. I'm about to send a patch out. As basically the
whole patch and title changed it is back to V1.

Thanks,

Alistair

>
> thanks
> -- PMM
>

Re: [Qemu-devel] [PATCH v14 0/8] Block replication for continuous checkpoints

[Hailiang Zhang]

ping...

It seems that there is no feedback for a long time, we hope COLO prototype could
be merged in QEMU 2.6, it depends on this series, so please help us.

Thanks.
zhanghailiang

On 2016/1/14 9:12, Changlong Xie wrote:

It seems i missed someone in CC list, add them.

Thanks
 -Xie

On 01/13/2016 05:18 PM, Changlong Xie wrote:

Block replication is a very important feature which is used for
continuous checkpoints(for example: COLO).

You can get the detailed information about block replication from here:
http://wiki.qemu.org/Features/BlockReplication

Usage:
Please refer to docs/block-replication.txt

This patch series is based on the following patch series:
1. http://lists.nongnu.org/archive/html/qemu-devel/2015-12/msg04570.html

You can get the patch here:
https://github.com/Pating/qemu/tree/changlox/block-replication-v14

You can get the patch with framework here:
https://github.com/Pating/qemu/tree/changlox/colo_framework_v13

TODO:
1. Continuous block replication. It will be started after basic functions
are accepted.

Changs Log:
V14:
1. Implement auto complete active commit
2. Implement active commit block job for replication.c
3. Address the comments from Stefan, add replication-specific API and data
structure, also remove old block layer APIs
V13:
1. Rebase to the newest codes
2. Remove redundant marcos and semicolon in replication.c
3. Fix typos in block-replication.txt
V12:
1. Rebase to the newest codes
2. Use backing reference to replcace 'allow-write-backing-file'
V11:
1. Reopen the backing file when starting blcok replication if it is not
opened in R/W mode
2. Unblock BLOCK_OP_TYPE_BACKUP_SOURCE and BLOCK_OP_TYPE_BACKUP_TARGET
when opening backing file
3. Block the top BDS so there is only one block job for the top BDS and
its backing chain.
V10:
1. Use blockdev-remove-medium and blockdev-insert-medium to replace backing
reference.
2. Address the comments from Eric Blake
V9:
1. Update the error messages
2. Rebase to the newest qemu
3. Split child add/delete support. These patches are sent in another patchset.
V8:
1. Address Alberto Garcia's comments
V7:
1. Implement adding/removing quorum child. Remove the option non-connect.
2. Simplify the backing refrence option according to Stefan Hajnoczi's 
suggestion
V6:
1. Rebase to the newest qemu.
V5:
1. Address the comments from Gong Lei
2. Speed the failover up. The secondary vm can take over very quickly even
if there are too many I/O requests.
V4:
1. Introduce a new driver replication to avoid touch nbd and qcow2.
V3:
1: use error_setg() instead of error_set()
2. Add a new block job API
3. Active disk, hidden disk and nbd target uses the same AioContext
4. Add a testcase to test new hbitmap API
V2:
1. Redesign the secondary qemu(use image-fleecing)
2. Use Error objects to return error message
3. Address the comments from Max Reitz and Eric Blake

Wen Congyang (8):
   unblock backup operations in backing file
   Store parent BDS in BdrvChild
   Backup: clear all bitmap when doing block checkpoint
   Allow creating backup jobs when opening BDS
   docs: block replication's description
   auto complete active commit
   Implement new driver for block replication
   support replication driver in blockdev-add

  block.c  |  19 ++
  block/Makefile.objs  |   3 +-
  block/backup.c   |  14 +
  block/mirror.c   |  13 +-
  block/replication-comm.c |  66 +
  block/replication.c  | 590 +++
  blockdev.c   |   2 +-
  blockjob.c   |  11 +
  docs/block-replication.txt   | 229 +++
  include/block/block_int.h|   4 +-
  include/block/blockjob.h |  12 +
  include/block/replication-comm.h |  50 
  qapi/block-core.json |  33 ++-
  qemu-img.c   |   2 +-
  14 files changed, 1038 insertions(+), 10 deletions(-)
  create mode 100644 block/replication-comm.c
  create mode 100644 block/replication.c
  create mode 100644 docs/block-replication.txt
  create mode 100644 include/block/replication-comm.h





.

Re: [Qemu-devel] [PATCH] migration: not send zero page header in ram bulk stage

[Li, Liang Z]

> On 2016/1/15 18:24, Li, Liang Z wrote:
> >> It seems that this patch is incorrect, if the no-zero pages are
> >> zeroed again during !ram_bulk_stage, we didn't send the new zeroed
> >> page, there will be an error.
> >>
> >
> > If not in ram_bulk_stage, still send the header, could you explain why it's
> wrong?
> >
> > Liang
> >
> 
> I have made a mistake, and yes, this patch can speed up the live migration
> time, especially when there are many zero pages, it will be more obvious.
> I like this idea. Did you test it with postcopy ? Does it break postcopy ?
> 

Not yet, I saw Dave's comment's, it will beak post copy, it's not hard to fix 
this. 
A more important thing is Paolo's comments, I don't know in which case this 
patch will break LM. Do you have any idea about this? 
Hope that QEMU don't write data to the block 'pc.ram'.

Liang

> Thanks,
> zhanghailiang
> 

Re: [Qemu-devel] CMSG_SPACE() causing compile time error on Mac OS X

[Programmingkid]

On Jan 18, 2016, at 5:09 PM, Peter Maydell wrote:

> On 18 January 2016 at 21:09, Programmingkid  wrote:
>> 
>> On Jan 18, 2016, at 3:49 PM, Peter Maydell wrote:
>>> Can you say what 'gcc --version' prints for you? That will
>>> tell us the clang version number, which is more interesting
>>> than what clang claims its gcc-compatibility is.
>> 
>> $ gcc-4.9 --version
>> gcc-4.9 (Homebrew gcc49 4.9.2_1) 4.9.2
>> Copyright (C) 2014 Free Software Foundation, Inc.
>> This is free software; see the source for copying conditions.  There is NO
>> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
> 
> Ah, I misread your earlier message and I thought you said that
> gcc-4.9 worked ok and it was plain 'gcc' that caused the warning.
> Is that expansion of the macro the one produced by gcc-4.9, then?

Actually it should have but wasn't. It is of gcc 4.2.1.

Here is gcc 4.9.2's output:

char control[(((__darwin_size_t)((char *)(__darwin_size_t)(sizeof(struct 
cmsghdr)) + (sizeof(__uint32_t) - 1)) &~ (sizeof(__uint32_t) - 1)) + 
((__darwin_size_t)((char *)(__darwin_size_t)(sizeof(int) * 16) + 
(sizeof(__uint32_t) - 1)) &~ (sizeof(__uint32_t) - 1)))] = { 0 };

This is gcc 4.2.1's output:

char control[(((__darwin_size_t)((char *)(__darwin_size_t)(sizeof(struct 
cmsghdr)) + (sizeof(__uint32_t) - 1)) &~ (sizeof(__uint32_t) - 1)) + 
((__darwin_size_t)((char *)(__darwin_size_t)(sizeof(int) * 16) + 
(sizeof(__uint32_t) - 1)) &~ (sizeof(__uint32_t) - 1)))] = { 0 };

They appear to be identical. 

> 
> Also, you're right that we should figure out why it seems to
> be using the wrong C compiler. Can you use 'V=1' to look at
> what command line make is using to compile this file, please?
> (you'll probably want to dump the whole make output to a file
> and then search through it).

It says gcc-4.9 for the compiler. I wonder if the wrong header file is being 
used somewhere. 

> Also:
> *  what does your config-host.mak say (in particular about what
> it's set CC, HOST_CC, CXX, etc to)?

CC=gcc-4.9
HOST_CC=cc
CXX=gcc-4.9
CPP=gcc-4.9 -E
OBJCC=gcc-4.9

> * can you check your environment doesn't have CC set to anything?
> ("env | grep CC" should tell you if it is)

Nothing is returned.

Re: [Qemu-devel] [PATCH 1/7] target-ppc: kvm: fix floating point registers sync on little-endian hosts

[David Gibson]

On Mon, Jan 18, 2016 at 09:51:56AM +0100, Greg Kurz wrote:
> On Mon, 18 Jan 2016 13:16:44 +1100
> David Gibson  wrote:
> 
> > On Fri, Jan 15, 2016 at 04:00:12PM +0100, Greg Kurz wrote:
> > > On VSX capable CPUs, the 32 FP registers are mapped to the high-bits
> > > of the 32 first VSX registers. So if you have:
> > > 
> > > VSR31 = (uint128) 0x0102030405060708090a0b0c0d0e0f00
> > > 
> > > then
> > > 
> > > FPR31 = (uint64) 0x0102030405060708
> > > 
> > > The kernel stores the VSX registers in the fp_state struct following the
> > > host endian element ordering.
> > > 
> > > On big-endian:
> > > 
> > > fp_state.fpr[31][0] = 0x0102030405060708
> > > fp_state.fpr[31][1] = 0x090a0b0c0d0e0f00
> > > 
> > > On little-endian:
> > > 
> > > fp_state.fpr[31][0] = 0x090a0b0c0d0e0f00
> > > fp_state.fpr[31][1] = 0x0102030405060708
> > > 
> > > The KVM_GET_ONE_REG and KVM_SET_ONE_REG ioctls preserve this ordering, but
> > > QEMU considers it as big-endian and always copies element [0] to the
> > > fpr[] array and element [1] to the vsr[] array. This does not work with
> > > little-endian hosts, and you will get:
> > > 
> > > (qemu) p $f31
> > > 0x90a0b0c0d0e0f00
> > > 
> > > instead of:
> > > 
> > > (qemu) p $f31
> > > 0x102030405060708
> > > 
> > > This patch fixes the element ordering for little-endian hosts.
> > > 
> > > Signed-off-by: Greg Kurz   
> > 
> > If I'm understanding correctly, the only reason this bug didn't affect
> > things other than the gdbstub is because the get and put routines had
> 
> Well it is not only gdbstub actually... as showed in the changelog, it also
> affects the QEMU monitor which outputs wrong values since it calls 
> kvm_get_fpu()
> as well.

Yes, sorry, I didn't express that well.  My point is that the only
reason things aren't going horribly wrong is that qemu is only ever
touching the FP/VSX values for debug, and the get/put into KVM is
wrong in such a way that the right values go back again as long as
qemu doesn't try to change them.

> > mirrored bugs.  So although qemu ended up with definitely wrong
> > information in its internal state, it reshuffled it to be right on
> > setting it back into KVM.
> > 
> > Is that correct?
> > 
> 
> My guess is that the bug only affects gdbstub and ppc_cpu_dump_state(), 
> because
> these are the only cases where QEMU parses the state of FP registers... this
> is indeed confirmed by the KVM bug you are referring to, that had no visible
> effect for more than a year BTW.

Ok.

Still waiting for a reply for my query on 5/7, then I'm happy to apply
these.

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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH] sysbus: Remove ignored return value of FindSysbusDeviceFunc

[David Gibson]

On Mon, Jan 18, 2016 at 06:45:14PM +0100, Andreas Färber wrote:
> Am 18.01.2016 um 05:39 schrieb David Gibson:
> > Functions of type FindSysbusDeviceFunc currently return an integer.  I
> > recently made an error in a patch because I assumed that this return value
> > would control whether iteration of the function across devices continues
> > or not.  In fact, the function's return value is always ignored.
> > 
> > This changes the function type to return void, so that others don't make
> > the same mistake.
> 
> Have you considered implementing the behavior you expected? :)
> Not necessary for your use case or too complicated?

A bit of both.

> > Signed-off-by: David Gibson 
> > ---
> > 
> > Please apply.
> 
> Patch looks okay, too short notice for today's pull though.
> Usually we avoid "I" in a commit message.

True, that probably won't make so much sense sitting in the commit
history.  I'll reword and resend.

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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCHv3 3/9] pseries: Clean up hash page table allocation error handling

[David Gibson]

On Mon, Jan 18, 2016 at 09:47:59AM +0100, Thomas Huth wrote:
> On 18.01.2016 05:24, David Gibson wrote:
> > The spapr_alloc_htab() and spapr_reset_htab() functions currently handle
> > all errors with error_setg(&error_abort, ...).
> > 
> > But really, the callers are really better placed to decide on the error
> > handling.  So, instead make the functions use the error propagation
> > infrastructure.
> > 
> > In the callers we change to &error_fatal instead of &error_abort, since
> > this can be triggered by a bad configuration or kernel error rather than
> > indicating a programming error in qemu.
> > 
> > While we're at it improve the messages themselves a bit, and clean up the
> > indentation a little.
> > 
> > Signed-off-by: David Gibson 
> > ---
> >  hw/ppc/spapr.c | 24 
> >  1 file changed, 16 insertions(+), 8 deletions(-)
> > 
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index b7fd09a..d28e349 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -1016,7 +1016,7 @@ static void emulate_spapr_hypercall(PowerPCCPU *cpu)
> >  #define CLEAN_HPTE(_hpte)  ((*(uint64_t *)(_hpte)) &= 
> > tswap64(~HPTE64_V_HPTE_DIRTY))
> >  #define DIRTY_HPTE(_hpte)  ((*(uint64_t *)(_hpte)) |= 
> > tswap64(HPTE64_V_HPTE_DIRTY))
> >  
> > -static void spapr_alloc_htab(sPAPRMachineState *spapr)
> > +static void spapr_alloc_htab(sPAPRMachineState *spapr, Error **errp)
> >  {
> >  long shift;
> >  int index;
> > @@ -1031,7 +1031,8 @@ static void spapr_alloc_htab(sPAPRMachineState *spapr)
> >   * For HV KVM, host kernel will return -ENOMEM when requested
> >   * HTAB size can't be allocated.
> >   */
> > -error_setg(&error_abort, "Failed to allocate HTAB of requested 
> > size, try with smaller maxmem");
> > +error_setg_errno(errp, -shift,
> > + "Error allocating KVM hash page table, try 
> > smaller maxmem");
> >  } else if (shift > 0) {
> >  /*
> >   * Kernel handles htab, we don't need to allocate one
> > @@ -1040,7 +1041,10 @@ static void spapr_alloc_htab(sPAPRMachineState 
> > *spapr)
> >   * but we don't allow booting of such guests.
> >   */
> >  if (shift != spapr->htab_shift) {
> > -error_setg(&error_abort, "Failed to allocate HTAB of requested 
> > size, try with smaller maxmem");
> > +error_setg(errp,
> > +"Small allocation for KVM hash page table (%ld < %"
> > +PRIu32 "), try smaller maxmem",
> > +shift, spapr->htab_shift);
> 
> Maybe you should add an "return" statement here - theoretically you do
> not want to continue with "kvmppc_kern_htab = true" in case of errors.
> (practically this does not happen because errp = error_fatal, but in
> case the caller gets changed, this might introduce subtle errors
> otherwise)

No, actually.  If the error is non-fatal, then we *must* set
kvmppc_kern_htab = true.  It is possible we can continue without the
size of hash table we wanted - we did so until pretty recently.  But
it *is* still a kernel provided hash table, and must be marked as such
to operate correctly.

> 
> >  }
> >  
> >  spapr->htab_shift = shift;
> > @@ -1064,17 +1068,21 @@ static void spapr_alloc_htab(sPAPRMachineState 
> > *spapr)
> >   * If host kernel has allocated HTAB, KVM_PPC_ALLOCATE_HTAB ioctl is
> >   * used to clear HTAB. Otherwise QEMU-allocated HTAB is cleared manually.
> >   */
> > -static void spapr_reset_htab(sPAPRMachineState *spapr)
> > +static void spapr_reset_htab(sPAPRMachineState *spapr, Error **errp)
> >  {
> >  long shift;
> >  int index;
> >  
> >  shift = kvmppc_reset_htab(spapr->htab_shift);
> >  if (shift < 0) {
> > -error_setg(&error_abort, "Failed to reset HTAB");
> > +error_setg_errno(errp, -shift,
> > +   "Error resetting KVM hash page table, try smaller 
> > maxmem");
> 
> dito, better do an "return" here...

No.  The remaining statement in the function could be relevant if
we're somehow able to keep going here.

> >  } else if (shift > 0) {
> >  if (shift != spapr->htab_shift) {
> > -error_setg(&error_abort, "Requested HTAB allocation failed 
> > during reset");
> > +error_setg(errp,
> > +"Reduced size on reset of KVM hash page table (%ld < %"
> > +PRIu32 "), try smaller maxmem",
> > +shift, spapr->htab_shift);
> 
> ... and here.

Hrm.. here, yes we would be in trouble, but 'return' wouldn't help in
the slightest.  Instead we'd need to change spapr->htab_shift to have
any hope of continuing.

I'll make that change.

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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCHv3 6/9] pseries: Clean up error handling in spapr_rtas_register()

[David Gibson]

On Mon, Jan 18, 2016 at 10:20:24AM +0100, Thomas Huth wrote:
> On 18.01.2016 05:24, David Gibson wrote:
> > The errors detected in this function necessarily indicate bugs in the rest
> > of the qemu code, rather than an external or configuration problem.
> > 
> > So, a simple assert() is more appropriate than any more complex error
> > reporting.
> > 
> > Signed-off-by: David Gibson 
> > ---
> >  hw/ppc/spapr_rtas.c | 12 +++-
> >  1 file changed, 3 insertions(+), 9 deletions(-)
> > 
> > diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> > index 34b12a3..0be52ae 100644
> > --- a/hw/ppc/spapr_rtas.c
> > +++ b/hw/ppc/spapr_rtas.c
> > @@ -648,17 +648,11 @@ target_ulong spapr_rtas_call(PowerPCCPU *cpu, 
> > sPAPRMachineState *spapr,
> >  
> >  void spapr_rtas_register(int token, const char *name, spapr_rtas_fn fn)
> >  {
> > -if (!((token >= RTAS_TOKEN_BASE) && (token < RTAS_TOKEN_MAX))) {
> > -fprintf(stderr, "RTAS invalid token 0x%x\n", token);
> > -exit(1);
> > -}
> > +assert((token >= RTAS_TOKEN_BASE) && (token < RTAS_TOKEN_MAX));
> 
> While you're at it, you could also get rid of some superfluous
> parentheses in that statement:
> 
>   assert(token >= RTAS_TOKEN_BASE && token < RTAS_TOKEN_MAX);

I could, but I won't because I think it's clearer as it is.

> >  token -= RTAS_TOKEN_BASE;
> > -if (rtas_table[token].name) {
> > -fprintf(stderr, "RTAS call \"%s\" is registered already as 0x%x\n",
> > -rtas_table[token].name, token);
> > -exit(1);
> > -}
> > +
> > +assert(!rtas_table[token].name);
> >  
> >  rtas_table[token].name = name;
> >  rtas_table[token].fn = fn;
> > 
> 
> Anyway, patch sounds reasonable,
> Reviewed-by: Thomas Huth 
> 

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


signature.asc
Description: PGP signature

Re: [Qemu-devel] [Qemu-ppc] [PATCH 2/4] target-ppc: use cpu_write_xer() helper in cpu_post_load

[David Gibson]

On Mon, Jan 18, 2016 at 08:31:02AM +, Mark Cave-Ayland wrote:
> On 18/01/16 03:12, David Gibson wrote:
> 
> > On Fri, Jan 08, 2016 at 01:25:32PM +1100, Alexey Kardashevskiy wrote:
> >> On 01/07/2016 05:22 AM, Mark Cave-Ayland wrote:
> >>> Otherwise some internal xer variables fail to get set post-migration.
> >>>
> >>> Signed-off-by: Mark Cave-Ayland 
> >>> ---
> >>>  target-ppc/machine.c |2 +-
> >>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/target-ppc/machine.c b/target-ppc/machine.c
> >>> index 98fc63a..322ce84 100644
> >>> --- a/target-ppc/machine.c
> >>> +++ b/target-ppc/machine.c
> >>> @@ -168,7 +168,7 @@ static int cpu_post_load(void *opaque, int version_id)
> >>>  env->spr[SPR_PVR] = env->spr_cb[SPR_PVR].default_value;
> >>>  env->lr = env->spr[SPR_LR];
> >>>  env->ctr = env->spr[SPR_CTR];
> >>> -env->xer = env->spr[SPR_XER];
> >>> +cpu_write_xer(env, env->spr[SPR_XER]);
> >>>  #if defined(TARGET_PPC64)
> >>>  env->cfar = env->spr[SPR_CFAR];
> >>>  #endif
> >>>
> >>
> >> Reviewed-by: Alexey Kardashevskiy  
> > 
> > I've merged just this patch into ppc-for-2.6.  The others in the
> > series have some problems which have been pointed out elsewhere.
> 
> Thanks David.
> 
> Any chance of being able to merge the macio VMStateDescription patchset
> too at
> https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg00558.html?
> (this was sent without an explicit CC as I didn't know you were still
> handling ppc-next).

*digs through mail archive*

Ok, done.

One small nit, as noted in the discussion 2/4 introduces a migration
breakage.  That's ok, since migration was broken beforehand anyway,
but I think it would make sense to set the minimum_version_id to
document the fact that you're dropping compatibility with all older
versions.

I'm happy for that to be a followup change though.


> With the macio fixes in place then Mac machine migration should start to
> work once the respin of this series is applied.
> 
> 
> Many thanks,
> 
> Mark.
> 

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


signature.asc
Description: PGP signature

Re: [Qemu-devel] usb-storage assertions

[Andrey Korolyov]

On Mon, Jan 18, 2016 at 4:55 PM, Gerd Hoffmann  wrote:
>   Hi,
>
>> > ok.  Had no trouble with freebsd, will go fetch netbsd images.  What
>> > arch is this?  i386?  x86_64?
>>
>> i386 7.0 for the reference, but I`m sure that this wouldn`t matter in
>> any way.
>
> 7.0 trace:

Whoops, sorry, should be 5.1/i386. On a 7.0 I observe same endless
loop as you do.

>
> [ ... ]
> 12417@1453119296.506252:usb_ehci_opreg_write wr mmio 0020 [USBCMD] = 0
> 12417@1453119296.506284:usb_ehci_queue_action q 0x7f94f7a98bb0: free
> [ ... ]
> 12417@1453119296.507336:usb_ehci_state periodic schedule INACTIVE
> 12417@1453119296.507340:usb_ehci_usbsts usbsts PSS 0
> 12417@1453119296.507344:usb_ehci_queue_action q 0x7f94f7a98cd0: free
> 12417@1453119296.507349:usb_ehci_queue_action q 0x7f94f7a98c40: free
> 12417@1453119296.507353:usb_ehci_queue_action q 0x7f94f749f040: free
> 12417@1453119296.507357:usb_ehci_queue_action q 0x7f94f749efb0: free
> 12417@1453119296.507361:usb_ehci_state async schedule INACTIVE
> 12417@1453119296.507365:usb_ehci_usbsts usbsts ASS 0
> 12417@1453119296.507369:usb_ehci_usbsts usbsts HALT 1
> 12417@1453119296.507402:usb_ehci_opreg_write wr mmio 0020 [USBCMD] = 2
> 12417@1453119296.507413:usb_ehci_reset === RESET ===
> 12417@1453119296.507418:usb_ehci_port_detach detach port #0, owner ehci
> 12417@1453119296.507423:usb_ehci_irq level 1, frindex 0x2958, sts
> 0x1004, mask 0x37
> 12417@1453119296.507435:usb_ehci_port_attach attach port #0, owner comp,
> device QEMU USB MSD
> 12417@1453119296.507444:usb_ehci_opreg_change ch mmio 0020 [USBCMD] =
> 8 (old: 0)
> 12417@1453119296.507466:usb_ehci_opreg_read rd mmio 0024 [USBSTS] = 1000
> 12417@1453119296.507510:usb_ehci_opreg_read rd mmio 0024 [USBSTS] = 1000
> [ ... ]
>
> So, to shutdown ehci netbsd clears the cmd register, then sets the reset
> bit in the cmd register.  Fine.
>
> Then it goes read the status register, in a loop, forever.  No idea why,
> and I also can't spot then place in the source code.  Hmm ...
>
>>  Just to mention - ancient 2.6, like 2.6.18, are actually
>> doing things quite faster using same frontend+backend combination, may
>> be due to lack of proper timeout checks... Actually there is a very
>> small chance that the real performance regression was introduced
>> during further development, so I instead believe in improper
>> interaction of a newer guest EHCI driver and qemu frontend.
>
> Could also be older ehci guest drivers took a shortcut which turned out
> to not be correct and not working reliable ...
>
>> Please let
>> me know if any countable measurements like fio could be a matter of
>> interest - I don`t think that many people are concerned about USB/USB2
>> frontend performance at all, since they are bringing in a ton of
>> unwelcomed wakeups and the one thing which could be a matter of
>> concern in that case is an emulated xHCI, IMHO.
>
> Yes, xhci is clearly the best choice when it comes to performance.
>
> Reasons to use ehci instead basically boils down to (a) lacking/broken
> guest drivers (old windows versions, also early linux xhci driver
> versions had endian issues, firmware needs xhci support too to boot from
> usb) and (b) historical (ehci emulation was there first, so support in
> the management stack tends to be better for that, also people are used
> to it).
>
> cheers,
>   Gerd
>

[Qemu-devel] [PATCH] linux-user,ppc: synchronize syscall_nr.h

[Laurent Vivier]

Synchronize with include/uapi/asm/unistd.h from kernel v4.4

This allows to use timerfd_create().

Signed-off-by: Laurent Vivier 
---
 linux-user/ppc/syscall_nr.h | 14 +-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/linux-user/ppc/syscall_nr.h b/linux-user/ppc/syscall_nr.h
index 1e1736e..7382294 100644
--- a/linux-user/ppc/syscall_nr.h
+++ b/linux-user/ppc/syscall_nr.h
@@ -319,7 +319,7 @@
 #define TARGET_NR_epoll_pwait  303
 #define TARGET_NR_utimensat304
 #define TARGET_NR_signalfd 305
-#define TARGET_NR_timerfd  306
+#define TARGET_NR_timerfd_create306
 #define TARGET_NR_eventfd  307
 #define TARGET_NR_sync_file_range2 308
 #define TARGET_NR_fallocate309
@@ -368,3 +368,15 @@
 #define TARGET_NR_process_vm_writev 352
 #define TARGET_NR_finit_module  353
 #define TARGET_NR_kcmp  354
+#define TARGET_NR_sched_setattr 355
+#define TARGET_NR_sched_getattr 356
+#define TARGET_NR_renameat2 357
+#define TARGET_NR_seccomp   358
+#define TARGET_NR_getrandom 359
+#define TARGET_NR_memfd_create  360
+#define TARGET_NR_bpf   361
+#define TARGET_NR_execveat  362
+#define TARGET_NR_switch_endian 363
+#define TARGET_NR_userfaultfd   364
+#define TARGET_NR_membarrier365
+#define TARGET_NR_mlock2378
-- 
2.5.0

[Qemu-devel] [RE-RESEND PATCH] pci: Adjust PCI config limit based on bus topology

[Alex Williamson]

A conventional PCI bus does not support config space accesses above
the standard 256 byte configuration space.  PCIe-to-PCI bridges are
not permitted to forward transactions if the extended register address
field is non-zero and must handle it as an unsupported request (PCIe
bridge spec rev 1.0, 4.1.3, 4.1.4).  Therefore, we should not support
extended config space if there is a conventional bus anywhere on the
path to a device.

Signed-off-by: Alex Williamson 
---
Previous postings:
https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg05384.html
https://lists.gnu.org/archive/html/qemu-devel/2015-11/msg02422.html

 hw/pci/pci_host.c |   26 ++
 1 file changed, 26 insertions(+)

diff --git a/hw/pci/pci_host.c b/hw/pci/pci_host.c
index 49f59a5..3a3e294 100644
--- a/hw/pci/pci_host.c
+++ b/hw/pci/pci_host.c
@@ -19,6 +19,7 @@
  */
 
 #include "hw/pci/pci.h"
+#include "hw/pci/pci_bridge.h"
 #include "hw/pci/pci_host.h"
 #include "hw/pci/pci_bus.h"
 #include "trace.h"
@@ -49,9 +50,29 @@ static inline PCIDevice *pci_dev_find_by_addr(PCIBus *bus, 
uint32_t addr)
 return pci_find_device(bus, bus_num, devfn);
 }
 
+static void pci_adjust_config_limit(PCIBus *bus, uint32_t *limit)
+{
+if (*limit > PCI_CONFIG_SPACE_SIZE) {
+if (!pci_bus_is_express(bus)) {
+*limit = PCI_CONFIG_SPACE_SIZE;
+return;
+}
+
+if (!pci_bus_is_root(bus)) {
+PCIDevice *bridge = pci_bridge_get_device(bus);
+pci_adjust_config_limit(bridge->bus, limit);
+}
+}
+}
+
 void pci_host_config_write_common(PCIDevice *pci_dev, uint32_t addr,
   uint32_t limit, uint32_t val, uint32_t len)
 {
+pci_adjust_config_limit(pci_dev->bus, &limit);
+if (limit <= addr) {
+return;
+}
+
 assert(len <= 4);
 /* non-zero functions are only exposed when function 0 is present,
  * allowing direct removal of unexposed functions.
@@ -70,6 +91,11 @@ uint32_t pci_host_config_read_common(PCIDevice *pci_dev, 
uint32_t addr,
 {
 uint32_t ret;
 
+pci_adjust_config_limit(pci_dev->bus, &limit);
+if (limit <= addr) {
+return ~0x0;
+}
+
 assert(len <= 4);
 /* non-zero functions are only exposed when function 0 is present,
  * allowing direct removal of unexposed functions.

[Qemu-devel] [PATCH] linux-user: fix realloc size of target_fd_trans.

[Laurent Vivier]

target_fd_trans is an array of "TargetFdTrans *": compute size
accordingly. Use g_renew() as proposed by Paolo.

Reported-by: Paolo Bonzini 
Signed-off-by: Laurent Vivier 
---
 linux-user/syscall.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 0cbace4..fd04e5f 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -330,8 +330,8 @@ static void fd_trans_register(int fd, TargetFdTrans *trans)
 if (fd >= target_fd_max) {
 oldmax = target_fd_max;
 target_fd_max = ((fd >> 6) + 1) << 6; /* by slice of 64 entries */
-target_fd_trans = g_realloc(target_fd_trans,
-target_fd_max * sizeof(TargetFdTrans));
+target_fd_trans = g_renew(TargetFdTrans *,
+  target_fd_trans, target_fd_max);
 memset((void *)(target_fd_trans + oldmax), 0,
(target_fd_max - oldmax) * sizeof(TargetFdTrans *));
 }
-- 
2.5.0

Re: [Qemu-devel] [PATCH v1 1/1] arm_gic: Include the GIC ArchRev in the ICPIDR2 register

[Peter Maydell]

On 18 January 2016 at 21:41, Alistair Francis
 wrote:
> On Mon, Jan 11, 2016 at 5:58 AM, Peter Maydell  
> wrote:
>> The current implementation of the ID registers seems to be
>> simply "like the 11MPCore interrupt controller". I think we
>> should get them right more generally if we're going to fix them:
>>
>>fd0 .. fdc fe0 .. fec  ff0 ... ffc
>> 11MPCore   reserved   90 13 04 00 0d f0 05 b1
>> ARM GICv1 (eg A9)  04 00 00 0090 b3 1b 00 0d f0 05 b1
>> ARM GICv2 (eg A15) 04 00 00 0090 b4 2b 00 0d f0 05 b1
>>
>> Your patch doesn't account for ICPIDR1 also having a field that
>> changes between GICv1 and GICv2 (for ARM implementations), and
>> we're missing the fd0..fdc bytes.
>>
>> I think this is probably simplest modelled with several
>> gic_id arrays and using the appropriate one for 11MPCore/GICv1/GICv2,
>> rather than trying to OR extra values into the 11MPCore ID values.
>
> Ok, just to make sure I am reading this right. You think I should just
> create three arrays and then if() the revision to determine which one
> to use?

Yes, something like that. The fd0..fdc being reserved for 11MPCore
is documented to mean RAZ/WI, so you can just make those array elements
zeroes.

thanks
-- PMM

Re: [Qemu-devel] CMSG_SPACE() causing compile time error on Mac OS X

[Peter Maydell]

On 18 January 2016 at 21:09, Programmingkid  wrote:
>
> On Jan 18, 2016, at 3:49 PM, Peter Maydell wrote:
>> Can you say what 'gcc --version' prints for you? That will
>> tell us the clang version number, which is more interesting
>> than what clang claims its gcc-compatibility is.
>
> $ gcc-4.9 --version
> gcc-4.9 (Homebrew gcc49 4.9.2_1) 4.9.2
> Copyright (C) 2014 Free Software Foundation, Inc.
> This is free software; see the source for copying conditions.  There is NO
> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

Ah, I misread your earlier message and I thought you said that
gcc-4.9 worked ok and it was plain 'gcc' that caused the warning.
Is that expansion of the macro the one produced by gcc-4.9, then?

Also, you're right that we should figure out why it seems to
be using the wrong C compiler. Can you use 'V=1' to look at
what command line make is using to compile this file, please?
(you'll probably want to dump the whole make output to a file
and then search through it). Also:
*  what does your config-host.mak say (in particular about what
 it's set CC, HOST_CC, CXX, etc to)?
* can you check your environment doesn't have CC set to anything?
("env | grep CC" should tell you if it is)

thanks
-- PMM

Re: [Qemu-devel] [PATCH v1 08/17] target-arm: cpu: Move cpu_is_big_endian to header

[Alistair Francis]

On Sun, Jan 17, 2016 at 11:12 PM, Peter Crosthwaite
 wrote:
> From: Peter Crosthwaite 
>
> There is a CPU data endianness test that is used to drive the
> virtio_big_endian test.
>
> Move this up to the header so it can be more generally used for endian
> tests. The KVM specific cpu_syncronize_state call is left behind in the
> virtio specific function.
>
> Signed-off-by: Peter Crosthwaite 

Reviewed-by: Alistair Francis 

Also, I'm not too sure what the appropriate action is here, but this
is still signed off by your Xilinx address.

Thanks,

Alistair

> ---
>
>  target-arm/cpu.c | 19 +++
>  target-arm/cpu.h | 19 +++
>  2 files changed, 22 insertions(+), 16 deletions(-)
>
> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> index 35a1f12..d3b73bf 100644
> --- a/target-arm/cpu.c
> +++ b/target-arm/cpu.c
> @@ -368,26 +368,13 @@ static void arm_cpu_kvm_set_irq(void *opaque, int irq, 
> int level)
>  #endif
>  }
>
> -static bool arm_cpu_is_big_endian(CPUState *cs)
> +static bool arm_cpu_virtio_is_big_endian(CPUState *cs)
>  {
>  ARMCPU *cpu = ARM_CPU(cs);
>  CPUARMState *env = &cpu->env;
> -int cur_el;
>
>  cpu_synchronize_state(cs);
> -
> -/* In 32bit guest endianness is determined by looking at CPSR's E bit */
> -if (!is_a64(env)) {
> -return (env->uncached_cpsr & CPSR_E) ? 1 : 0;
> -}
> -
> -cur_el = arm_current_el(env);
> -
> -if (cur_el == 0) {
> -return (env->cp15.sctlr_el[1] & SCTLR_E0E) != 0;
> -}
> -
> -return (env->cp15.sctlr_el[cur_el] & SCTLR_EE) != 0;
> +return arm_cpu_is_big_endian(env);
>  }
>
>  #endif
> @@ -1420,7 +1407,7 @@ static void arm_cpu_class_init(ObjectClass *oc, void 
> *data)
>  cc->do_unaligned_access = arm_cpu_do_unaligned_access;
>  cc->get_phys_page_debug = arm_cpu_get_phys_page_debug;
>  cc->vmsd = &vmstate_arm_cpu;
> -cc->virtio_is_big_endian = arm_cpu_is_big_endian;
> +cc->virtio_is_big_endian = arm_cpu_virtio_is_big_endian;
>  #endif
>  cc->gdb_num_core_regs = 26;
>  cc->gdb_core_xml_file = "arm-core.xml";
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index f83070a..54675c7 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -1795,6 +1795,25 @@ static inline bool arm_singlestep_active(CPUARMState 
> *env)
>  && arm_generate_debug_exceptions(env);
>  }
>
> +/* Return true if the processor is in big-endian mode. */
> +static bool arm_cpu_is_big_endian(CPUARMState *env)
> +{
> +int cur_el;
> +
> +/* In 32bit endianness is determined by looking at CPSR's E bit */
> +if (!is_a64(env)) {
> +return (env->uncached_cpsr & CPSR_E) ? 1 : 0;
> +}
> +
> +cur_el = arm_current_el(env);
> +
> +if (cur_el == 0) {
> +return (env->cp15.sctlr_el[1] & SCTLR_E0E) != 0;
> +}
> +
> +return (env->cp15.sctlr_el[cur_el] & SCTLR_EE) != 0;
> +}
> +
>  #include "exec/cpu-all.h"
>
>  /* Bit usage in the TB flags field: bit 31 indicates whether we are
> --
> 1.9.1
>
>

Re: [Qemu-devel] [PATCH v1 1/1] arm_gic: Include the GIC ArchRev in the ICPIDR2 register

[Alistair Francis]

On Mon, Jan 11, 2016 at 5:58 AM, Peter Maydell  wrote:
> On 8 January 2016 at 18:57, Alistair Francis
>  wrote:
>> The ARM GIC documentation (page 4-119) describes that bits
>> 7 to 4 of the ICPIDR2 register should include the GIC architecture
>> version. This patche ORs the version into the existing return value.
>>
>> Signed-off-by: Alistair Francis 
>> Tested-by: Sören Brinkmann 
>> ---
>>
>>  hw/intc/arm_gic.c | 4 
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
>> index 13e297d..f47d606 100644
>> --- a/hw/intc/arm_gic.c
>> +++ b/hw/intc/arm_gic.c
>> @@ -688,6 +688,10 @@ static uint32_t gic_dist_readb(void *opaque, hwaddr 
>> offset, MemTxAttrs attrs)
>>  } else /* offset >= 0xfe0 */ {
>>  if (offset & 3) {
>>  res = 0;
>> +} else if (offset == 0xfe8 && s->revision != REV_11MPCORE &&
>> +  s->revision != REV_NVIC) {
>> +/* ICPIDR2 includes the GICv1 or GICv2 version information */
>> +res = gic_id[(offset - 0xfe0) >> 2] | (s->revision << 4);
>>  } else {
>>  res = gic_id[(offset - 0xfe0) >> 2];
>>  }
>
> The current implementation of the ID registers seems to be
> simply "like the 11MPCore interrupt controller". I think we
> should get them right more generally if we're going to fix them:
>
>fd0 .. fdc fe0 .. fec  ff0 ... ffc
> 11MPCore   reserved   90 13 04 00 0d f0 05 b1
> ARM GICv1 (eg A9)  04 00 00 0090 b3 1b 00 0d f0 05 b1
> ARM GICv2 (eg A15) 04 00 00 0090 b4 2b 00 0d f0 05 b1
>
> Your patch doesn't account for ICPIDR1 also having a field that
> changes between GICv1 and GICv2 (for ARM implementations), and
> we're missing the fd0..fdc bytes.
>
> I think this is probably simplest modelled with several
> gic_id arrays and using the appropriate one for 11MPCore/GICv1/GICv2,
> rather than trying to OR extra values into the 11MPCore ID values.

Ok, just to make sure I am reading this right. You think I should just
create three arrays and then if() the revision to determine which one
to use?

Thanks,

Alistair

>
> For the NVIC these registers don't exist at all, and the
> construction of the memory regions in armv7m_nvic.c ensures
> we can't reach this bit of the function, so we don't need
> to specially handle it. (Also there's a rewrite of the NVIC
> to disentangle it from the GIC which hopefully will land
> some time before 2.6.)
>
> thanks
> -- PMM
>

Re: [Qemu-devel] [PATCH] vhost-user: Slave crashes as Master unmaps vrings during guest reboot

[shesha Sreenivasamurthy (shesha)]

Got it. Thanks, I missed that line while reading the spec. Is 
docs/specs/vhost-user.txt the official spec ?

--
- Thanks
char * (*shesha) (uint64_t cache, uint8_t F00D)
{ return 0xC0DE; }

From: "Michael S. Tsirkin" mailto:m...@redhat.com>>
Date: Sunday, January 17, 2016 at 3:23 AM
To: Cisco Employee mailto:she...@cisco.com>>
Cc: "qemu-devel@nongnu.org" 
mailto:qemu-devel@nongnu.org>>
Subject: Re: [PATCH] vhost-user: Slave crashes as Master unmaps vrings during 
guest reboot

On Fri, Jan 15, 2016 at 12:12:43PM -0800, Shesha Sreenivasamurthy wrote:
Problem:

If a guest has vhost-user enabled, then on reboot vhost_virtqueue_stop
is invoked. This unmaps vring memory mappings. However, it will not give
any indication to the underlying DPDK slave application about it.
Therefore, a pollmode DPDK driver tries to read the ring to check for
packets and segfaults.

The spec currently says:
Client must start ring upon receiving a kick (that is, detecting that file
descriptor is readable) on the descriptor specified by
VHOST_USER_SET_VRING_KICK, and stop ring upon receiving
VHOST_USER_GET_VRING_BASE.

Why isn't this sufficient?

Solution:
--
VHOST_USER_RESET_OWNER API is issued by QEMU so that DPDK slave
application is informed that mappings will be soon gone so that
it can take necessary steps.
Shesha Sreenivasamurthy (1):
   vhost-user: Slave crashes as Master unmaps vrings during guest reboot
  hw/virtio/vhost.c | 5 +
  1 file changed, 5 insertions(+)
--
1.9.5 (Apple Git-50.3)

Re: [Qemu-devel] [PATCH] cpu: Clean up includes

[Eric Blake]

On 01/18/2016 11:05 AM, Peter Maydell wrote:

>>> +++ b/qom/cpu.c
>>> @@ -18,6 +18,7 @@
>>>   * 
>>>   */
>>>
>>> +#include "qemu/osdep.h"
>>>  #include "qemu-common.h"
>>
>> Shouldn't qemu-common.h include osdep.h?
> 
> It does, but the intention is that every .c file should include
> qemu/osdep.h as its first include (even if some other include
> it has also results in osdep.h being pulled in).

Eventually, we want to force that NO .h file includes qemu/osdep.h.  If
every .c file includes it first, then all other .h files can count on it
already being included and therefore don't need to include it
themselves.  (This is comparable to the rule used in other projects,
like libvirt's handling of config.h which must be first in all .c files
and must not be included in .h files).

> This is a simple
> rule that's easy to check in code review and hopefully also
> in an automated way.

Indeed - we MUST turn on automation after all these individual patches
are in, to make sure we don't regress (again, something that libvirt has
already managed to do, so it shouldn't be too hard to automate, except
that libvirt's automation is courtesy of gnulib which we don't use here).

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature