Re: [Qemu-devel] [PATCH for-2.3] powerpc: fix -machine usb=no for newworld and pseries machines

[Paolo Bonzini]

On 23/03/2015 23:00, Marcel Apfelbaum wrote:
 I know it comes to solve a bug, but we talked about it in another mail
 thread and this change in semantics was approved.

I forgot to reply to this---my understanding is that it was okay for the
sake of your patch series, but it would be fixed before 2.3.

 Let me explain *why* I don't like it.
 1. We add an "usb_disabled" field to a base class (actually object)
 of all the machines and the only place it is interesting is
 for 2 machines on ppc.
>>
>> So we do for kernel_irqchip_requested/allowed.  Both approaches could be
>> replaced by a tri-state on/off/auto.
> Personally I prefer this one, but out of the scope of this patch.

Yes, that was my rationale as well.

 2. Even for these 2 machines, the scenario of defaults=on and usb=off
 is not practical.
>>
>> Why?  For example you could add a virtio-input device instead of a USB
>> keyboard and mouse.
> You got me there :)
> From what I understood for those boards there is no need for this
> combination but I don't know them enough (OK.. at all).

Well, you can always find a reason.  USB is a good default, but it
doesn't have to be the only one.  You might even be okay with USB, but
prefer a different host controller.

> Bottom line, of course I don't have anything against fixing this bug,
> my problem was only with the way we add those fields (usb_disabled),
> maybe a three state QOM property (and variable behind it) is a
> solution, but not for now of course.

I think the QOM property should not be tristate, only the variable.
Another possibility is backing "xyz" with a bool xyz, but adding a
bool xyz_set.

Then irqchip_required = irqchip_set && irqchip, and irqchip_allowed =
!irqchip_set || irqchip.

Paolo

> I also didn't like the required/allowed fields and I added them anyway...
> 
> Thanks,
> Marcel
> 
> 
>>
>> Paolo
>>
> 
> 
> 

Re: [Qemu-devel] [Qemu-ppc] [PATCH v2 2/2] i6300esb: Fix signed integer overflow

[Paolo Bonzini]

On 24/03/2015 01:22, David Gibson wrote:
> On Mon, Mar 23, 2015 at 10:54:39AM +0100, BALATON Zoltan wrote:
>> On Mon, 23 Mar 2015, David Gibson wrote:
>>> If the guest programs a sufficiently large timeout value an
>>> integer overflow can occur in i6300esb_restart_timer().  e.g.
>>> if the maximum possible timer preload value of 0xf is
>>> programmed then we end up with the calculation:
>>> 
>>> timeout = get_ticks_per_sec() * (0xf << 15) / 3300;
>>> 
>>> get_ticks_per_sec() returns 10 (10^9) giving:
>>> 
>>> 10^9 * (0xf * 2^15) == 0x1dcd632329b00 (65 bits)
>>> 
>>> Obviously the division by 33MHz brings it back under 64-bits,
>>> but the overflow has already occurred.
>>> 
>>> Since signed integer overflow has undefined behaviour in C, in
>>> theory this could be arbitrarily bad.  In practice, the
>>> overflowed value wraps around to something negative, causing
>>> the watchdog to immediately expire, killing the guest, which is
>>> still fairly bad.
>>> 
>>> The bug can be triggered by running a Linux guest, loading the
>>> i6300esb driver with parameter "heartbeat=2046" and opening
>>> /dev/watchdog.  The watchdog will trigger as soon as the device
>>> is opened.
>>> 
>>> This patch corrects the problem by using muldiv64(), which
>>> effectively allows a 128-bit intermediate value between the
>>> multiplication and division.
>>> 
>>> Signed-off-by: David Gibson  --- 
>>> hw/watchdog/wdt_i6300esb.c | 10 -- 1 file changed, 8
>>> insertions(+), 2 deletions(-)
>>> 
>>> diff --git a/hw/watchdog/wdt_i6300esb.c
>>> b/hw/watchdog/wdt_i6300esb.c index e694fa9..c7316f5 100644 ---
>>> a/hw/watchdog/wdt_i6300esb.c +++ b/hw/watchdog/wdt_i6300esb.c 
>>> @@ -125,8 +125,14 @@ static void
>>> i6300esb_restart_timer(I6300State *d, int stage) else timeout
>>> <<= 5;
>>> 
>>> -/* Get the timeout in units of ticks_per_sec. */ -
>>> timeout = get_ticks_per_sec() * timeout / 3300; +/* Get
>>> the timeout in units of ticks_per_sec. + * + *
>>> ticks_per_sec is typically 10^9 == 0x3B9ACA00 (30 bits), with +
>>> * 20 bits of user supplied preload, and 15 bits of scale, the +
>>> * multiply here can exceed 64-bits, before we divide by 33MHz,
>>> so + * we use a 128-bit temporary + */
>> 
>> Is the comment still correct saying "we use a 128-bit temporary"
>> when the code does not do that explicitely any more?
> 
> Bother.  I fixed the commit message, but not this comment.  It's
> still kind of correct, in that muldiv64 does effectively have a
> 128-bit temporary internally.

Yes, that's how I interpreted it.  Though strictly speaking it's
96-bit.  I'll change it to "higher-precision intermediate value".

Paolo

Re: [Qemu-devel] [PATCH RFC for-2.3 1/1] block: New command line option --no-format-probing

[Paolo Bonzini]

On 23/03/2015 21:19, Markus Armbruster wrote:
> Paolo Bonzini  writes:
> 
>> On 23/03/2015 18:48, Eric Blake wrote:
 Why can't libvirt just add ,format=raw instead of leaving out the
 format key altogether?
>>>
>>> Libvirt DOES add format=raw.  This patch is an extra insurance
>>> policy to guarantee that libvirt does not have any code paths that
>>> omit the explicit format (as we have had a couple of CVEs in
>>> libvirt over the years where that was the case).
>>
>> And where's the extra insurance policy to guarantee that QEMU does not
>> have any code paths that ignore the new command line option?
> 
> Right here (in the non-RFC patch, not this one):
> 
> @@ -751,6 +752,11 @@ static int find_image_format(BlockDriverState *bs, const 
> char *filename,
>  return ret;
>  }
>  
> +if (bdrv_image_probing_disabled) {
> +error_setg(errp, "Format not specified and image probing disabled");
> +return -EINVAL;
> +}
> +
>  ret = bdrv_pread(bs, 0, buf, sizeof(buf));
>  if (ret < 0) {
>  error_setg_errno(errp, -ret, "Could not read image for determining 
> its "
> 
> The option sets bdrv_image_probing_disabled in a straightforward manner,
> and bdrv_image_probing_disabled guards the probing code in an equally
> straightforward manner.

But what about migration from newer to older QEMU?  Libvirt even
supports QEMU versions where the only way to specify disks is "-hda
XYZ", so it is _impossible_ to honor the format=raw specifier.

Also, libvirt can start qemu-nbd and doesn't force format=raw in that
case.  So the protection is far from complete.  This reinforces my
opinion that the false sense of safety provided by this patch is worse
than the "insurance" against future CVEs (also, have there been any
actual libvirt CVEs about this after 2010?  near misses don't count IMHO).

> How to get p0wned anyway:
> 
> 1. Use your raw image with format=raw.  Malicious guest writes qcow2
> header.
> 
> 2. Use the image again without a format.  Probe returns qcow2, no
> warning is printed.
> 
> Plausible attack?  Maybe not.  Worth stopping cold anyway?  I think so,
> as long as it's easy.
> 
> My new option is a different kind of mitigation.  It's for users who
> want more airtight protection, prefer writes to sector 0 just work,
> funny or not, and are willing to always specify the format.  At least
> one such user exists: libvirt.
> 
> Without this patch:
> 
> * Alternate use of raw images with and without format is still insecure;
>   Kevin's mitigation can't protect you then.

That's PEBKAC.

Paolo

> * If you somehow miss specifying a format, and probing detects raw, you
>   get a warning on stderr.  If your guest writes something funny to
>   sector 0, the write fails.
> 
> With this patch:
> 
> * If you somehow miss specifying a format, open fails.  This is what
>   libvirt wants.
> 
>>   There certainly hasn't been enough discussion
>> for this to get into 2.3.
> 
> Isn't that what were doing now?  :)
> 
> 

[Qemu-devel] One question to lowlevel/xl/xl.c and lowlevel/xc/xc.c

[Chen, Tiejun]

All guys,

Sorry to bother you.

I have a question to two files, tools/python/xen/lowlevel/xc/xc.c and 
tools/python/xen/lowlevel/xl/xl.c. Who is a caller to those methods like 
pyxc_methods[] and pyxl_methods[]? And how should we call these approaches?


In my specific case, I'm trying to introduce a new flag to each a device 
while assigning device. So this means I have to add a parameter, 'flag', 
into


int xc_assign_device(
xc_interface *xch,
uint32_t domid,
uint32_t machine_sbdf)

Then this is extended as

int xc_assign_device(
xc_interface *xch,
uint32_t domid,
uint32_t machine_sbdf,
uint32_t flag)

After this introduction, obviously I should cover all cases using 
xc_assign_device(). And also I found this fallout goes into these two 
files. For example, here pyxc_assign_device() is involved. Currently it 
has two parameters, 'dom' and 'pci_str', and as I understand 'pci_str' 
should represent all pci devices with SBDF format, right?


But I don't know exactly what rule should be complied to construct this 
sort of flag into 'pci_str', or any reasonable idea to achieve my goal?


Thanks
Tiejun

[Qemu-devel] [PULL 1/4] target-tricore: Fix two helper functions (clang warnings)

[Bastian Koppelmann]

From: Stefan Weil 

clang report:

target-tricore/op_helper.c:1247:24: warning:
  taking the absolute value of unsigned type 'uint32_t' (aka 'unsigned int')
  has no effect [-Wabsolute-value]
target-tricore/op_helper.c:1248:25: warning:
  taking the absolute value of unsigned type 'uint32_t' (aka 'unsigned int')
  has no effect [-Wabsolute-value]
target-tricore/op_helper.c:1249:19: warning:
  taking the absolute value of unsigned type 'uint32_t' (aka 'unsigned int')
  has no effect [-Wabsolute-value]
target-tricore/op_helper.c:1297:24: warning:
  taking the absolute value of unsigned type 'uint32_t' (aka 'unsigned int')
  has no effect [-Wabsolute-value]
target-tricore/op_helper.c:1298:25: warning:
  taking the absolute value of unsigned type 'uint32_t' (aka 'unsigned int')
  has no effect [-Wabsolute-value]
target-tricore/op_helper.c:1299:19: warning:
  taking the absolute value of unsigned type 'uint32_t' (aka 'unsigned int')
  has no effect [-Wabsolute-value]

Fix also the divisor which was taken from the wrong register
(thanks to Peter Maydell for this hint).

Cc: Bastian Koppelmann 
Signed-off-by: Stefan Weil 
Message-Id: <1425739412-8144-1-git-send-email...@weilnetz.de>
Signed-off-by: Bastian Koppelmann 
---
 target-tricore/op_helper.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/target-tricore/op_helper.c b/target-tricore/op_helper.c
index 97b0c8b..2cfa95d 100644
--- a/target-tricore/op_helper.c
+++ b/target-tricore/op_helper.c
@@ -1953,9 +1953,9 @@ uint64_t helper_dvinit_b_13(CPUTriCoreState *env, 
uint32_t r1, uint32_t r2)
 quotient_sign = 1;
 }
 
-abs_sig_dividend = abs(r1) >> 7;
-abs_base_dividend = abs(r1) & 0x7f;
-abs_divisor = abs(r1);
+abs_sig_dividend = abs((int32_t)r1) >> 7;
+abs_base_dividend = abs((int32_t)r1) & 0x7f;
+abs_divisor = abs((int32_t)r2);
 /* calc overflow */
 env->PSW_USB_V = 0;
 if ((quotient_sign) && (abs_divisor)) {
@@ -2003,9 +2003,9 @@ uint64_t helper_dvinit_h_13(CPUTriCoreState *env, 
uint32_t r1, uint32_t r2)
 quotient_sign = 1;
 }
 
-abs_sig_dividend = abs(r1) >> 7;
-abs_base_dividend = abs(r1) & 0x7f;
-abs_divisor = abs(r1);
+abs_sig_dividend = abs((int32_t)r1) >> 7;
+abs_base_dividend = abs((int32_t)r1) & 0x7f;
+abs_divisor = abs((int32_t)r2);
 /* calc overflow */
 env->PSW_USB_V = 0;
 if ((quotient_sign) && (abs_divisor)) {
-- 
2.3.3

[Qemu-devel] [PULL 4/4] target-tricore: properly fix dvinit_b/h_13

[Bastian Koppelmann]

The TriCore documentation was wrong on how to calculate ovf bits for those two
instructions, which I confirmed with real hardware (TC1796 chip). An ovf
actually happens, if the result (without remainder) does not fit into 8/16 bits.

Signed-off-by: Bastian Koppelmann 
---
 target-tricore/op_helper.c | 40 ++--
 1 file changed, 10 insertions(+), 30 deletions(-)

diff --git a/target-tricore/op_helper.c b/target-tricore/op_helper.c
index 2cfa95d..220ec4a 100644
--- a/target-tricore/op_helper.c
+++ b/target-tricore/op_helper.c
@@ -1942,29 +1942,19 @@ uint64_t helper_unpack(target_ulong arg1)
 uint64_t helper_dvinit_b_13(CPUTriCoreState *env, uint32_t r1, uint32_t r2)
 {
 uint64_t ret;
-int32_t abs_sig_dividend, abs_base_dividend, abs_divisor;
-int32_t quotient_sign;
+int32_t abs_sig_dividend, abs_divisor;
 
 ret = sextract32(r1, 0, 32);
 ret = ret << 24;
-quotient_sign = 0;
 if (!((r1 & 0x8000) == (r2 & 0x8000))) {
 ret |= 0xff;
-quotient_sign = 1;
 }
 
-abs_sig_dividend = abs((int32_t)r1) >> 7;
-abs_base_dividend = abs((int32_t)r1) & 0x7f;
+abs_sig_dividend = abs((int32_t)r1) >> 8;
 abs_divisor = abs((int32_t)r2);
-/* calc overflow */
-env->PSW_USB_V = 0;
-if ((quotient_sign) && (abs_divisor)) {
-env->PSW_USB_V = (((abs_sig_dividend == abs_divisor) &&
- (abs_base_dividend >= abs_divisor)) ||
- (abs_sig_dividend > abs_divisor));
-} else {
-env->PSW_USB_V = (abs_sig_dividend >= abs_divisor);
-}
+/* calc overflow
+   ofv if (a/b >= 255) <=> (a/255 >= b) */
+env->PSW_USB_V = (abs_sig_dividend >= abs_divisor) << 31;
 env->PSW_USB_V = env->PSW_USB_V << 31;
 env->PSW_USB_SV |= env->PSW_USB_V;
 env->PSW_USB_AV = 0;
@@ -1992,29 +1982,19 @@ uint64_t helper_dvinit_b_131(CPUTriCoreState *env, 
uint32_t r1, uint32_t r2)
 uint64_t helper_dvinit_h_13(CPUTriCoreState *env, uint32_t r1, uint32_t r2)
 {
 uint64_t ret;
-int32_t abs_sig_dividend, abs_base_dividend, abs_divisor;
-int32_t quotient_sign;
+int32_t abs_sig_dividend, abs_divisor;
 
 ret = sextract32(r1, 0, 32);
 ret = ret << 16;
-quotient_sign = 0;
 if (!((r1 & 0x8000) == (r2 & 0x8000))) {
 ret |= 0x;
-quotient_sign = 1;
 }
 
-abs_sig_dividend = abs((int32_t)r1) >> 7;
-abs_base_dividend = abs((int32_t)r1) & 0x7f;
+abs_sig_dividend = abs((int32_t)r1) >> 16;
 abs_divisor = abs((int32_t)r2);
-/* calc overflow */
-env->PSW_USB_V = 0;
-if ((quotient_sign) && (abs_divisor)) {
-env->PSW_USB_V = (((abs_sig_dividend == abs_divisor) &&
- (abs_base_dividend >= abs_divisor)) ||
- (abs_sig_dividend > abs_divisor));
-} else {
-env->PSW_USB_V = (abs_sig_dividend >= abs_divisor);
-}
+/* calc overflow
+   ofv if (a/b >= 0x) <=> (a/0x >= b) */
+env->PSW_USB_V = (abs_sig_dividend >= abs_divisor) << 31;
 env->PSW_USB_V = env->PSW_USB_V << 31;
 env->PSW_USB_SV |= env->PSW_USB_V;
 env->PSW_USB_AV = 0;
-- 
2.3.3

[Qemu-devel] [PULL 2/4] target-tricore: fix DVINIT_HU/BU calculating overflow before result

[Bastian Koppelmann]

dvinit_hu/bu for ISA v1.3 calculate the higher part of the result, that is 
needed
for the overflow bits, after calculating the overflow bits.

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

diff --git a/target-tricore/translate.c b/target-tricore/translate.c
index 0b7cf06..989a047 100644
--- a/target-tricore/translate.c
+++ b/target-tricore/translate.c
@@ -6240,7 +6240,7 @@ static void decode_rr_divide(CPUTriCoreState *env, 
DisasContext *ctx)
 uint32_t op2;
 int r1, r2, r3;
 
-TCGv temp, temp2;
+TCGv temp, temp2, temp3;
 
 op2 = MASK_OP_RR_OP2(ctx->opcode);
 r3 = MASK_OP_RR_D(ctx->opcode);
@@ -6261,14 +6261,17 @@ static void decode_rr_divide(CPUTriCoreState *env, 
DisasContext *ctx)
 case OPC2_32_RR_DVINIT_BU:
 temp = tcg_temp_new();
 temp2 = tcg_temp_new();
+temp3 = tcg_temp_new();
+
+tcg_gen_shri_tl(temp3, cpu_gpr_d[r1], 8);
 /* reset av */
 tcg_gen_movi_tl(cpu_PSW_AV, 0);
 if (!tricore_feature(env, TRICORE_FEATURE_131)) {
 /* overflow = (abs(D[r3+1]) >= abs(D[r2])) */
-tcg_gen_neg_tl(temp, cpu_gpr_d[r3+1]);
+tcg_gen_neg_tl(temp, temp3);
 /* use cpu_PSW_AV to compare against 0 */
-tcg_gen_movcond_tl(TCG_COND_LT, temp, cpu_gpr_d[r3+1], cpu_PSW_AV,
-   temp, cpu_gpr_d[r3+1]);
+tcg_gen_movcond_tl(TCG_COND_LT, temp, temp3, cpu_PSW_AV,
+   temp, temp3);
 tcg_gen_neg_tl(temp2, cpu_gpr_d[r2]);
 tcg_gen_movcond_tl(TCG_COND_LT, temp2, cpu_gpr_d[r2], cpu_PSW_AV,
temp2, cpu_gpr_d[r2]);
@@ -6281,12 +6284,12 @@ static void decode_rr_divide(CPUTriCoreState *env, 
DisasContext *ctx)
 /* sv */
 tcg_gen_or_tl(cpu_PSW_SV, cpu_PSW_SV, cpu_PSW_V);
 /* write result */
-tcg_gen_shri_tl(temp, cpu_gpr_d[r1], 8);
 tcg_gen_shli_tl(cpu_gpr_d[r3], cpu_gpr_d[r1], 24);
-tcg_gen_mov_tl(cpu_gpr_d[r3+1], temp);
+tcg_gen_mov_tl(cpu_gpr_d[r3+1], temp3);
 
 tcg_temp_free(temp);
 tcg_temp_free(temp2);
+tcg_temp_free(temp3);
 break;
 case OPC2_32_RR_DVINIT_H:
 gen_dvinit_h(env, cpu_gpr_d[r3], cpu_gpr_d[r3+1], cpu_gpr_d[r1],
@@ -6295,14 +6298,17 @@ static void decode_rr_divide(CPUTriCoreState *env, 
DisasContext *ctx)
 case OPC2_32_RR_DVINIT_HU:
 temp = tcg_temp_new();
 temp2 = tcg_temp_new();
+temp3 = tcg_temp_new();
+
+tcg_gen_shri_tl(temp3, cpu_gpr_d[r1], 16);
 /* reset av */
 tcg_gen_movi_tl(cpu_PSW_AV, 0);
 if (!tricore_feature(env, TRICORE_FEATURE_131)) {
 /* overflow = (abs(D[r3+1]) >= abs(D[r2])) */
-tcg_gen_neg_tl(temp, cpu_gpr_d[r3+1]);
+tcg_gen_neg_tl(temp, temp3);
 /* use cpu_PSW_AV to compare against 0 */
-tcg_gen_movcond_tl(TCG_COND_LT, temp, cpu_gpr_d[r3+1], cpu_PSW_AV,
-   temp, cpu_gpr_d[r3+1]);
+tcg_gen_movcond_tl(TCG_COND_LT, temp, temp3, cpu_PSW_AV,
+   temp, temp3);
 tcg_gen_neg_tl(temp2, cpu_gpr_d[r2]);
 tcg_gen_movcond_tl(TCG_COND_LT, temp2, cpu_gpr_d[r2], cpu_PSW_AV,
temp2, cpu_gpr_d[r2]);
@@ -6315,11 +6321,11 @@ static void decode_rr_divide(CPUTriCoreState *env, 
DisasContext *ctx)
 /* sv */
 tcg_gen_or_tl(cpu_PSW_SV, cpu_PSW_SV, cpu_PSW_V);
 /* write result */
-tcg_gen_mov_tl(temp, cpu_gpr_d[r1]);
-tcg_gen_shri_tl(cpu_gpr_d[r3+1], temp, 16);
-tcg_gen_shli_tl(cpu_gpr_d[r3], temp, 16);
+tcg_gen_mov_tl(cpu_gpr_d[r3+1], temp3);
+tcg_gen_shli_tl(cpu_gpr_d[r3], cpu_gpr_d[r1], 16);
 tcg_temp_free(temp);
 tcg_temp_free(temp2);
+tcg_temp_free(temp3);
 break;
 case OPC2_32_RR_DVINIT:
 temp = tcg_temp_new();
-- 
2.3.3

[Qemu-devel] [PULL 0/4] tricore-patches for 2.3-rc1

[Bastian Koppelmann]

The following changes since commit 362ca922eea03240916287a8a6267801ab095d12:

  Merge remote-tracking branch 'remotes/jnsnow/tags/ide-pull-request' into 
staging (2015-03-23 17:02:12 +)

are available in the git repository at:

  https://github.com/bkoppelmann/qemu-tricore-upstream.git 
tags/pull-tricore-20150324

for you to fetch changes up to f69c24e4584f2161f90ee7caba38728aa77f937f:

  target-tricore: properly fix dvinit_b/h_13 (2015-03-24 09:45:28 +0100)


TriCore bugfixes for 2.3-rc1


Bastian Koppelmann (3):
  target-tricore: fix DVINIT_HU/BU calculating overflow before result
  target-tricore: fix RRPW_DEXTR using wrong reg
  target-tricore: properly fix dvinit_b/h_13

Stefan Weil (1):
  target-tricore: Fix two helper functions (clang warnings)

 target-tricore/op_helper.c | 44 
 target-tricore/translate.c | 34 --
 2 files changed, 32 insertions(+), 46 deletions(-)
-- 
2.3.3

[Qemu-devel] [PULL 3/4] target-tricore: fix RRPW_DEXTR using wrong reg

[Bastian Koppelmann]

RRPW_DEXTR used r1 for the low part and r2 for the high part. It should be the
other way round. This also fixes that the result of the first shift was not
saved in a temp and could overwrite registers that were needed for the second
shift.

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

diff --git a/target-tricore/translate.c b/target-tricore/translate.c
index 989a047..bbcfee9 100644
--- a/target-tricore/translate.c
+++ b/target-tricore/translate.c
@@ -8044,8 +8044,8 @@ static void decode_32Bit_opc(CPUTriCoreState *env, 
DisasContext *ctx)
 tcg_gen_rotli_tl(cpu_gpr_d[r3], cpu_gpr_d[r1], const16);
 } else {
 temp = tcg_temp_new();
-tcg_gen_shli_tl(cpu_gpr_d[r3], cpu_gpr_d[r2], const16);
-tcg_gen_shri_tl(temp, cpu_gpr_d[r1], 32 - const16);
+tcg_gen_shli_tl(temp, cpu_gpr_d[r1], const16);
+tcg_gen_shri_tl(cpu_gpr_d[r3], cpu_gpr_d[r2], 32 - const16);
 tcg_gen_or_tl(cpu_gpr_d[r3], cpu_gpr_d[r3], temp);
 tcg_temp_free(temp);
 }
-- 
2.3.3

[Qemu-devel] [PATCH] target-ppc: gdbstub: Add VSX support

[Anton Blanchard]

Add the XML and functions to get and set VSX registers.

Signed-off-by: Anton Blanchard 
---
 configure   |  6 +++---
 gdb-xml/power-vsx.xml   | 44 
 target-ppc/translate_init.c | 22 ++
 3 files changed, 69 insertions(+), 3 deletions(-)
 create mode 100644 gdb-xml/power-vsx.xml

diff --git a/configure b/configure
index 589798e..235b3d2 100755
--- a/configure
+++ b/configure
@@ -5182,20 +5182,20 @@ case "$target_name" in
   ppc64)
 TARGET_BASE_ARCH=ppc
 TARGET_ABI_DIR=ppc
-gdb_xml_files="power64-core.xml power-fpu.xml power-altivec.xml 
power-spe.xml"
+gdb_xml_files="power64-core.xml power-fpu.xml power-altivec.xml 
power-spe.xml power-vsx.xml"
   ;;
   ppc64le)
 TARGET_ARCH=ppc64
 TARGET_BASE_ARCH=ppc
 TARGET_ABI_DIR=ppc
-gdb_xml_files="power64-core.xml power-fpu.xml power-altivec.xml 
power-spe.xml"
+gdb_xml_files="power64-core.xml power-fpu.xml power-altivec.xml 
power-spe.xml power-vsx.xml"
   ;;
   ppc64abi32)
 TARGET_ARCH=ppc64
 TARGET_BASE_ARCH=ppc
 TARGET_ABI_DIR=ppc
 echo "TARGET_ABI32=y" >> $config_target_mak
-gdb_xml_files="power64-core.xml power-fpu.xml power-altivec.xml 
power-spe.xml"
+gdb_xml_files="power64-core.xml power-fpu.xml power-altivec.xml 
power-spe.xml power-vsx.xml"
   ;;
   sh4|sh4eb)
 TARGET_ARCH=sh4
diff --git a/gdb-xml/power-vsx.xml b/gdb-xml/power-vsx.xml
new file mode 100644
index 000..fd290e9
--- /dev/null
+++ b/gdb-xml/power-vsx.xml
@@ -0,0 +1,44 @@
+
+
+
+
+
+
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index d74f4f0..efde425 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -8870,6 +8870,24 @@ static int gdb_set_spe_reg(CPUPPCState *env, uint8_t 
*mem_buf, int n)
 return 0;
 }
 
+static int gdb_get_vsx_reg(CPUPPCState *env, uint8_t *mem_buf, int n)
+{
+if (n < 32) {
+stq_p(mem_buf, env->vsr[n]);
+return 8;
+}
+return 0;
+}
+
+static int gdb_set_vsx_reg(CPUPPCState *env, uint8_t *mem_buf, int n)
+{
+if (n < 32) {
+env->vsr[n] = ldq_p(mem_buf);
+return 8;
+}
+return 0;
+}
+
 static int ppc_fixup_cpu(PowerPCCPU *cpu)
 {
 CPUPPCState *env = &cpu->env;
@@ -8967,6 +8985,10 @@ static void ppc_cpu_realizefn(DeviceState *dev, Error 
**errp)
 gdb_register_coprocessor(cs, gdb_get_spe_reg, gdb_set_spe_reg,
  34, "power-spe.xml", 0);
 }
+if (pcc->insns_flags2 & PPC2_VSX) {
+gdb_register_coprocessor(cs, gdb_get_vsx_reg, gdb_set_vsx_reg,
+ 32, "power-vsx.xml", 0);
+}
 
 qemu_init_vcpu(cs);
 
-- 
2.1.0

Re: [Qemu-devel] [libvirt] [RFC PATCH v2 00/12] qemu: add support to hot-plug/unplug cpu device

[Zhu Guihua]

On 03/24/2015 04:30 PM, Zhi Yong Wu wrote:

HI,

Do you have plan to update this patchset based on the comments
recently or have the latest one to post?


The develop plan for cpu hotplug in qemu has been changed, it is to add 
socket-based

device_add.

So I think we could not continue this cpu hotplug work in libvirt until the
interface about this feature in qemu could be determined.

Thanks,
Zhu



I noticed that the patchset for memory hotplug had got merged, is
there any plan to merge this patchset?


On Fri, Feb 13, 2015 at 12:13 AM, Peter Krempa  wrote:

On Thu, Feb 12, 2015 at 19:53:13 +0800, Zhu Guihua wrote:

Hi all,

Any comments about this series?

I'm not sure whether this series' method to support cpu hotplug in
libvirt is reasonable, so could anyone give more suggestions about this
function? Thanks.

Well, as Dan pointed out in his review for this series and previous
version, we are not convinced that we need a way to specify the CPU
model and other parameters as having dissimilar CPUs doesn't make sense.

A solution is either to build the cpu plug on top of the existing API or
provide enough information to convince us that having the cpu model in
the XML actually makes sense.


Regards,
Zhu

Peter

--
libvir-list mailing list
libvir-l...@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [Qemu-devel] [PATCH v5 38/45] Don't sync dirty bitmaps in postcopy

[Dr. David Alan Gilbert]

* David Gibson (da...@gibson.dropbear.id.au) wrote:
> On Wed, Feb 25, 2015 at 04:52:01PM +, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" 
> > 
> > Once we're in postcopy the source processors are stopped and memory
> > shouldn't change any more, so there's no need to look at the dirty
> > map.
> > 
> > There are two notes to this:
> >   1) If we do resync and a page had changed then the page would get
> >  sent again, which the destination wouldn't allow (since it might
> >  have also modified the page)
> 
> I'm not quite sure what you mean by "resync" in this context.

I mean synchronise the raw dirty bitmap with the migration bitmap,
which we normally do at every cycle through migration.

> >   2) Before disabling this I'd seen very rare cases where a page had been
> >  marked dirtied although the memory contents are apparently identical
> > 
> > Signed-off-by: Dr. David Alan Gilbert 
> 
> Reviewed-by: David Gibson 

Thanks,

Dave

> 
> -- 
> 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


--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH] target-ppc: gdbstub: Add VSX support

[Alexander Graf]

On 24.03.15 09:59, Anton Blanchard wrote:
> Add the XML and functions to get and set VSX registers.

Awesome, thanks. Have you verified that this works for LE as well as BE
guests?


Alex

Re: [Qemu-devel] [RESEND PATCH v4 6/6] acpi: Add hardware implementation for memory hot unplug

[Zhu Guihua]

On 03/23/2015 08:47 PM, Igor Mammedov wrote:

On Mon, 23 Mar 2015 18:59:28 +0800
Zhu Guihua  wrote:


On 03/16/2015 10:59 PM, Igor Mammedov wrote:
[...]
   
diff --git a/hw/i386/acpi-dsdt-mem-hotplug.dsl b/hw/i386/acpi-dsdt-mem-hotplug.dsl

index 1e9ec39..ef847e2 100644
--- a/hw/i386/acpi-dsdt-mem-hotplug.dsl
+++ b/hw/i386/acpi-dsdt-mem-hotplug.dsl
@@ -29,6 +29,7 @@
   External(MEMORY_SLOT_PROXIMITY, FieldUnitObj) // read only
   External(MEMORY_SLOT_ENABLED, FieldUnitObj) // 1 if enabled, 
read only
   External(MEMORY_SLOT_INSERT_EVENT, FieldUnitObj) // (read) 1 if 
has a insert event. (write) 1 to clear event
+External(MEMORY_SLOT_REMOVE_EVENT, FieldUnitObj) // (read) 1 if 
has a remove event. (write) 1 to clear event
   External(MEMORY_SLOT_SLECTOR, FieldUnitObj) // DIMM selector, 
write only
   External(MEMORY_SLOT_OST_EVENT, FieldUnitObj) // _OST event 
code, write only
   External(MEMORY_SLOT_OST_STATUS, FieldUnitObj) // _OST status 
code, write only
@@ -55,6 +56,8 @@
   If (LEqual(MEMORY_SLOT_INSERT_EVENT, One)) { // Memory 
device needs check
   MEMORY_SLOT_NOTIFY_METHOD(Local0, 1)
   Store(1, MEMORY_SLOT_INSERT_EVENT)
+} Elseif (LEqual(MEMORY_SLOT_REMOVE_EVENT, One)) { // 
Ejection request
+MEMORY_SLOT_NOTIFY_METHOD(Local0, 3)
clear removing field here.

You mean clear remove event here?

yes


I tested this method, clear remove event here will lead to guest
kernel panic.


   }
   // TODO: handle memory eject request
   Add(Local0, One, Local0) // goto next DIMM
@@ -156,5 +159,12 @@
   Store(Arg2, MEMORY_SLOT_OST_STATUS)
   Release(MEMORY_SLOT_LOCK)
   }
+
+Method(MEMORY_SLOT_EJECT_METHOD, 2) {
+Acquire(MEMORY_SLOT_LOCK, 0x)
+Store(ToInteger(Arg0), MEMORY_SLOT_SLECTOR) // select DIMM
+Store(One, MEMORY_SLOT_REMOVE_EVENT)

redo it using enabled field. Otherwise it could cause guest/QEMU crash:

- if 1st memory was asked to be removed
- then OSPM processes it but has not called _EJ0 yet leaving is_removed set
- then QEMU adds/removes another DIMM triggering slots scan
 which would issue second Notify(remove) since is_removed is still set
- as result it will cause failure in OSPM or in QEMU if OSPM issues double EJ0()


If OSPM processed the ejection request but not called _EJ0, the device
will still be present in qemu,
we must handle this.

There is nothing to handle in this case, if OSPM hasn't called _EJ0 then
nothing happens and device stays in QEMU.


So I think OSPM issues double EJ0 maybe reasonable
in this situation.
What's your opinion?

the first _EJ0 must do ejection, as for the second I think it should be NOP.


So we should judge the enabled field to check whether the device is 
present before

issuing Notify(remove)?

Thanks,
Zhu


Thanks,
Zhu


[...]
.

Re: [Qemu-devel] [PATCH v2 0/2] e1000: fixes for Phar Lap ETS

[Stefan Hajnoczi]

On Wed, Dec 10, 2014 at 11:23:45PM -0600, Richard Tollerton wrote:
> The 8254x driver in certain versions of Phar Lap ETS hasn't been
> initializing the e1000 device properly in qemu. It looks like the driver
> is relying on two specific pieces of behavior which (anecdotally) exist
> in hardware, although I can't cite any datasheets on the matter; in any
> case, these two patches adjust this behavior. Tested lightly on both ETS
> and Win7.
> 
> This is a resend; there are no changes compared to v1.
> 
> Richard Tollerton (2):
>   e1000: Clear MDIC register when PHY addr is invalid
>   e1000: decrement RDT if equal to RDH
> 
>  hw/net/e1000.c | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)

Hi,
Just working through unread patches in my mailbox.

Has this patch series stalled?

Stefan


pgpJiIF8BVfQi.pgp
Description: PGP signature

Re: [Qemu-devel] [RESEND PATCH v4 6/6] acpi: Add hardware implementation for memory hot unplug

[Zhu Guihua]

On 03/16/2015 10:59 PM, Igor Mammedov wrote:

On Mon, 16 Mar 2015 16:58:18 +0800
Zhu Guihua  wrote:


This patch adds a new bit to memory hotplug IO port indicating that

actually bit was added in 2/6 where is_removing had been added.


EJ0 has been evaluated by guest OS. And call pc-dimm unplug cb to do
the real removal.

Signed-off-by: Zhu Guihua 
---
  docs/specs/acpi_mem_hotplug.txt   | 11 +--
  hw/acpi/memory_hotplug.c  | 21 +++--
  hw/core/qdev.c|  2 +-
  hw/i386/acpi-build.c  |  9 +
  hw/i386/acpi-dsdt-mem-hotplug.dsl | 10 ++
  include/hw/acpi/pc-hotplug.h  |  2 ++
  include/hw/qdev-core.h|  1 +
  trace-events  |  1 +
  8 files changed, 52 insertions(+), 5 deletions(-)

diff --git a/docs/specs/acpi_mem_hotplug.txt b/docs/specs/acpi_mem_hotplug.txt
index 1290994..85cd4b8 100644
--- a/docs/specs/acpi_mem_hotplug.txt
+++ b/docs/specs/acpi_mem_hotplug.txt
@@ -19,7 +19,9 @@ Memory hot-plug interface (IO port 0xa00-0xa17, 1-4 byte 
access):
1: Device insert event, used to distinguish device for which
   no device check event to OSPM was issued.
   It's valid only when bit 1 is set.
-  2-7: reserved and should be ignored by OSPM
+  2: Device remove event, used to distinguish device for which
+ no device check event to OSPM was issued.
+  3-7: reserved and should be ignored by OSPM
[0x15-0x17] reserved
  
write access:

@@ -35,7 +37,12 @@ Memory hot-plug interface (IO port 0xa00-0xa17, 1-4 byte 
access):
1: if set to 1 clears device insert event, set by OSPM
   after it has emitted device check event for the
   selected memory device
-  2-7: reserved, OSPM must clear them before writing to register
+  2: if set to 1 clears device remove event, set by OSPM
+ after it has emitted device check event for the
+ selected memory device. if guest fails to eject device, it
+ should send OST event about it and forget about device
+ removal.
+  3-7: reserved, OSPM must clear them before writing to register
  
  Selecting memory device slot beyond present range has no effect on platform:

 - write accesses to memory hot-plug registers not documented above are
diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
index 687b2f1..d6b8c89 100644
--- a/hw/acpi/memory_hotplug.c
+++ b/hw/acpi/memory_hotplug.c
@@ -2,6 +2,7 @@
  #include "hw/acpi/pc-hotplug.h"
  #include "hw/mem/pc-dimm.h"
  #include "hw/boards.h"
+#include "hw/qdev-core.h"
  #include "trace.h"
  #include "qapi-event.h"
  
@@ -91,6 +92,8 @@ static void acpi_memory_hotplug_write(void *opaque, hwaddr addr, uint64_t data,

  MemHotplugState *mem_st = opaque;
  MemStatus *mdev;
  ACPIOSTInfo *info;
+DeviceState *dev = NULL;
+HotplugHandler *hotplug_ctrl = NULL;
  
  if (!mem_st->dev_count) {

  return;
@@ -122,19 +125,33 @@ static void acpi_memory_hotplug_write(void *opaque, 
hwaddr addr, uint64_t data,
  mdev = &mem_st->devs[mem_st->selector];
  mdev->ost_status = data;
  trace_mhp_acpi_write_ost_status(mem_st->selector, mdev->ost_status);
-/* TODO: implement memory removal on guest signal */
  
  info = acpi_memory_device_status(mem_st->selector, mdev);

  qapi_event_send_acpi_device_ost(info, &error_abort);
  qapi_free_ACPIOSTInfo(info);
  break;
-case 0x14:
+case 0x14: /* set is_* fields */
  mdev = &mem_st->devs[mem_st->selector];
  if (data & 2) { /* clear insert event */
  mdev->is_inserting  = false;
  trace_mhp_acpi_clear_insert_evt(mem_st->selector);
+} else if (data & 4) { /* request removal of device */

fix comment to match docs above.


+mdev->is_removing = false;
+trace_mhp_acpi_clear_remove_evt(mem_st->selector);

just clear event here and don't do removal part as it doesn't match
documentation you've written above regarding this field.

It would be better to move is_removing handling from here to 2/6
+ related ASL code from DSDT which should clear it after sending device check.


+/*
+ * QEMU memory hot unplug is an asynchronous procedure. QEMU first
+ * calls pc-dimm unplug request cb to send a SCI to guest. When the
+ * guest OS finished handling the SCI, it evaluates ACPI EJ0, and
+ * QEMU calls pc-dimm unplug cb to remove memory device.
+ */

something like this comment, should be in acpi_mem_hotplug.txt not here.


There is 'is_enabled' field, which is 1 if device is present, we can use it
for triggering actual ejecting in QEMU from EJ0(), something like:

} else if (data & 1) { /* eject device */


I think this is

Re: [Qemu-devel] [PATCH] target-ppc: gdbstub: Add VSX support

[Anton Blanchard]

Hi Alex,

> On 24.03.15 09:59, Anton Blanchard wrote:
> > Add the XML and functions to get and set VSX registers.
> 
> Awesome, thanks. Have you verified that this works for LE as well as
> BE guests?

Unfortunately all our XML gdbstub routines have endian issues (FPU,
Altivec and now VMX). I only caught that the other day.

I can work on reusing maybe_bswap_register() from gdbstub.c.

Anton

Re: [Qemu-devel] One question to lowlevel/xl/xl.c and lowlevel/xc/xc.c

[Ian Campbell]

On Tue, 2015-03-24 at 16:47 +0800, Chen, Tiejun wrote:
> All guys,
> 
> Sorry to bother you.
> 
> I have a question to two files, tools/python/xen/lowlevel/xc/xc.c and 
> tools/python/xen/lowlevel/xl/xl.c. Who is a caller to those methods like 
> pyxc_methods[] and pyxl_methods[]?

They are registered with the Python runtime, so they are called from
Python code. The first member of the struct is the pythonic function
name, e.g. from xc.c:
{ "domain_create", 
  (PyCFunction)pyxc_domain_create, 
  METH_VARARGS | METH_KEYWORDS, "\n"
  "Create a new domain.\n"
  " dom[int, 0]:Domain identifier to use (allocated if zero).\n"
  "Returns: [int] new domain identifier; -1 on error.\n" },
defines a method called domain_create, in the xen.lowlevel.xc namespace.

>  And how should we call these approaches?

I'm not sure what you are asking here.

> In my specific case, I'm trying to introduce a new flag to each a device 
> while assigning device. So this means I have to add a parameter, 'flag', 
> into
> 
> int xc_assign_device(
>  xc_interface *xch,
>  uint32_t domid,
>  uint32_t machine_sbdf)
> 
> Then this is extended as
> 
> int xc_assign_device(
>  xc_interface *xch,
>  uint32_t domid,
>  uint32_t machine_sbdf,
>  uint32_t flag)
> 
> After this introduction, obviously I should cover all cases using 
> xc_assign_device(). And also I found this fallout goes into these two 
> files. For example, here pyxc_assign_device() is involved. Currently it 
> has two parameters, 'dom' and 'pci_str', and as I understand 'pci_str' 
> should represent all pci devices with SBDF format, right?

It appears so, yes.

> But I don't know exactly what rule should be complied to construct this 
> sort of flag into 'pci_str', or any reasonable idea to achieve my goal?

If it is non-trivial to fix them IMHO it is acceptable for the new
parameter to not be plumbed up to the Python bindings until someone
comes along with a requirement to use it from Python. IOW you can just
pass whatever the nop value is for the new argument.

Ian.

Re: [Qemu-devel] [PATCH] qmp: implement a qmp command get_link_status

[Stefan Hajnoczi]

On Thu, Dec 11, 2014 at 02:36:14PM +0100, Wolfgang Link wrote:
> this qmp command returns the current link state from the given nic
> this is impotent if the set_link failed or get an timeout.

s/impotent/important/

I don't understand the rationale for this patch:

set_link does not fail or time out asynchronously, and the caller
immediately knows whether set_link succeeded or failed.  So I think this
command is not strictly necessary as implied by the commit description,
but it is useful if the QMP client wishes to query the current link
status.

> +SQMP
> +get_link_status
> +
> +
> +Get the link status of a network adapter.
> +
> +Arguments:
> +
> +- "name": network device name (json-string)
> +
> +Example:
> +
> +-> { "execute": "set_link", "arguments": { "name": "e1000.0" } }
> +<- { "return": {1} }
> +EQMP

Please add a query-link-status command.  All QMP commands that fetch
data are called query-*.


pgp3ARm7F_yrt.pgp
Description: PGP signature

Re: [Qemu-devel] [PATCH] target-ppc: gdbstub: Add VSX support

[Alexander Graf]

On 24.03.15 10:50, Anton Blanchard wrote:
> Hi Alex,
> 
>> On 24.03.15 09:59, Anton Blanchard wrote:
>>> Add the XML and functions to get and set VSX registers.
>>
>> Awesome, thanks. Have you verified that this works for LE as well as
>> BE guests?
> 
> Unfortunately all our XML gdbstub routines have endian issues (FPU,
> Altivec and now VMX). I only caught that the other day.
> 
> I can work on reusing maybe_bswap_register() from gdbstub.c.

Ouch. So I think we should fix the endianness issues for 2.3 (which is
in rc right now), but enable VMX for 2.4 since it's really a new feature.

Would you mind to work on and send at least the endian fixes reasonably
soon, so that we can still get them into the release?


Thanks a lot,

Alex

Re: [Qemu-devel] One question to lowlevel/xl/xl.c and lowlevel/xc/xc.c

[Chen, Tiejun]

On 2015/3/24 17:51, Ian Campbell wrote:

On Tue, 2015-03-24 at 16:47 +0800, Chen, Tiejun wrote:

All guys,



Thanks for your reply.


Sorry to bother you.

I have a question to two files, tools/python/xen/lowlevel/xc/xc.c and
tools/python/xen/lowlevel/xl/xl.c. Who is a caller to those methods like
pyxc_methods[] and pyxl_methods[]?


They are registered with the Python runtime, so they are called from
Python code. The first member of the struct is the pythonic function


Sorry I don't understanding this. So seems you mean instead of xl, this 
is called by the third party user with python?



name, e.g. from xc.c:
 { "domain_create",


Otherwise, often we always perform `xl create xxx' to create a VM. So I 
think this should go into this flow like this,


xl_cmdtable.c:main_create()
|
+ create_domain()
|
+ libxl_domain_create_new()
|
+ do_domain_create()
|
+ 
Right?


   (PyCFunction)pyxc_domain_create,


So I don't see 'pyxc_domain_create' is called. Or I'm missing something...


   METH_VARARGS | METH_KEYWORDS, "\n"
   "Create a new domain.\n"
   " dom[int, 0]:Domain identifier to use (allocated if 
zero).\n"
   "Returns: [int] new domain identifier; -1 on error.\n" },
defines a method called domain_create, in the xen.lowlevel.xc namespace.


  And how should we call these approaches?


I'm not sure what you are asking here.


If you can give a real case to call this, things couldn't be better :)




In my specific case, I'm trying to introduce a new flag to each a device
while assigning device. So this means I have to add a parameter, 'flag',
into

int xc_assign_device(
  xc_interface *xch,
  uint32_t domid,
  uint32_t machine_sbdf)

Then this is extended as

int xc_assign_device(
  xc_interface *xch,
  uint32_t domid,
  uint32_t machine_sbdf,
  uint32_t flag)

After this introduction, obviously I should cover all cases using
xc_assign_device(). And also I found this fallout goes into these two
files. For example, here pyxc_assign_device() is involved. Currently it
has two parameters, 'dom' and 'pci_str', and as I understand 'pci_str'
should represent all pci devices with SBDF format, right?


It appears so, yes.


But I don't know exactly what rule should be complied to construct this
sort of flag into 'pci_str', or any reasonable idea to achieve my goal?


If it is non-trivial to fix them IMHO it is acceptable for the new
parameter to not be plumbed up to the Python bindings until someone
comes along with a requirement to use it from Python. IOW you can just
pass whatever the nop value is for the new argument.



Should I extend this 'pci_str' like "Seg,bus,device,function:flag"? But 
I'm not sure if I'm breaking the existing usage since like I said, I 
don't know what scenarios are using these methods.


Thanks
Tiejun

Re: [Qemu-devel] One question to lowlevel/xl/xl.c and lowlevel/xc/xc.c

[Ian Campbell]

On Tue, 2015-03-24 at 18:15 +0800, Chen, Tiejun wrote:
> On 2015/3/24 17:51, Ian Campbell wrote:
> > On Tue, 2015-03-24 at 16:47 +0800, Chen, Tiejun wrote:
> >> All guys,
> >>
> 
> Thanks for your reply.
> 
> >> Sorry to bother you.
> >>
> >> I have a question to two files, tools/python/xen/lowlevel/xc/xc.c and
> >> tools/python/xen/lowlevel/xl/xl.c. Who is a caller to those methods like
> >> pyxc_methods[] and pyxl_methods[]?
> >
> > They are registered with the Python runtime, so they are called from
> > Python code. The first member of the struct is the pythonic function
> 
> Sorry I don't understanding this. So seems you mean instead of xl, this 
> is called by the third party user with python?

Yes, tools/python/xen is the python bindings for various C libraries
supported by Xen.

NB, the libxl ones are broken and not even compiled right now, you can
ignore them.

> 
> > name, e.g. from xc.c:
> >  { "domain_create",
> 
> Otherwise, often we always perform `xl create xxx' to create a VM. So I 
> think this should go into this flow like this,
> 
> xl_cmdtable.c:main_create()
>   |
>   + create_domain()
>   |
>   + libxl_domain_create_new()
>   |
>   + do_domain_create()
>   |
>   + 
> Right?

Yes, xl is written in C not python so tools/python doesn't enter the
picture.

> 
> >(PyCFunction)pyxc_domain_create,
> 
> So I don't see 'pyxc_domain_create' is called. Or I'm missing something...

Chances are that there are no intree users of this code any more, xend
would have used it at one time with something like:
import xen.lowlevel.xc
xc = xen.lowlevel.xc.xc()
xc.domain_create()
etc.

> >
> >> In my specific case, I'm trying to introduce a new flag to each a device
> >> while assigning device. So this means I have to add a parameter, 'flag',
> >> into
> >>
> >> int xc_assign_device(
> >>   xc_interface *xch,
> >>   uint32_t domid,
> >>   uint32_t machine_sbdf)
> >>
> >> Then this is extended as
> >>
> >> int xc_assign_device(
> >>   xc_interface *xch,
> >>   uint32_t domid,
> >>   uint32_t machine_sbdf,
> >>   uint32_t flag)
> >>
> >> After this introduction, obviously I should cover all cases using
> >> xc_assign_device(). And also I found this fallout goes into these two
> >> files. For example, here pyxc_assign_device() is involved. Currently it
> >> has two parameters, 'dom' and 'pci_str', and as I understand 'pci_str'
> >> should represent all pci devices with SBDF format, right?
> >
> > It appears so, yes.
> >
> >> But I don't know exactly what rule should be complied to construct this
> >> sort of flag into 'pci_str', or any reasonable idea to achieve my goal?
> >
> > If it is non-trivial to fix them IMHO it is acceptable for the new
> > parameter to not be plumbed up to the Python bindings until someone
> > comes along with a requirement to use it from Python. IOW you can just
> > pass whatever the nop value is for the new argument.
> >
> 
> Should I extend this 'pci_str' like "Seg,bus,device,function:flag"? But 
> I'm not sure if I'm breaking the existing usage since like I said, I 
> don't know what scenarios are using these methods.

Like I said in the paragraph above, if it is complicated then it is fine
to ignore this new parameter from Python.

I don't know what the semantics of flag is, if it is per SBDF then I
suppose if you really wanted to expose this here then you would need to
invent some syntax for doing so.

Ian.

Re: [Qemu-devel] [PATCH for-2.3 0/3] Contain drive_get() misuse

[Paolo Bonzini]

On 23/03/2015 20:09, Markus Armbruster wrote:
> Drives defined with if!=none are for board initialization to wire up.
> Board code calls drive_get() or similar to find them, and creates
> devices with their qdev drive properties set accordingly.
> 
> Except a few devices go on a fishing expedition for a suitable backend
> instead of exposing a drive property for board code to set: they call
> driver_get() or drive_get_next() in their realize() or init() method.
> Wrong.
> 
> We can't fix this in time for the release, so do the next best thing:
> contain the mistakes as far as possible so they don't become ABI:
> 
> * Mark them all with suitable FIXME comments [PATCH 1]
> 
> * sdhci-pci is new, set cannot_instantiate_with_device_add_yet to make
>   it unavailable with -device [PATCH 2]
> 
> * A few more aren't currently available with -device, set
>   cannot_instantiate_with_device_add_yet to ensure they stay
>   unavailable [PATCH 3]
> 
> * Left alone: m25p80-generic and its derivatives, ssi-sd, pc87312

Maybe worth documenting as future incompatible changes?  These machines
are not versioned, so it's not the end of the world to make things saner
if somebody comes and qdevifies the SD card.

> Markus Armbruster (3):
>   hw: Mark devices misusing drive_get(), drive_get_next() FIXME
>   sdhci: Make device "sdhci-pci" unavailable with -device
>   sysbus: Contain drive_get_next() misuse
> 
>  hw/arm/spitz.c| 3 +++
>  hw/block/m25p80.c | 1 +
>  hw/isa/pc87312.c  | 2 ++
>  hw/sd/milkymist-memcard.c | 3 +++
>  hw/sd/pl181.c | 3 +++
>  hw/sd/sdhci.c | 6 ++
>  hw/sd/ssi-sd.c| 1 +
>  7 files changed, 19 insertions(+)
> 

Acked-by: Paolo Bonzini 

Re: [Qemu-devel] [RESEND PATCH v4 6/6] acpi: Add hardware implementation for memory hot unplug

[Igor Mammedov]

On Tue, 24 Mar 2015 17:34:29 +0800
Zhu Guihua  wrote:

> 
> On 03/23/2015 08:47 PM, Igor Mammedov wrote:
> > On Mon, 23 Mar 2015 18:59:28 +0800
> > Zhu Guihua  wrote:
> >
> >> On 03/16/2015 10:59 PM, Igor Mammedov wrote:
> >> [...]
> >>>
> >>> diff --git a/hw/i386/acpi-dsdt-mem-hotplug.dsl 
> >>> b/hw/i386/acpi-dsdt-mem-hotplug.dsl
> >>> index 1e9ec39..ef847e2 100644
> >>> --- a/hw/i386/acpi-dsdt-mem-hotplug.dsl
> >>> +++ b/hw/i386/acpi-dsdt-mem-hotplug.dsl
> >>> @@ -29,6 +29,7 @@
> >>>External(MEMORY_SLOT_PROXIMITY, FieldUnitObj) // read only
> >>>External(MEMORY_SLOT_ENABLED, FieldUnitObj) // 1 if 
> >>> enabled, read only
> >>>External(MEMORY_SLOT_INSERT_EVENT, FieldUnitObj) // (read) 
> >>> 1 if has a insert event. (write) 1 to clear event
> >>> +External(MEMORY_SLOT_REMOVE_EVENT, FieldUnitObj) // (read) 1 
> >>> if has a remove event. (write) 1 to clear event
> >>>External(MEMORY_SLOT_SLECTOR, FieldUnitObj) // DIMM 
> >>> selector, write only
> >>>External(MEMORY_SLOT_OST_EVENT, FieldUnitObj) // _OST 
> >>> event code, write only
> >>>External(MEMORY_SLOT_OST_STATUS, FieldUnitObj) // _OST 
> >>> status code, write only
> >>> @@ -55,6 +56,8 @@
> >>>If (LEqual(MEMORY_SLOT_INSERT_EVENT, One)) { // 
> >>> Memory device needs check
> >>>MEMORY_SLOT_NOTIFY_METHOD(Local0, 1)
> >>>Store(1, MEMORY_SLOT_INSERT_EVENT)
> >>> +} Elseif (LEqual(MEMORY_SLOT_REMOVE_EVENT, One)) { 
> >>> // Ejection request
> >>> +MEMORY_SLOT_NOTIFY_METHOD(Local0, 3)
> >>> clear removing field here.
> >> You mean clear remove event here?
> > yes
> 
> I tested this method, clear remove event here will lead to guest
> kernel panic.
it shouldn't cause panic if it only clears flag in QEMU
(that's what it should do).


> 
> }
> // TODO: handle memory eject request
> Add(Local0, One, Local0) // goto next DIMM
>  @@ -156,5 +159,12 @@
> Store(Arg2, MEMORY_SLOT_OST_STATUS)
> Release(MEMORY_SLOT_LOCK)
> }
>  +
>  +Method(MEMORY_SLOT_EJECT_METHOD, 2) {
>  +Acquire(MEMORY_SLOT_LOCK, 0x)
>  +Store(ToInteger(Arg0), MEMORY_SLOT_SLECTOR) // select 
>  DIMM
>  +Store(One, MEMORY_SLOT_REMOVE_EVENT)
> >>> redo it using enabled field. Otherwise it could cause guest/QEMU crash:
> >>>
> >>> - if 1st memory was asked to be removed
> >>> - then OSPM processes it but has not called _EJ0 yet leaving is_removed 
> >>> set
> >>> - then QEMU adds/removes another DIMM triggering slots scan
> >>>  which would issue second Notify(remove) since is_removed is still set
> >>> - as result it will cause failure in OSPM or in QEMU if OSPM issues 
> >>> double EJ0()
> >>>
> >> If OSPM processed the ejection request but not called _EJ0, the device
> >> will still be present in qemu,
> >> we must handle this.
> > There is nothing to handle in this case, if OSPM hasn't called _EJ0 then
> > nothing happens and device stays in QEMU.
> >
> >> So I think OSPM issues double EJ0 maybe reasonable
> >> in this situation.
> >> What's your opinion?
> > the first _EJ0 must do ejection, as for the second I think it should be NOP.
> 
> So we should judge the enabled field to check whether the device is 
> present before
> issuing Notify(remove)?
I wouldn't check if device is present.
I'd unconditionally clear it and make sure on QEMU side that
operation is NOP if device is not present.

> 
> Thanks,
> Zhu
> 
> >> Thanks,
> >> Zhu
> >>
> > [...]
> > .
> >
> 

Re: [Qemu-devel] One question to lowlevel/xl/xl.c and lowlevel/xc/xc.c

[Chen, Tiejun]

On 2015/3/24 18:20, Ian Campbell wrote:

On Tue, 2015-03-24 at 18:15 +0800, Chen, Tiejun wrote:

On 2015/3/24 17:51, Ian Campbell wrote:

On Tue, 2015-03-24 at 16:47 +0800, Chen, Tiejun wrote:

All guys,



Thanks for your reply.


Sorry to bother you.

I have a question to two files, tools/python/xen/lowlevel/xc/xc.c and
tools/python/xen/lowlevel/xl/xl.c. Who is a caller to those methods like
pyxc_methods[] and pyxl_methods[]?


They are registered with the Python runtime, so they are called from
Python code. The first member of the struct is the pythonic function


Sorry I don't understanding this. So seems you mean instead of xl, this
is called by the third party user with python?


Yes, tools/python/xen is the python bindings for various C libraries
supported by Xen.


Thanks for your explanation.



NB, the libxl ones are broken and not even compiled right now, you can
ignore them.


Looks this is still compiled now.






name, e.g. from xc.c:
  { "domain_create",


Otherwise, often we always perform `xl create xxx' to create a VM. So I
think this should go into this flow like this,

xl_cmdtable.c:main_create()
|
+ create_domain()
|
+ libxl_domain_create_new()
|
+ do_domain_create()
|
+ 
Right?


Yes, xl is written in C not python so tools/python doesn't enter the
picture.


Yeah.






(PyCFunction)pyxc_domain_create,


So I don't see 'pyxc_domain_create' is called. Or I'm missing something...


Chances are that there are no intree users of this code any more, xend
would have used it at one time with something like:
import xen.lowlevel.xc
 xc = xen.lowlevel.xc.xc()
 xc.domain_create()
etc.




In my specific case, I'm trying to introduce a new flag to each a device
while assigning device. So this means I have to add a parameter, 'flag',
into

int xc_assign_device(
   xc_interface *xch,
   uint32_t domid,
   uint32_t machine_sbdf)

Then this is extended as

int xc_assign_device(
   xc_interface *xch,
   uint32_t domid,
   uint32_t machine_sbdf,
   uint32_t flag)

After this introduction, obviously I should cover all cases using
xc_assign_device(). And also I found this fallout goes into these two
files. For example, here pyxc_assign_device() is involved. Currently it
has two parameters, 'dom' and 'pci_str', and as I understand 'pci_str'
should represent all pci devices with SBDF format, right?


It appears so, yes.


But I don't know exactly what rule should be complied to construct this
sort of flag into 'pci_str', or any reasonable idea to achieve my goal?


If it is non-trivial to fix them IMHO it is acceptable for the new
parameter to not be plumbed up to the Python bindings until someone
comes along with a requirement to use it from Python. IOW you can just
pass whatever the nop value is for the new argument.



Should I extend this 'pci_str' like "Seg,bus,device,function:flag"? But
I'm not sure if I'm breaking the existing usage since like I said, I
don't know what scenarios are using these methods.


Like I said in the paragraph above, if it is complicated then it is fine
to ignore this new parameter from Python.

I don't know what the semantics of flag is, if it is per SBDF then I


Yes, this should be a flag specific to a SBDF.

You know, I'm working to fix RMRR completely. Based on some discussion 
about that design ( I assume you may read that thread previously :) ), 
now we probably need to pass a flag to introduce our policy.



suppose if you really wanted to expose this here then you would need to
invent some syntax for doing so.



Definitely.

When I finish this I will send you to review technically.

Again, really appreciate your clarification to me.

Thanks
Tiejun

Re: [Qemu-devel] [RESEND PATCH v4 6/6] acpi: Add hardware implementation for memory hot unplug

[Igor Mammedov]

On Tue, 24 Mar 2015 17:38:53 +0800
Zhu Guihua  wrote:

> 
> On 03/16/2015 10:59 PM, Igor Mammedov wrote:
> > On Mon, 16 Mar 2015 16:58:18 +0800
> > Zhu Guihua  wrote:
> >
> >> This patch adds a new bit to memory hotplug IO port indicating that
> > actually bit was added in 2/6 where is_removing had been added.
> >
> >> EJ0 has been evaluated by guest OS. And call pc-dimm unplug cb to do
> >> the real removal.
> >>
> >> Signed-off-by: Zhu Guihua 
> >> ---
> >>   docs/specs/acpi_mem_hotplug.txt   | 11 +--
> >>   hw/acpi/memory_hotplug.c  | 21 +++--
> >>   hw/core/qdev.c|  2 +-
> >>   hw/i386/acpi-build.c  |  9 +
> >>   hw/i386/acpi-dsdt-mem-hotplug.dsl | 10 ++
> >>   include/hw/acpi/pc-hotplug.h  |  2 ++
> >>   include/hw/qdev-core.h|  1 +
> >>   trace-events  |  1 +
> >>   8 files changed, 52 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/docs/specs/acpi_mem_hotplug.txt 
> >> b/docs/specs/acpi_mem_hotplug.txt
> >> index 1290994..85cd4b8 100644
> >> --- a/docs/specs/acpi_mem_hotplug.txt
> >> +++ b/docs/specs/acpi_mem_hotplug.txt
> >> @@ -19,7 +19,9 @@ Memory hot-plug interface (IO port 0xa00-0xa17, 1-4 byte 
> >> access):
> >> 1: Device insert event, used to distinguish device for 
> >> which
> >>no device check event to OSPM was issued.
> >>It's valid only when bit 1 is set.
> >> -  2-7: reserved and should be ignored by OSPM
> >> +  2: Device remove event, used to distinguish device for which
> >> + no device check event to OSPM was issued.
> >> +  3-7: reserved and should be ignored by OSPM
> >> [0x15-0x17] reserved
> >>   
> >> write access:
> >> @@ -35,7 +37,12 @@ Memory hot-plug interface (IO port 0xa00-0xa17, 1-4 
> >> byte access):
> >> 1: if set to 1 clears device insert event, set by OSPM
> >>after it has emitted device check event for the
> >>selected memory device
> >> -  2-7: reserved, OSPM must clear them before writing to 
> >> register
> >> +  2: if set to 1 clears device remove event, set by OSPM
> >> + after it has emitted device check event for the
> >> + selected memory device. if guest fails to eject device, 
> >> it
> >> + should send OST event about it and forget about device
> >> + removal.
> >> +  3-7: reserved, OSPM must clear them before writing to 
> >> register
> >>   
> >>   Selecting memory device slot beyond present range has no effect on 
> >> platform:
> >>  - write accesses to memory hot-plug registers not documented above are
> >> diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
> >> index 687b2f1..d6b8c89 100644
> >> --- a/hw/acpi/memory_hotplug.c
> >> +++ b/hw/acpi/memory_hotplug.c
> >> @@ -2,6 +2,7 @@
> >>   #include "hw/acpi/pc-hotplug.h"
> >>   #include "hw/mem/pc-dimm.h"
> >>   #include "hw/boards.h"
> >> +#include "hw/qdev-core.h"
> >>   #include "trace.h"
> >>   #include "qapi-event.h"
> >>   
> >> @@ -91,6 +92,8 @@ static void acpi_memory_hotplug_write(void *opaque, 
> >> hwaddr addr, uint64_t data,
> >>   MemHotplugState *mem_st = opaque;
> >>   MemStatus *mdev;
> >>   ACPIOSTInfo *info;
> >> +DeviceState *dev = NULL;
> >> +HotplugHandler *hotplug_ctrl = NULL;
> >>   
> >>   if (!mem_st->dev_count) {
> >>   return;
> >> @@ -122,19 +125,33 @@ static void acpi_memory_hotplug_write(void *opaque, 
> >> hwaddr addr, uint64_t data,
> >>   mdev = &mem_st->devs[mem_st->selector];
> >>   mdev->ost_status = data;
> >>   trace_mhp_acpi_write_ost_status(mem_st->selector, 
> >> mdev->ost_status);
> >> -/* TODO: implement memory removal on guest signal */
> >>   
> >>   info = acpi_memory_device_status(mem_st->selector, mdev);
> >>   qapi_event_send_acpi_device_ost(info, &error_abort);
> >>   qapi_free_ACPIOSTInfo(info);
> >>   break;
> >> -case 0x14:
> >> +case 0x14: /* set is_* fields */
> >>   mdev = &mem_st->devs[mem_st->selector];
> >>   if (data & 2) { /* clear insert event */
> >>   mdev->is_inserting  = false;
> >>   trace_mhp_acpi_clear_insert_evt(mem_st->selector);
> >> +} else if (data & 4) { /* request removal of device */
> > fix comment to match docs above.
> >
> >> +mdev->is_removing = false;
> >> +trace_mhp_acpi_clear_remove_evt(mem_st->selector);
> > just clear event here and don't do removal part as it doesn't match
> > documentation you've written above regarding this field.
> >
> > It would be better to move is_removing handling from here to 2/6
> > + related ASL code from DSDT which should clear it after sending device 
> > check.
> >
> >> +/*
> >> +

[Qemu-devel] [PATCH 1/3] apic: Added helper function apic_match_dest, apic_match_[physical, logical]_dest

[James Sullivan]

Added three helper functions apic_match_dest(),
apic_match_physical_dest(), and apic_match_logical_dest() which
can be used to determine if a logical or physical APIC ID match a
given LAPIC under a given dest_mode. This does not account for shorthand.

Signed-off-by: James Sullivan 
---
 hw/intc/apic.c | 26 ++
 1 file changed, 26 insertions(+)

diff --git a/hw/intc/apic.c b/hw/intc/apic.c
index 0f97b47..a5b5990 100644
--- a/hw/intc/apic.c
+++ b/hw/intc/apic.c
@@ -217,6 +217,32 @@ static void apic_external_nmi(APICCommonState *s)
 }\
 }
 
+static bool apic_match_physical_dest(struct APICCommonState *apic, uint8_t 
dest)
+{
+return (dest == 0xff || apic->id == dest);
+}
+
+static bool apic_match_logical_dest(struct APICCommonState *apic, uint8_t dest)
+{
+if (apic->dest_mode == 0xf) {
+return (dest & apic->log_dest) > 0;
+} else if (apic->dest_mode == 0) {
+return ((dest & 0xf0) == (apic->log_dest & 0xf0)) &&
+(dest & apic->log_dest & 0x0f) > 0;
+}
+return false;
+}
+
+static bool apic_match_dest(struct APICCommonState *apic, uint8_t dest_mode,
+ uint8_t dest)
+{
+if (dest_mode == 0) {
+return apic_match_physical_dest(apic, dest);
+} else {
+return apic_match_logical_dest(apic, dest);
+}
+}
+
 static void apic_bus_deliver(const uint32_t *deliver_bitmask,
  uint8_t delivery_mode, uint8_t vector_num,
  uint8_t trigger_mode)
-- 
2.3.3

[Qemu-devel] [PATCH] apic: Implement low-priority arbitration for IRQ delivery

[James Sullivan]

Currently, there is no arbitration among processors for low priority IRQ
delivery. Implemented apic_get_arb_pri(), and added two new functions
apic_compare_prio() and apic_lowest_prio() to support arbitration in
apic_bus_deliver().

Signed-off-by: James Sullivan 
---
 hw/intc/apic.c | 67 ++
 1 file changed, 49 insertions(+), 18 deletions(-)

diff --git a/hw/intc/apic.c b/hw/intc/apic.c
index 0f97b47..47d2fb1 100644
--- a/hw/intc/apic.c
+++ b/hw/intc/apic.c
@@ -38,6 +38,7 @@ static void apic_set_irq(APICCommonState *s, int vector_num, 
int trigger_mode);
 static void apic_update_irq(APICCommonState *s);
 static void apic_get_delivery_bitmask(uint32_t *deliver_bitmask,
   uint8_t dest, uint8_t dest_mode);
+static int apic_get_arb_pri(APICCommonState *s);
 
 /* Find first bit starting from msb */
 static int apic_fls_bit(uint32_t value)
@@ -199,6 +200,29 @@ static void apic_external_nmi(APICCommonState *s)
 apic_local_deliver(s, APIC_LVT_LINT1);
 }
 
+static int apic_compare_prio(struct APICCommonState *cpu1,
+ struct APICCommonState *cpu2)
+{
+return apic_get_arb_pri(cpu1) - apic_get_arb_pri(cpu2);
+}
+
+static struct APICCommonState *apic_lowest_prio(const uint32_t
+*deliver_bitmask)
+{
+APICCommonState *lowest = NULL;
+int i, d;
+
+for (i = 0; i < MAX_APIC_WORDS; i++) {
+if (deliver_bitmask[i]) {
+d = i * 32 + apic_ffs_bit(deliver_bitmask[i]);
+if (!lowest || apic_compare_prio(local_apics[d], lowest) < 0) {
+lowest = local_apics[d];
+}
+}
+}
+return lowest;
+}
+
 #define foreach_apic(apic, deliver_bitmask, code) \
 {\
 int __i, __j;\
@@ -225,22 +249,10 @@ static void apic_bus_deliver(const uint32_t 
*deliver_bitmask,
 
 switch (delivery_mode) {
 case APIC_DM_LOWPRI:
-/* XXX: search for focus processor, arbitration */
-{
-int i, d;
-d = -1;
-for(i = 0; i < MAX_APIC_WORDS; i++) {
-if (deliver_bitmask[i]) {
-d = i * 32 + apic_ffs_bit(deliver_bitmask[i]);
-break;
-}
-}
-if (d >= 0) {
-apic_iter = local_apics[d];
-if (apic_iter) {
-apic_set_irq(apic_iter, vector_num, trigger_mode);
-}
-}
+/* XXX: search for focus processor */
+apic_iter = apic_lowest_prio(deliver_bitmask);
+if (apic_iter) {
+apic_set_irq(apic_iter , vector_num, trigger_mode);
 }
 return;
 
@@ -336,8 +348,27 @@ static int apic_get_ppr(APICCommonState *s)
 
 static int apic_get_arb_pri(APICCommonState *s)
 {
-/* XXX: arbitration */
-return 0;
+int tpr, isrv, irrv, apr;
+
+tpr = apic_get_tpr(s);
+isrv = get_highest_priority_int(s->isr);
+if (isrv < 0) {
+isrv = 0;
+}
+isrv >>= 4;
+irrv = get_highest_priority_int(s->irr);
+if (irrv < 0) {
+irrv = 0;
+}
+irrv >>= 4;
+
+if ((tpr >= irrv) && (tpr > isrv)) {
+apr = s->tpr & 0xff;
+} else {
+apr = ((tpr & isrv) > irrv) ? (tpr & isrv) : irrv;
+apr <<= 4;
+}
+return apr;
 }
 
 
-- 
2.3.3

[Qemu-devel] [PATCH 2/3] apic: Set and pass in RH bit for MSI interrupts

[James Sullivan]

In apic_send_msi(), set msi_redir_hint to 0x1 when RH=1 in the
MSI Address Register.

Added an argument for msi_redir_hint to apic_deliver_irq(), and
changed calls to the function accordingly (using 0 as a default value
for non-MSI interrupts).

Signed-off-by: James Sullivan 
---
 hw/intc/apic.c | 10 ++
 hw/intc/ioapic.c   |  2 +-
 include/hw/i386/apic.h |  3 ++-
 trace-events   |  2 +-
 4 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/hw/intc/apic.c b/hw/intc/apic.c
index a5b5990..97dd649 100644
--- a/hw/intc/apic.c
+++ b/hw/intc/apic.c
@@ -306,12 +306,13 @@ static void apic_bus_deliver(const uint32_t 
*deliver_bitmask,
 }
 
 void apic_deliver_irq(uint8_t dest, uint8_t dest_mode, uint8_t delivery_mode,
-  uint8_t vector_num, uint8_t trigger_mode)
+  uint8_t vector_num, uint8_t trigger_mode,
+  uint8_t msi_redir_hint)
 {
 uint32_t deliver_bitmask[MAX_APIC_WORDS];
 
 trace_apic_deliver_irq(dest, dest_mode, delivery_mode, vector_num,
-   trigger_mode);
+   trigger_mode, msi_redir_hint);
 
 apic_get_delivery_bitmask(deliver_bitmask, dest, dest_mode);
 apic_bus_deliver(deliver_bitmask, delivery_mode, vector_num, trigger_mode);
@@ -777,8 +778,9 @@ static void apic_send_msi(hwaddr addr, uint32_t data)
 uint8_t dest_mode = (addr >> MSI_ADDR_DEST_MODE_SHIFT) & 0x1;
 uint8_t trigger_mode = (data >> MSI_DATA_TRIGGER_SHIFT) & 0x1;
 uint8_t delivery = (data >> MSI_DATA_DELIVERY_MODE_SHIFT) & 0x7;
-/* XXX: Ignore redirection hint. */
-apic_deliver_irq(dest, dest_mode, delivery, vector, trigger_mode);
+uint8_t msi_redir_hint = (data >> MSI_ADDR_REDIRECTION_SHIFT) & 0x1;
+apic_deliver_irq(dest, dest_mode, delivery, vector, trigger_mode,
+ msi_redir_hint);
 }
 
 static void apic_mem_writel(void *opaque, hwaddr addr, uint32_t val)
diff --git a/hw/intc/ioapic.c b/hw/intc/ioapic.c
index b527932..e69aa18 100644
--- a/hw/intc/ioapic.c
+++ b/hw/intc/ioapic.c
@@ -71,7 +71,7 @@ static void ioapic_service(IOAPICCommonState *s)
 vector = entry & IOAPIC_VECTOR_MASK;
 }
 apic_deliver_irq(dest, dest_mode, delivery_mode,
- vector, trig_mode);
+ vector, trig_mode, 0);
 }
 }
 }
diff --git a/include/hw/i386/apic.h b/include/hw/i386/apic.h
index 1d48e02..1e15edd 100644
--- a/include/hw/i386/apic.h
+++ b/include/hw/i386/apic.h
@@ -5,7 +5,8 @@
 
 /* apic.c */
 void apic_deliver_irq(uint8_t dest, uint8_t dest_mode, uint8_t delivery_mode,
-  uint8_t vector_num, uint8_t trigger_mode);
+  uint8_t vector_num, uint8_t trigger_mode,
+  uint8_t msi_redir_hint);
 int apic_accept_pic_intr(DeviceState *s);
 void apic_deliver_pic_intr(DeviceState *s, int level);
 void apic_deliver_nmi(DeviceState *d);
diff --git a/trace-events b/trace-events
index 30eba92..b430cf9 100644
--- a/trace-events
+++ b/trace-events
@@ -158,7 +158,7 @@ apic_get_irq_delivered(int apic_irq_delivered) "returning 
coalescing %d"
 
 # hw/intc/apic.c
 apic_local_deliver(int vector, uint32_t lvt) "vector %d delivery mode %d"
-apic_deliver_irq(uint8_t dest, uint8_t dest_mode, uint8_t delivery_mode, 
uint8_t vector_num, uint8_t trigger_mode) "dest %d dest_mode %d delivery_mode 
%d vector %d trigger_mode %d"
+apic_deliver_irq(uint8_t dest, uint8_t dest_mode, uint8_t delivery_mode, 
uint8_t vector_num, uint8_t trigger_mode, uint8_t msi_redir_hint) "dest %d 
dest_mode %d delivery_mode %d vector %d trigger_mode %d msi_redir_hint %d"
 apic_mem_readl(uint64_t addr, uint32_t val)  "%"PRIx64" = %08x"
 apic_mem_writel(uint64_t addr, uint32_t val) "%"PRIx64" = %08x"
 
-- 
2.3.3

[Qemu-devel] [PATCH 3/3] apic: Implement handling of RH=1 for MSI interrupt delivery

[James Sullivan]

Added argument to apic_get_delivery_bitmask() for msi_redir_hint,
and changed calls to the function accordingly (using 0 as a default
value for non-MSI interrupts).

Modified the implementation of apic_get_delivery_bitmask() to account
for the RH bit of an MSI IRQ. The RH bit indicates that the message
should target only the lowest-priority processor among those specified
by the logical destination of the IRQ.

Signed-off-by: James Sullivan 
---
 hw/intc/apic.c | 34 --
 1 file changed, 20 insertions(+), 14 deletions(-)

diff --git a/hw/intc/apic.c b/hw/intc/apic.c
index 97dd649..7ac60aa 100644
--- a/hw/intc/apic.c
+++ b/hw/intc/apic.c
@@ -37,7 +37,8 @@ static APICCommonState *local_apics[MAX_APICS + 1];
 static void apic_set_irq(APICCommonState *s, int vector_num, int trigger_mode);
 static void apic_update_irq(APICCommonState *s);
 static void apic_get_delivery_bitmask(uint32_t *deliver_bitmask,
-  uint8_t dest, uint8_t dest_mode);
+  uint8_t dest, uint8_t dest_mode,
+  uint8_t msi_redir_hint);
 
 /* Find first bit starting from msb */
 static int apic_fls_bit(uint32_t value)
@@ -314,7 +315,7 @@ void apic_deliver_irq(uint8_t dest, uint8_t dest_mode, 
uint8_t delivery_mode,
 trace_apic_deliver_irq(dest, dest_mode, delivery_mode, vector_num,
trigger_mode, msi_redir_hint);
 
-apic_get_delivery_bitmask(deliver_bitmask, dest, dest_mode);
+apic_get_delivery_bitmask(deliver_bitmask, dest, dest_mode, 
msi_redir_hint);
 apic_bus_deliver(deliver_bitmask, delivery_mode, vector_num, trigger_mode);
 }
 
@@ -472,7 +473,8 @@ static int apic_find_dest(uint8_t dest)
 }
 
 static void apic_get_delivery_bitmask(uint32_t *deliver_bitmask,
-  uint8_t dest, uint8_t dest_mode)
+  uint8_t dest, uint8_t dest_mode,
+  uint8_t msi_redir_hint)
 {
 APICCommonState *apic_iter;
 int i;
@@ -488,23 +490,27 @@ static void apic_get_delivery_bitmask(uint32_t 
*deliver_bitmask,
 }
 } else {
 /* XXX: cluster mode */
+int l = -1;
 memset(deliver_bitmask, 0x00, MAX_APIC_WORDS * sizeof(uint32_t));
 for(i = 0; i < MAX_APICS; i++) {
 apic_iter = local_apics[i];
-if (apic_iter) {
-if (apic_iter->dest_mode == 0xf) {
-if (dest & apic_iter->log_dest)
-apic_set_bit(deliver_bitmask, i);
-} else if (apic_iter->dest_mode == 0x0) {
-if ((dest & 0xf0) == (apic_iter->log_dest & 0xf0) &&
-(dest & apic_iter->log_dest & 0x0f)) {
-apic_set_bit(deliver_bitmask, i);
+if (!apic_iter) {
+break;
+}
+if (apic_match_dest(apic_iter, dest_mode, dest)) {
+if (msi_redir_hint) {
+if (l < 0 ||
+apic_compare_prio(apic_iter, local_apics[l]) < 0) {
+l = i;
 }
+} else {
+apic_set_bit(deliver_bitmask, i);
 }
-} else {
-break;
 }
 }
+if (msi_redir_hint && l >= 0) {
+apic_set_bit(deliver_bitmask, l);
+}
 }
 }
 
@@ -537,7 +543,7 @@ static void apic_deliver(DeviceState *dev, uint8_t dest, 
uint8_t dest_mode,
 
 switch (dest_shorthand) {
 case 0:
-apic_get_delivery_bitmask(deliver_bitmask, dest, dest_mode);
+apic_get_delivery_bitmask(deliver_bitmask, dest, dest_mode, 0);
 break;
 case 1:
 memset(deliver_bitmask, 0x00, sizeof(deliver_bitmask));
-- 
2.3.3

Re: [Qemu-devel] [Xen-devel] Question about scsi emulation

[Juergen Gross]

On 03/24/2015 01:20 AM, Yaoli Zheng wrote:

Thank you for the advice!
It will be grateful if someone can provide any guide how to config  MegaSAS HBA 
or how to use xen pvscsi in XEN.
There seems no option in XEN for these two driver emulation and no document 
found online.


The pvscsi driver is rather new in the Linux kernel. Support for pvscsi
in Xen tools (xl) is just being worked on:

http://lists.xen.org/archives/html/xen-devel/2015-03/msg00030.html

Juergen



Thanks!

-Original Message-
From: Stefano Stabellini [mailto:stefano.stabell...@eu.citrix.com]
Sent: Monday, March 23, 2015 11:18 AM
To: Stefan Hajnoczi
Cc: Yaoli Zheng; qemu-devel@nongnu.org; xen-de...@lists.xen.org
Subject: Re: [Xen-devel] [Qemu-devel] Question about scsi emulation

On Mon, 23 Mar 2015, Stefan Hajnoczi wrote:

On Wed, Mar 18, 2015 at 02:16:47PM -0700, Yaoli Zheng wrote:

We have problem using qemu emulated scsi driver(the old lsi). Wonder if any of 
other device model we can try for emulating scsi, and how we can get and config 
it in Xen? Having been told virtio-scsi is alternative one, but have no idea 
how to get it work in Xen.


The MegaSAS HBA might be another option.  Not sure whether MegaSAS or
virtio-scsi are supported under Xen though.


Making MegaSAS HBA work on Xen should be easy.

Rather than virtio-scsi I would consider the new Xen pvscsi frontend/backend 
drivers, now upstream in Linux (drivers/scsi/xen-scsifront.c and 
drivers/xen/xen-scsiback.c). Usually Xen PV drivers perform much better than 
virtio devices on Xen.

That said, probably most things would be better than the old lsi.

___
Xen-devel mailing list
xen-de...@lists.xen.org
http://lists.xen.org/xen-devel

[Qemu-devel] [PATCH] Haiku: Platform build fixes

[Alexander von Gluck IV]

* skip syscall.h on Haiku
* skip signal.h on Haiku
* no daemon function
* only attach SIGIO when it exists
* use termios.h on Haiku
---
 main-loop.c |2 +
 os-posix.c  |4 ++
 target-xtensa/xtensa-semi.c |   84 ---
 tests/Makefile  |2 +-
 util/compatfd.c |2 +
 util/oslib-posix.c  |7 
 util/qemu-openpty.c |4 ++-
 7 files changed, 58 insertions(+), 47 deletions(-)

diff --git a/main-loop.c b/main-loop.c
index 981bcb5..947d7cd 100644
--- a/main-loop.c
+++ b/main-loop.c
@@ -81,7 +81,9 @@ static int qemu_signal_init(void)
  */
 sigemptyset(&set);
 sigaddset(&set, SIG_IPI);
+#ifdef SIGIO
 sigaddset(&set, SIGIO);
+#endif
 sigaddset(&set, SIGALRM);
 sigaddset(&set, SIGBUS);
 /* SIGINT cannot be handled via signalfd, so that ^C can be used
diff --git a/os-posix.c b/os-posix.c
index ba091f1..43f7fec 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -323,6 +323,9 @@ bool is_daemonized(void)
 
 int os_mlock(void)
 {
+#if defined(CONFIG_HAIKU)
+return ENOSYS;
+#else
 int ret = 0;
 
 ret = mlockall(MCL_CURRENT | MCL_FUTURE);
@@ -331,4 +334,5 @@ int os_mlock(void)
 }
 
 return ret;
+#endif
 }
diff --git a/target-xtensa/xtensa-semi.c b/target-xtensa/xtensa-semi.c
index 16e9d8c..d0ea12a 100644
--- a/target-xtensa/xtensa-semi.c
+++ b/target-xtensa/xtensa-semi.c
@@ -95,59 +95,53 @@ enum {
 
 static uint32_t errno_h2g(int host_errno)
 {
-static const uint32_t guest_errno[] = {
-[EPERM] = TARGET_EPERM,
-[ENOENT]= TARGET_ENOENT,
-[ESRCH] = TARGET_ESRCH,
-[EINTR] = TARGET_EINTR,
-[EIO]   = TARGET_EIO,
-[ENXIO] = TARGET_ENXIO,
-[E2BIG] = TARGET_E2BIG,
-[ENOEXEC]   = TARGET_ENOEXEC,
-[EBADF] = TARGET_EBADF,
-[ECHILD]= TARGET_ECHILD,
-[EAGAIN]= TARGET_EAGAIN,
-[ENOMEM]= TARGET_ENOMEM,
-[EACCES]= TARGET_EACCES,
-[EFAULT]= TARGET_EFAULT,
+switch (host_errno) {
+case 0: return 0;
+case EPERM: return TARGET_EPERM;
+case ENOENT: return TARGET_ENOENT;
+case ESRCH: return TARGET_ESRCH;
+case EINTR: return TARGET_EINTR;
+case EIO: return TARGET_EIO;
+case ENXIO: return TARGET_ENXIO;
+case E2BIG: return TARGET_E2BIG;
+case ENOEXEC: return TARGET_ENOEXEC;
+case EBADF: return TARGET_EBADF;
+case ECHILD: return TARGET_ECHILD;
+case EAGAIN: return TARGET_EAGAIN;
+case ENOMEM: return TARGET_ENOMEM;
+case EACCES: return TARGET_EACCES;
+case EFAULT: return TARGET_EFAULT;
 #ifdef ENOTBLK
-[ENOTBLK]   = TARGET_ENOTBLK,
+case ENOTBLK: return TARGET_ENOTBLK;
 #endif
-[EBUSY] = TARGET_EBUSY,
-[EEXIST]= TARGET_EEXIST,
-[EXDEV] = TARGET_EXDEV,
-[ENODEV]= TARGET_ENODEV,
-[ENOTDIR]   = TARGET_ENOTDIR,
-[EISDIR]= TARGET_EISDIR,
-[EINVAL]= TARGET_EINVAL,
-[ENFILE]= TARGET_ENFILE,
-[EMFILE]= TARGET_EMFILE,
-[ENOTTY]= TARGET_ENOTTY,
+case EBUSY: return TARGET_EBUSY;
+case EEXIST: return TARGET_EEXIST;
+case EXDEV: return TARGET_EXDEV;
+case ENODEV: return TARGET_ENODEV;
+case ENOTDIR: return TARGET_ENOTDIR;
+case EISDIR: return TARGET_EISDIR;
+case EINVAL: return TARGET_EINVAL;
+case ENFILE: return TARGET_ENFILE;
+case EMFILE: return TARGET_EMFILE;
+case ENOTTY: return TARGET_ENOTTY;
 #ifdef ETXTBSY
-[ETXTBSY]   = TARGET_ETXTBSY,
+case ETXTBSY: return TARGET_ETXTBSY;
 #endif
-[EFBIG] = TARGET_EFBIG,
-[ENOSPC]= TARGET_ENOSPC,
-[ESPIPE]= TARGET_ESPIPE,
-[EROFS] = TARGET_EROFS,
-[EMLINK]= TARGET_EMLINK,
-[EPIPE] = TARGET_EPIPE,
-[EDOM]  = TARGET_EDOM,
-[ERANGE]= TARGET_ERANGE,
-[ENOSYS]= TARGET_ENOSYS,
+case EFBIG: return TARGET_EFBIG;
+case ENOSPC: return TARGET_ENOSPC;
+case ESPIPE: return TARGET_ESPIPE;
+case EROFS: return TARGET_EROFS;
+case EMLINK: return TARGET_EMLINK;
+case EPIPE: return TARGET_EPIPE;
+case EDOM: return TARGET_EDOM;
+case ERANGE: return TARGET_ERANGE;
+case ENOSYS: return TARGET_ENOSYS;
 #ifdef ELOOP
-[ELOOP] = TARGET_ELOOP,
+case ELOOP: return TARGET_ELOOP;
 #endif
 };
 
-if (host_errno == 0) {
-return 0;
-} else if (host_errno > 0 && host_errno < ARRAY_SIZE(guest_errno) &&
-guest_errno[host_errno]) {
-return guest_errno[host_errno];
-} else {
-return TARG

[Qemu-devel] [PATCH 0/3] apic: Implement handling for MSI redirection hint in IRQ delivery

[James Sullivan]

This set of patches implements handling for the MSI redirection hint
bit. The RH bit is used in logical destination mode to indicate that
the delivery of the interrupt shall only be to the lowest priority
candidate LAPIC.

Currently, there is no handling of the MSI RH bit. This patch implements 
the following logic:

* DM=0, RH=*  : Physical destination mode. Interrupt is delivered to
the LAPIC with the matching APIC ID. (Subject to
the usual restrictions, i.e. no broadcast dest)
* DM=1, RH=0  : Logical destination mode without redirection. Interrupt
is delivered to all LAPICs in the logical group 
specified by the IRQ's destination map and delivery
mode.
* DM=1, RH=1  : Logical destination mode with redirection. Interrupt
is delivered only to the lowest priority LAPIC in the 
logical group specified by the dest map and the
delivery mode. Delivery semantics are otherwise
specified by the delivery_mode of the IRQ, which
is unchanged.

These changes reflect those made in the KVM in
http://www.spinics.net/lists/kvm/msg114915.html ("kvm: x86: Implement
handling of RH=1 for MSI delivery in KVM"), which are under review but
are largely settled in terms of functionality.


James Sullivan (3):
  apic: Added helper function apic_match_dest,
apic_match_[physical,logical]_dest
  apic: Set and pass in RH bit for MSI interrupts
  apic: Implement handling of RH=1 for MSI interrupt delivery

 hw/intc/apic.c | 70 +-
 hw/intc/ioapic.c   |  2 +-
 include/hw/i386/apic.h |  3 ++-
 trace-events   |  2 +-
 4 files changed, 56 insertions(+), 21 deletions(-)

-- 
2.3.3

[Qemu-devel] Qemu 2.2.1 black screen of death in windows 2012 R2

[Stefan Priebe - Profihost AG]

Hi,

after upgrading Qemu from 2.2.0 to 2.2.1

Windows 2012 R2 works after installing. But after applying 72 updates it
breaks with a black screen of death.

Linking to this KB:
https://support.microsoft.com/en-us/kb/2939259

It works fine with Qemu 2.2.0

Greets,
Stefan

Re: [Qemu-devel] One question to lowlevel/xl/xl.c and lowlevel/xc/xc.c

[Ian Campbell]

On Tue, 2015-03-24 at 18:31 +0800, Chen, Tiejun wrote:
> > NB, the libxl ones are broken and not even compiled right now, you can
> > ignore them.
> 
> Looks this is still compiled now.

xc is, xl is not, I am sure of that.

> > I don't know what the semantics of flag is, if it is per SBDF then I
> 
> Yes, this should be a flag specific to a SBDF.
> 
> You know, I'm working to fix RMRR completely. Based on some discussion 
> about that design ( I assume you may read that thread previously :) ), 
> now we probably need to pass a flag to introduce our policy.

Unless you have a concrete requirement to expose RMRR via the Python
bindings to libxc (i.e. you know somebody is using them) then I think
you should not bother.

Making RMRR work via the (C) interface to libxl used by xl and libvirt
is sufficient for a new in tree feature.

Ian.
> > suppose if you really wanted to expose this here then you would need to
> > invent some syntax for doing so.
> >
> 
> Definitely.
> 
> When I finish this I will send you to review technically.
> 
> Again, really appreciate your clarification to me.
> 
> Thanks
> Tiejun

Re: [Qemu-devel] Qemu 2.2.1 black screen of death in windows 2012 R2

[Paolo Bonzini]

On 24/03/2015 11:39, Stefan Priebe - Profihost AG wrote:
> after upgrading Qemu from 2.2.0 to 2.2.1
> 
> Windows 2012 R2 works after installing. But after applying 72 updates it
> breaks with a black screen of death.

Can you bisect it?

> Linking to this KB:
> https://support.microsoft.com/en-us/kb/2939259

That KB, I think, refers to running a hypervisor (e.g. VMware
Workstation) _inside_ Windows.

Paolo

> It works fine with Qemu 2.2.0

Re: [Qemu-devel] [RESEND PATCH v4 6/6] acpi: Add hardware implementation for memory hot unplug

[Zhu Guihua]

On 03/24/2015 06:31 PM, Igor Mammedov wrote:

On Tue, 24 Mar 2015 17:38:53 +0800
Zhu Guihua  wrote:


On 03/16/2015 10:59 PM, Igor Mammedov wrote:

On Mon, 16 Mar 2015 16:58:18 +0800
Zhu Guihua  wrote:


This patch adds a new bit to memory hotplug IO port indicating that

actually bit was added in 2/6 where is_removing had been added.


EJ0 has been evaluated by guest OS. And call pc-dimm unplug cb to do
the real removal.

Signed-off-by: Zhu Guihua 
---
   docs/specs/acpi_mem_hotplug.txt   | 11 +--
   hw/acpi/memory_hotplug.c  | 21 +++--
   hw/core/qdev.c|  2 +-
   hw/i386/acpi-build.c  |  9 +
   hw/i386/acpi-dsdt-mem-hotplug.dsl | 10 ++
   include/hw/acpi/pc-hotplug.h  |  2 ++
   include/hw/qdev-core.h|  1 +
   trace-events  |  1 +
   8 files changed, 52 insertions(+), 5 deletions(-)

diff --git a/docs/specs/acpi_mem_hotplug.txt b/docs/specs/acpi_mem_hotplug.txt
index 1290994..85cd4b8 100644
--- a/docs/specs/acpi_mem_hotplug.txt
+++ b/docs/specs/acpi_mem_hotplug.txt
@@ -19,7 +19,9 @@ Memory hot-plug interface (IO port 0xa00-0xa17, 1-4 byte 
access):
 1: Device insert event, used to distinguish device for which
no device check event to OSPM was issued.
It's valid only when bit 1 is set.
-  2-7: reserved and should be ignored by OSPM
+  2: Device remove event, used to distinguish device for which
+ no device check event to OSPM was issued.
+  3-7: reserved and should be ignored by OSPM
 [0x15-0x17] reserved
   
 write access:

@@ -35,7 +37,12 @@ Memory hot-plug interface (IO port 0xa00-0xa17, 1-4 byte 
access):
 1: if set to 1 clears device insert event, set by OSPM
after it has emitted device check event for the
selected memory device
-  2-7: reserved, OSPM must clear them before writing to register
+  2: if set to 1 clears device remove event, set by OSPM
+ after it has emitted device check event for the
+ selected memory device. if guest fails to eject device, it
+ should send OST event about it and forget about device
+ removal.
+  3-7: reserved, OSPM must clear them before writing to register
   
   Selecting memory device slot beyond present range has no effect on platform:

  - write accesses to memory hot-plug registers not documented above are
diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
index 687b2f1..d6b8c89 100644
--- a/hw/acpi/memory_hotplug.c
+++ b/hw/acpi/memory_hotplug.c
@@ -2,6 +2,7 @@
   #include "hw/acpi/pc-hotplug.h"
   #include "hw/mem/pc-dimm.h"
   #include "hw/boards.h"
+#include "hw/qdev-core.h"
   #include "trace.h"
   #include "qapi-event.h"
   
@@ -91,6 +92,8 @@ static void acpi_memory_hotplug_write(void *opaque, hwaddr addr, uint64_t data,

   MemHotplugState *mem_st = opaque;
   MemStatus *mdev;
   ACPIOSTInfo *info;
+DeviceState *dev = NULL;
+HotplugHandler *hotplug_ctrl = NULL;
   
   if (!mem_st->dev_count) {

   return;
@@ -122,19 +125,33 @@ static void acpi_memory_hotplug_write(void *opaque, 
hwaddr addr, uint64_t data,
   mdev = &mem_st->devs[mem_st->selector];
   mdev->ost_status = data;
   trace_mhp_acpi_write_ost_status(mem_st->selector, mdev->ost_status);
-/* TODO: implement memory removal on guest signal */
   
   info = acpi_memory_device_status(mem_st->selector, mdev);

   qapi_event_send_acpi_device_ost(info, &error_abort);
   qapi_free_ACPIOSTInfo(info);
   break;
-case 0x14:
+case 0x14: /* set is_* fields */
   mdev = &mem_st->devs[mem_st->selector];
   if (data & 2) { /* clear insert event */
   mdev->is_inserting  = false;
   trace_mhp_acpi_clear_insert_evt(mem_st->selector);
+} else if (data & 4) { /* request removal of device */

fix comment to match docs above.


+mdev->is_removing = false;
+trace_mhp_acpi_clear_remove_evt(mem_st->selector);

just clear event here and don't do removal part as it doesn't match
documentation you've written above regarding this field.

It would be better to move is_removing handling from here to 2/6
+ related ASL code from DSDT which should clear it after sending device check.


+/*
+ * QEMU memory hot unplug is an asynchronous procedure. QEMU first
+ * calls pc-dimm unplug request cb to send a SCI to guest. When the
+ * guest OS finished handling the SCI, it evaluates ACPI EJ0, and
+ * QEMU calls pc-dimm unplug cb to remove memory device.
+ */

something like this comment, should be in acpi_mem_hotplug.txt not here.


There is 'is_enabled' field, which is 1 if device is pre

Re: [Qemu-devel] [Bug 1405385] Re: QEMU crashes when virtio network cards are used together with e1000 network cards

[Stefan Hajnoczi]

On Fri, Jan 09, 2015 at 07:30:05AM -, Bram Klein Gunnewiek wrote:
> I'm not sure if there is more information required from my side? I can
> still reproduce this and have no clue where to look for more
> information.

I cannot reproduce a crash from your command-line with qemu.git/master
(3e5f6234b4f45a11b7c357dde2d6da36641bc6f6).


pgpsX9Kaub9cc.pgp
Description: PGP signature

Re: [Qemu-devel] [Bug 1404278] Re: tap connections not working on windows host

[Stefan Hajnoczi]

On Wed, Jan 07, 2015 at 04:09:55PM -, timsoft wrote:
> I have tried what you suggested (breaking the bridge on the host, and giving 
> the host tap 192.168.5.1 and the guest eth0 192.168.5.2
> and tried pinging one from the other. I get 100% packet loss.
> This points to QEMU's tap networking as far as I can see.
> I have tried uninstalling the 64 bit version and installing the 32bit tap 
> adapter (and bridging it) from openvpn 2.3.6-I601  but that didn't seem to 
> make any difference.

You could put a fprintf(stderr, "Receiving a packet\n") into
net/tap-win32.c:tap_win32_send() as well as fprintf(stderr, "Sending a
packet\n") into tap_win32_write() if you think QEMU's Windows tap code
is broken.

Stefan


pgp_7Gz8TyESz.pgp
Description: PGP signature

Re: [Qemu-devel] [PULL 0/4] tricore-patches for 2.3-rc1

[Peter Maydell]

On 24 March 2015 at 08:58, Bastian Koppelmann
 wrote:
> The following changes since commit 362ca922eea03240916287a8a6267801ab095d12:
>
>   Merge remote-tracking branch 'remotes/jnsnow/tags/ide-pull-request' into 
> staging (2015-03-23 17:02:12 +)
>
> are available in the git repository at:
>
>   https://github.com/bkoppelmann/qemu-tricore-upstream.git 
> tags/pull-tricore-20150324
>
> for you to fetch changes up to f69c24e4584f2161f90ee7caba38728aa77f937f:
>
>   target-tricore: properly fix dvinit_b/h_13 (2015-03-24 09:45:28 +0100)
>
> 
> TriCore bugfixes for 2.3-rc1
>
> 
> Bastian Koppelmann (3):
>   target-tricore: fix DVINIT_HU/BU calculating overflow before result
>   target-tricore: fix RRPW_DEXTR using wrong reg
>   target-tricore: properly fix dvinit_b/h_13
>
> Stefan Weil (1):
>   target-tricore: Fix two helper functions (clang warnings)

Applied, thanks.

-- PMM

Re: [Qemu-devel] Qemu 2.2.1 black screen of death in windows 2012 R2

[Stefan Priebe - Profihost AG]

Am 24.03.2015 um 11:45 schrieb Paolo Bonzini:
> 
> 
> On 24/03/2015 11:39, Stefan Priebe - Profihost AG wrote:
>> after upgrading Qemu from 2.2.0 to 2.2.1
>>
>> Windows 2012 R2 works after installing. But after applying 72 updates it
>> breaks with a black screen of death.
> 
> Can you bisect it?

Have to try might be possible.

>> Linking to this KB:
>> https://support.microsoft.com/en-us/kb/2939259
> 
> That KB, I think, refers to running a hypervisor (e.g. VMware
> Workstation) _inside_ Windows.

Screenshot of the windows screen attached. This is just a blank windows
2012 R2 with virtio 94 drivers installed.

Stefan

Re: [Qemu-devel] [PATCH] savevm: create snapshot failed when id_str already exits

[Stefan Hajnoczi]

On Thu, Mar 12, 2015 at 11:29:38PM +0800, Yi Wang wrote:
> How about this?
> 
> From 913cf2cd04167b7f6b892ac1ab405a617d886b97 Mon Sep 17 00:00:00 2001
> From: Yi Wang 
> Date: Thu, 12 Mar 2015 22:54:42 +0800
> Subject: [PATCH] savevm: create snapshot failed when id_str already exists
> 
> The command "virsh create" will fail in such condition: vm has two
> disks: vda and vdb. vda has snapshot s1 with id "1", vdb doesn't have
> s1 but has snapshot s2 with id "1"。When we want to run command "virsh
> create s1", del_existing_snapshots() only deletes s1 in vda, and
> bdrv_snapshot_create() tries to create vdb's snapshot s1 with id "1",
> but id "1" alreay exists in vdb with name "s2"!
> 
> The simplest way is call find_new_snapshot_id() unconditionally.
> 
> Signed-off-by: Yi Wang 
> ---
>  block/qcow2-snapshot.c | 6 ++
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
> index 5b3903c..cb00f56 100644
> --- a/block/qcow2-snapshot.c
> +++ b/block/qcow2-snapshot.c
> @@ -351,10 +351,8 @@ int qcow2_snapshot_create(BlockDriverState *bs,
> QEMUSnapshotInfo *sn_info)
> 
>  memset(sn, 0, sizeof(*sn));
> 
> -/* Generate an ID if it wasn't passed */
> -if (sn_info->id_str[0] == '\0') {
> -find_new_snapshot_id(bs, sn_info->id_str, sizeof(sn_info->id_str));
> -}
> +/* Generate an ID */
> +find_new_snapshot_id(bs, sn_info->id_str, sizeof(sn_info->id_str));
> 
>  /* Check that the ID is unique */
>  if (find_snapshot_by_id_and_name(bs, sn_info->id_str, NULL) >= 0) {
> -- 
> 1.9.5.msysgit.0

Thanks, I've applied the patch to the block-next branch for QEMU 2.4:
https://github.com/stefanha/qemu/commits/block-next

Please send future patches as separate top-level email threads using
git-send-email(1).  I missed this reply since it was not a new email
thread.  git-am(1) refuses to apply the patch because of line-wrapping
in your email, I had to fix that manually.

Also, please stick to ASCII characters when possible.  Your patch
description uses the Chinese full stop (句號).  I had to manually edit
the patch because git-am(1) refuses to apply a copy-pasted patch without
explicit character encoding information.

Stefan


pgp4YczfQu1y4.pgp
Description: PGP signature

Re: [Qemu-devel] [PATCH for-2.3 0/3] Contain drive_get() misuse

[Markus Armbruster]

Adding Andreas because he's the odd fixer for pc87312.

Paolo Bonzini  writes:

> On 23/03/2015 20:09, Markus Armbruster wrote:
>> Drives defined with if!=none are for board initialization to wire up.
>> Board code calls drive_get() or similar to find them, and creates
>> devices with their qdev drive properties set accordingly.
>> 
>> Except a few devices go on a fishing expedition for a suitable backend
>> instead of exposing a drive property for board code to set: they call
>> driver_get() or drive_get_next() in their realize() or init() method.
>> Wrong.
>> 
>> We can't fix this in time for the release, so do the next best thing:
>> contain the mistakes as far as possible so they don't become ABI:
>> 
>> * Mark them all with suitable FIXME comments [PATCH 1]
>> 
>> * sdhci-pci is new, set cannot_instantiate_with_device_add_yet to make
>>   it unavailable with -device [PATCH 2]
>> 
>> * A few more aren't currently available with -device, set
>>   cannot_instantiate_with_device_add_yet to ensure they stay
>>   unavailable [PATCH 3]
>> 
>> * Left alone: m25p80-generic and its derivatives, ssi-sd, pc87312
>
> Maybe worth documenting as future incompatible changes?  These machines
> are not versioned, so it's not the end of the world to make things saner
> if somebody comes and qdevifies the SD card.

Two questions: what exactly is going to change, and where do we want to
document it?

On the former:

Use of -drive if=floppy with onboard pc87312 (machine "prep") shouldn't
be affected.  Likewise for connecting onboard m25p80-generic derivatives
with if=mtd drives, or onboard ssi-sd with if=sd.

Weird usage similar to the one you caught in time for sdhci-pci (--drive
if=sd --device sdhci-pci) would break.  It's possible when the target
has the device, and the machine type has a suitable bus.

* pc87312

  Depends on CONFIG_PC87312, set in {ppc,ppc64}-softmmu.mak.

  Requires an ISA bus.  I believe "prep" is the only machine providing
  one.  Adding a second one with --device seems unlikely to work (I
  didn't try).  If that's correct, there's nothing to document.

  If Andreas agrees, I can set cannot_instantiate_with_device_add_yet
  for pc87312 now.

* ssi-sd

  Depends on CONFIG_SSI_SD, set in arm-softmmu.mak.

  Requires an SSI bus.  At least the following targets provide one:
  collie, highbank, lm3s6965evb, lm3s811evb, midway, xilinx-zynq-a9.
  Can't exclude use of --device ssi-sd.

  I guess we want to document that --device ssi-sd will at some point
  cease to auto-connect to the next available if=sd drive and require
  the usual drive property instead.  Okay?

* m25p80-generic
 
  Depends on CONFIG_SSI_M25P80, set in
  {arm,microblaze,microblazeel}-softmmu.mak.

  Requires an SSI bus.  At least the ARM targets above provide one.
  Can't exclude use with --device.

  Document just like ssi-sd.

[...]
> Acked-by: Paolo Bonzini 

Thanks!

Re: [Qemu-devel] [Qemu-ppc] [PATCH v2 1/3] VFIO: Clear stale MSIx table during EEH reset

[Alex Williamson]

On Tue, 2015-03-24 at 17:54 +1100, David Gibson wrote:
> On Tue, Mar 24, 2015 at 05:24:55PM +1100, Gavin Shan wrote:
> > On Tue, Mar 24, 2015 at 04:41:21PM +1100, David Gibson wrote:
> > >On Mon, Mar 23, 2015 at 04:25:10PM +1100, Gavin Shan wrote:
> > >> On Mon, Mar 23, 2015 at 04:06:56PM +1100, David Gibson wrote:
> > >> >On Fri, Mar 20, 2015 at 05:27:29PM +1100, Gavin Shan wrote:
> > >> >> On Fri, Mar 20, 2015 at 05:04:01PM +1100, David Gibson wrote:
> > >> >> >On Tue, Mar 17, 2015 at 03:31:24AM +1100, Gavin Shan wrote:
> > >> >> >> The PCI device MSIx table is cleaned out in hardware after EEH PE
> > >> >> >> reset. However, we still hold the stale MSIx entries in QEMU, which
> > >> >> >> should be cleared accordingly. Otherwise, we will run into another
> > >> >> >> (recursive) EEH error and the PCI devices contained in the PE have
> > >> >> >> to be offlined exceptionally.
> > >> >> >> 
> > >> >> >> The patch clears stale MSIx table before EEH PE reset so that MSIx
> > >> >> >> table could be restored properly after EEH PE reset.
> > >> >> >> 
> > >> >> >> Signed-off-by: Gavin Shan 
> > >> >> >> ---
> > >> >> >> v2: vfio_container_eeh_event() stub for !CONFIG_PCI and separate
> > >> >> >> error message for this function. Dropped vfio_put_group()
> > >> >> >> on NULL group
> > >> >> >> ---
> > >> >> >>  hw/vfio/Makefile.objs  |  6 +-
> > >> >> >>  hw/vfio/common.c   |  7 +++
> > >> >> >>  hw/vfio/pci-stub.c | 17 +
> > >> >> >>  hw/vfio/pci.c  | 38 ++
> > >> >> >>  include/hw/vfio/vfio.h |  2 ++
> > >> >> >>  5 files changed, 69 insertions(+), 1 deletion(-)
> > >> >> >>  create mode 100644 hw/vfio/pci-stub.c
> > >> >> >> 
> > >> >> >> diff --git a/hw/vfio/Makefile.objs b/hw/vfio/Makefile.objs
> > >> >> >> index e31f30e..1b8a065 100644
> > >> >> >> --- a/hw/vfio/Makefile.objs
> > >> >> >> +++ b/hw/vfio/Makefile.objs
> > >> >> >> @@ -1,4 +1,8 @@
> > >> >> >>  ifeq ($(CONFIG_LINUX), y)
> > >> >> >>  obj-$(CONFIG_SOFTMMU) += common.o
> > >> >> >> -obj-$(CONFIG_PCI) += pci.o
> > >> >> >> +ifeq ($(CONFIG_PCI), y)
> > >> >> >> +obj-y += pci.o
> > >> >> >> +else
> > >> >> >> +obj-y += pci-stub.o
> > >> >> >> +endif
> > >> >> >>  endif
> > >> >> >> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> > >> >> >> index 148eb53..ed07814 100644
> > >> >> >> --- a/hw/vfio/common.c
> > >> >> >> +++ b/hw/vfio/common.c
> > >> >> >> @@ -949,7 +949,14 @@ int vfio_container_ioctl(AddressSpace *as, 
> > >> >> >> int32_t groupid,
> > >> >> >>  switch (req) {
> > >> >> >>  case VFIO_CHECK_EXTENSION:
> > >> >> >>  case VFIO_IOMMU_SPAPR_TCE_GET_INFO:
> > >> >> >> +break;
> > >> >> >>  case VFIO_EEH_PE_OP:
> > >> >> >> +if (vfio_container_eeh_event(as, groupid, param) != 0) {
> > >> >> >
> > >> >> >I really dislike the idea of having an arbitrarily complex side 
> > >> >> >effect
> > >> >> >from a function whose name suggest's it's just a trivial wrapper
> > >> >> >around the ioctl().
> > >> >> >
> > >> >> 
> > >> >> Ok. I guess you would like putting the complex in the callers of
> > >> >> vfio_container_ioctl().
> > >> >
> > >> >Well.. maybe.  I'd also be happy if helper functions were implemeneted
> > >> >which both called the ioctl() and did the other necessary pieces.
> > >> >They should just be called something that indicates their full
> > >> >function, not a name which suggests they're just an ioctl wrapper.
> > >> >
> > >> 
> > >> Indeed, vfio_container_ioctl() isn't indicating what the function is 
> > >> doing.
> > >> How about renaming it to vfio_container_event_and_ioctl()? I'm always bad
> > >> at giving a good function name :)
> > >
> > >Well, I don't think your wrapper should be multiplexed.  The multiplex
> > >works for the simple ioctl() wrapper, because there really is nothing
> > >that varies apart from the exact ioctl number called.
> > >
> > >But now that you have different operations here, I think you want
> > >wrappers for each one - each one will call the ioctl(), then do the
> > >specific extra steps necessary for that operation.  So
> > >vfio_container_event() will go away as well, split into various other
> > >functions.
> > >
> > 
> > It wouldn't a good idea if I understand your proposal correctly. Currnetly,
> > the global function vfio_container_ioctl() can be called from sPAPR platform
> > for any ioctl commands handled in kernel source file vfio_iommu_spapr_tce.c,
> > which means the function isn't called for EEH only. Other sPAPR TCE 
> > container
> > ioctl commands are also routed by this function. There will be lots if 
> > having
> > one global function for each ioctl commands, which just improve the cost to
> > maintain the code.
> 
> I don't really follow your objection.  I'm only suggesting separate
> wrappers for things which require extra actions currently implemented
> in vfio_container_event().  Things which only ned the plain ioctl()
> can still use the simple vfio_contai

Re: [Qemu-devel] [PATCH] MAINTAINERS: Add myself as the maintainer of the Quorum driver

[Stefan Hajnoczi]

On Mon, Mar 16, 2015 at 06:22:05PM +0200, Alberto Garcia wrote:
> Signed-off-by: Alberto Garcia 
> ---
>  MAINTAINERS | 5 +
>  1 file changed, 5 insertions(+)

Thanks, applied to my block-next tree:
https://github.com/stefanha/qemu/commits/block-next

Stefan


pgpzd3tmoRxE7.pgp
Description: PGP signature

Re: [Qemu-devel] [PATCH v2] block: Switch to host monotonic clock for IO throttling

[Stefan Hajnoczi]

On Tue, Mar 24, 2015 at 09:17:38AM +0800, Fam Zheng wrote:
> Alberto saw that this patch also fixes the odd behavior: block jobs, which 
> need
> to R/W a throttled BDS, will not make progress if VCPU is not running. If we
> don't consider this as a bug, we should document the inconsistency 
> (confusion):
> if no throttling, they DO make progress after VCPU stopped.

This is a strong justification for the patch.  Please make this the main
point in the commit description.

Stefan


pgp45bB41Eaw3.pgp
Description: PGP signature

Re: [Qemu-devel] Output of fprintf messages in qemu source

[Toms Varghese]

In my Fedora, I couldn't find the file in the mentioned path. I got it
finally from
/var/log/libvirt/qemu/.log
Thanks a lot for the help.

Regards
Toms Varghese

On Mon, Mar 23, 2015 at 6:17 PM, Stefan Hajnoczi  wrote:

> On Thu, Mar 19, 2015 at 12:17:29AM +0530, Toms Varghese wrote:
> > Hi all,
> >
> > I can see some fprintf (stderr, ...) statements in qemu source code. I
> > expect the output to be shown in  the terminal window. I use libvirt to
> > manage my VMs and hence have no idea about the exact qemu command line
> > being used. From the comments in the below post
> >
> http://blog.vmsplice.net/2011/03/how-to-write-trace-analysis-scripts-for.html
> > I found that libvirt is supposed to store the stderr outputs
> > in /var/lib/libvirt/qemu/.log file. But I am unable to find any such log
> > file in /var/lib/libvirt/qemu. Can anyone please tell me how to get the
> > output of fprintfs in this case as it would help me a lot?
> > I am using fedora 21, qemu 2.0.2 and libvirt 1.2.9.2.
>
> The file name is:
> /var/lib/libvirt/qemu/.log
>
> It includes the QEMU command-line as well as stderr output.
>
> Stefan
>

Re: [Qemu-devel] [PATCH v3 7/9] omap_intc: convert ffs(3) to ctz32() in omap_inth_sir_update()

[Stefan Hajnoczi]

On Mon, Mar 23, 2015 at 04:40:26PM +0100, Paolo Bonzini wrote:
> 
> 
> On 23/03/2015 16:29, Stefan Hajnoczi wrote:
> > From: Paolo Bonzini 
> > 
> > The loop previously terminated on ffs(0) == 0, now it terminates on
> > ctz32(0) + 1 == 33.
> 
> ... now it terminates on level == 0.

Old commit description.  I'll update it when it gets merged.

Thanks!


pgpwrPF1W26t3.pgp
Description: PGP signature

Re: [Qemu-devel] [PATCH v2 0/2] block: Fix unaligned zero write

[Stefan Hajnoczi]

On Tue, Mar 24, 2015 at 09:23:48AM +0800, Fam Zheng wrote:
> This fixes a segfault when doing unaligned zero write to an image that is 4k
> aligned.
> 
> v2: Don't drop the unset of UNMAP flag. [Stefan, Peter]
> 
> Reproducer:
> 
> $ (echo "open -o file.align=4k blkdebug::img"; echo "write -z 512 1024") 
> | qemu-io
> 
> Fam Zheng (2):
>   block: Fix unaligned zero write
>   qemu-iotests: Test unaligned 4k zero write
> 
>  block.c| 45 ++--
>  tests/qemu-iotests/033 | 47 
> +-
>  tests/qemu-iotests/033.out | 26 +
>  3 files changed, 95 insertions(+), 23 deletions(-)

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan


pgpyIxm1dKf0O.pgp
Description: PGP signature

Re: [Qemu-devel] RFC: memory API changes

[Peter Maydell]

On 23 March 2015 at 12:24, Peter Maydell  wrote:
> (This is part of the work I'm doing for transaction attributes.)

OK, here's try 2, based on feedback on the first proposal:

 * address_space_rw &c remain with their current names, but
   take an extra MemTxAttrs argument and return MemTxResult
   rather than bool. (The latter conveniently doesn't require changes to
   callsites because conversion to bool gives the same true-on-error
   semantics as before.)
   [maybe readbuf/writebuf would be clearer than read/write,
   but it didn't seem sufficiently obvious a win to make the change]
 * the ld/st_*phys functions are renamed as:
 ldl_be_phys -> address_space_ldl_be &c
   and all take MemTxAttrs, *MemTxResult
 * rather than MEMTXATTRS_UNSPECIFIED, use TXATTRS_NONE, so the
   extra arguments (TXATTRS_NONE, NULL) aren't too unwieldy
 * prototypes in memory.h
 * no default-to-no-attrs/etc versions of ld/st*_ phys
   (if in specific devices/buses it's the best thing we should
   have bus-specific dma accessors, as we do for pci)
 * mechanically convert all uses of cpu_physical_memory_* to
   address_space_*(&address_space_memory, ...)

[This leaves the "where do we call the unassigned-access
hooks" problem for a different patchset.]

Is there anything in there people strongly dislike, or
should I start writing coccinelle patches for this?

-- PMM

Re: [Qemu-devel] GSoC Proposal: ARM Virtualization Extensions

[Sergey Fedorov]

On 23.03.2015 04:29, Merten Sach wrote:
> On 21/03/15 04:16, Sergey Fedorov wrote:
>> Hi!
>>
>> I am currently working on AArch64 EL2 support. There is a plan to submit
>> the changes to the community. Merten, IIUYC, you are going to support
>> only AArch32 EL2?
>>
>> Best regards,
>> Sergey
> Hi
>
> Yes, my intention is to focus on AArch32 as I'm more familiar with that
> architecture.
>
> Thanks for the insight into the current state. As I understand, some parts of
> AArch64 and AArch32 EL2 modes share a some functionality. For example,
> * VGIC - compatible
> * Exception Routing - different interface but same function
> * Pagetable format - implemented with LPAE
> * Generic Timer - different interface but same function
>
> Are these assumptions correct? I hope I didn't simplify it to much.
>
> And also: does it make sense to work with your code? Are you planing to commit
> the changes before May?
>
> Would someone mentor this project?
>
> Best regards,
> Merten
>

Hi,

I didn't investigate the difference between AArch64 and AArch32. But I
believe that AArch64 should be better structured. As of GIC and Generic
Timer, I think you are right.

I plan to start committing it in coherent chunks as soon as I can. And
yes, I hope I finish it before May.

Best regards,
Sergey

Re: [Qemu-devel] Qemu 2.2.1 black screen of death in windows 2012 R2

[Stefan Priebe]

Hi,

it started to work again with virtio 100 instead of 94. No idea why it 
works with qemu 2.2.0.


Stefan

Am 24.03.2015 um 12:15 schrieb Stefan Priebe - Profihost AG:


Am 24.03.2015 um 11:45 schrieb Paolo Bonzini:



On 24/03/2015 11:39, Stefan Priebe - Profihost AG wrote:

after upgrading Qemu from 2.2.0 to 2.2.1

Windows 2012 R2 works after installing. But after applying 72 updates it
breaks with a black screen of death.


Can you bisect it?


Have to try might be possible.


Linking to this KB:
https://support.microsoft.com/en-us/kb/2939259


That KB, I think, refers to running a hypervisor (e.g. VMware
Workstation) _inside_ Windows.


Screenshot of the windows screen attached. This is just a blank windows
2012 R2 with virtio 94 drivers installed.

Stefan

Re: [Qemu-devel] PCI passthrough of 40G ethernet interface (Openstack/KVM)

[jacob jacob]

After update to latest firmware and using version 1.2.37 of i40e
driver, things are looking better with PCI passthrough.

]# ethtool -i eth3
driver: i40e
version: 1.2.37
firmware-version: f4.33.31377 a1.2 n4.42 e1930
bus-info: :00:07.0
supports-statistics: yes
supports-test: yes
supports-eeprom-access: yes
supports-register-dump: yes
supports-priv-flags: yes

There are still issues running dpdk 1.8.0 from the VM using the pci
passthrough devices and looks like it puts the devices in a bad state.
i40e driver will not bind after this happens and a host reboot is
required to recover.
I'll post further updates as i make progress.
Thanks for all the help.

On Mon, Mar 23, 2015 at 3:19 AM, Stefan Assmann  wrote:
> On 20.03.2015 21:55, jacob jacob wrote:
>> On Thu, Mar 19, 2015 at 10:18 AM, Stefan Assmann  wrote:
>>> On 19.03.2015 15:04, jacob jacob wrote:
 Hi Stefan,
 have you been able to get PCI passthrough working without any issues
 after the upgrade?
>>>
>>> My XL710 fails to transfer regular TCP traffic (netperf). If that works
>>> for you then you're already one step ahead of me. Afraid I can't help
>>> you there.
>>
>> I have data transfer working when trying the test runs on the host
>> itself. Are you seeing problems when directly trying the TCP traffic
>> from the host itself?
>
> Correct.
>
>> The issues that i am seeing are specific to the case when the devices
>> are passed via PCI passthrough into the VM.
>>
>> Any ideas whether this would be a kvm/qemu or i40e driver issue?
>> (Updating to the latest firmware and using latest i40e driver didn't
>> seem to help.)
>
> Hard to say, that's probably something for Intel to look into.
>
>   Stefan

Re: [Qemu-devel] [PATCH for-2.3 1/1] block: New command line option --misc format-probing=off

[Eric Blake]

On 03/23/2015 02:42 PM, Markus Armbruster wrote:

>>
>> I don't think it's the right solution.  Libvirt knows where to add a
>> format=raw option, and it can do it without waiting for QEMU to
>> implement this.  Direct command-line users are not going to use the
>> option anyway.
> 
> Two separate bones of contention here:
> 
> 1. Do we want to give libvirt the bug insurance it wants?

If we add this in 2.3, libvirt will use it.  If we wait until 2.4,
libvirt will manage without (as it has already managed without for all
earlier releases); it is just a little bit harder to ensure that formats
are being uniformly used.  I'm okay if this topic proves too
controversial to add into 2.3 at this late in the cycle, even though I'm
in favor of adding it.

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [Qemu-block] [PATCH RFC for-2.3 1/1] block: New command line option --no-format-probing

[Eric Blake]

On 03/24/2015 02:37 AM, Paolo Bonzini wrote:

>> The option sets bdrv_image_probing_disabled in a straightforward manner,
>> and bdrv_image_probing_disabled guards the probing code in an equally
>> straightforward manner.
> 
> But what about migration from newer to older QEMU?  Libvirt even
> supports QEMU versions where the only way to specify disks is "-hda
> XYZ", so it is _impossible_ to honor the format=raw specifier.

No one migrates from new qemu with this option back to a qemu version
that old.  Libvirt continues to drive old qemu, but driving old qemu is
different than migrating to old qemu.  And this feature is
introspectible, so libvirt knows when to use it and when to avoid it.

Furthermore, libvirt already has a knob in /etc/libvirt/qemu.conf to
enable probing - if this command line option ever gets in the way, a
one-line change to that conf file will tell libvirt to quit using it.

> 
> Also, libvirt can start qemu-nbd and doesn't force format=raw in that
> case.  So the protection is far from complete.  This reinforces my

Sounds like we have a bug to fix in libvirt.

> opinion that the false sense of safety provided by this patch is worse
> than the "insurance" against future CVEs (also, have there been any
> actual libvirt CVEs about this after 2010?  near misses don't count IMHO).

CVE-2011-2178 (http://security.libvirt.org/2011/0003.html).

And more recently, I argued that
http://security.libvirt.org/2014/0006.html should have been a CVE; it
was no near miss (in the wild for several months), and the only reason I
did not win my case for making it a CVE was because of the qemu.conf
default setting.

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v5 1/6] target-arm: Store SPSR_EL1 state in banked_spsr[1] (SPSR_svc)

[Greg Bellows]

On Mon, Mar 23, 2015 at 12:05 PM, Alex Bennée  wrote:
> From: Peter Maydell 
>
> The AArch64 SPSR_EL1 register is architecturally mandated to
> be mapped to the AArch32 SPSR_svc register. This means its
> state should live in QEMU's env->banked_spsr[1] field.
> Correct the various places in the code that incorrectly
> put it in banked_spsr[0].
>
> Signed-off-by: Peter Maydell 
>
> diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
> index 7e0d038..861f6fa 100644
> --- a/target-arm/helper-a64.c
> +++ b/target-arm/helper-a64.c
> @@ -523,7 +523,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
>  aarch64_save_sp(env, arm_current_el(env));
>  env->elr_el[new_el] = env->pc;
>  } else {
> -env->banked_spsr[0] = cpsr_read(env);
> +env->banked_spsr[aarch64_banked_spsr_index(new_el)] = cpsr_read(env);

Are the other banks (2-5) only used for KVM?  It seems we go out of
our way to manage this larger SPSR array then not use all of the slots
in QEMU itself.

>  if (!env->thumb) {
>  env->cp15.esr_el[new_el] |= 1 << 25;
>  }
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 10886c5..d77c6de 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -2438,7 +2438,7 @@ static const ARMCPRegInfo v8_cp_reginfo[] = {
>  { .name = "SPSR_EL1", .state = ARM_CP_STATE_AA64,
>.type = ARM_CP_ALIAS,
>.opc0 = 3, .opc1 = 0, .crn = 4, .crm = 0, .opc2 = 0,
> -  .access = PL1_RW, .fieldoffset = offsetof(CPUARMState, banked_spsr[0]) 
> },
> +  .access = PL1_RW, .fieldoffset = offsetof(CPUARMState, banked_spsr[1]) 
> },
>  /* We rely on the access checks not allowing the guest to write to the
>   * state field when SPSel indicates that it's being used as the stack
>   * pointer.
> diff --git a/target-arm/internals.h b/target-arm/internals.h
> index bb171a7..2cc3017 100644
> --- a/target-arm/internals.h
> +++ b/target-arm/internals.h
> @@ -82,11 +82,14 @@ static inline void arm_log_exception(int idx)
>
>  /*
>   * For AArch64, map a given EL to an index in the banked_spsr array.
> + * Note that this mapping and the AArch32 mapping defined in bank_number()
> + * must agree such that the AArch64<->AArch32 SPSRs have the architecturally
> + * mandated mapping between each other.
>   */
>  static inline unsigned int aarch64_banked_spsr_index(unsigned int el)
>  {
>  static const unsigned int map[4] = {
> -[1] = 0, /* EL1.  */
> +[1] = 1, /* EL1.  */
>  [2] = 6, /* EL2.  */
>  [3] = 7, /* EL3.  */
>  };
> --
> 2.3.2
>
>

Re: [Qemu-devel] [PATCH RFC 0/4] target-i386: PC socket/core/thread modeling, part 1

[Christian Borntraeger]

Am 23.03.2015 um 18:31 schrieb Andreas Färber:
> Hello,
> 
> This long-postponed series proposes a hierarchical QOM model of socket
> and core objects for the x86 PC machines.

Just some comments from the s390 side as we probably want more than status 
quo in the future as well.

Traditionally all CPUs were equal from the software side, but hardware was
already organized in a node like fashion.

Starting with z10 (2008) the cpu topology was exposed to software (not the
memory topology!) which allowed Linux to optimize scheduling based on
locality.
So on an LPAR you might see something like
$ lscpu -e
CPU BOOK SOCKET CORE L1d:L1i:L2 ONLINE CONFIGURED POLARIZATION ADDRESS
0   00  00:0:0  ja ja horizontal   0
1   00  11:1:1  ja ja horizontal   1
2   00  22:2:2  ja ja horizontal   2
3   01  33:3:3  ja ja horizontal   3
4   01  44:4:4  ja ja horizontal   4
5   01  55:5:5  ja ja horizontal   5
6   02  66:6:6  ja ja horizontal   6
7   02  77:7:7  ja ja horizontal   7
8   02  88:8:8  ja ja horizontal   8
9   03  99:9:9  ja ja horizontal   9
10  03  10   10:10:10   ja ja horizontal   10
11  03  11   11:11:11   ja ja horizontal   11
12  14  12   12:12:12   ja ja horizontal   12
13  14  13   13:13:13   ja ja horizontal   13
14  14  14   14:14:14   ja ja horizontal   14
15  15  15   15:15:15   ja ja horizontal   15


Under z/VM the topology is flat (each CPU on a separate node)
# lscpu -e
CPU BOOK SOCKET CORE L1d:L1i:L2d:L2i ONLINE CONFIGURED POLARIZATION ADDRESS
0   00  00:0:0:0 ja ja horizontal   0
1   11  11:1:1:1 ja ja horizontal   1
2   22  22:2:2:2 ja ja horizontal   2

Thats what we have today in KVM as well. Now...
> 
> Background is that due to qdev limitations we had to introduce an ICC bus
> to be able to hot-add CPUs and their APICs. By now this limitation could be
> resolved via a QOM hotplug handler interface.
> 
> However, the QOM hotplug model is associated with having link<> properties.
> Given that physically we cannot hot-add hyperthreads but full CPU sockets,
> this series prepares the underbelly for having those link properties be of
> the new type X86CPUSocket rather than X86CPU.
> 
> As final step in this series, the X86CPU allocation model is changed to be
> init'ed in-place, as part of an "atomic" socket object. A follow-up will be
> to attempt the same in-place allocation model for the APIC; one difficulty
> there is that several places do a NULL check for the APIC pointer as quick
> way of telling whether an APIC is present or not.
> 
> NOT IN THIS SERIES is converting cpu-add to the same socket/core/thread model
> and initializing them in-place. The current PoC implementation assumes that
> CPUs get added sequentially and that the preceding CPU can be used to obtain
> the core via unclean QOM parent accesses.
> IIUC that must be changed so that an arbitrary thread added via cpu-add
> creates the full socket and core(s). That would work best if indexed link<>
> properties could be used. That's an undecided design question of whether
> we want to have /machine/cpu-socket[n] or whether it makes sense to integrate
> NUMA modeling while at it and rather have /machine/node[n]/socket[m].

...looking into the future, we probably want to be able to model a cpu node
topology as newer machines are getting bigger and bigger and the locality
gets more important. So maybe we want to have a KVM-guest with 40 vCPUs and
pin 20 vCPUs on host book (node) 0 and 20 vcpu on host book 1.
- so node is of interest.


Regarding threads: z13 has SMT2. Right now with upstream code, we can fan 
out at the lpar level so that each thread becomes a host CPU and that is
used to back guest vCPUs. The alternative to pass a full host core to 
the guest and do the fan out in the guest is currently not supported.

Regarding cpu hotplug: This work is currently halted due to other things
but we plan to continue to work on that. 

I will have a look at your code. Thanks for working on this.

> 
> Note that this socket modeling is not only PC-specific in the softmmu sense
> but also in that not every X86CPU must be on a socket (e.g., Quark X1000).
> Therefore it was a concious decision to not label some things target-i386
> and to place code in pc.c rather than cpu.c.
> 
> Further note that it is ignored here that -smp enforces that AMD CPUs don't
> have hyperthreads, i.e. AMD X86CPUs will have only one thread[n] child<>.
> 
> Context:
> 
>qemu.git master
>"pc: Ensure non-zero CPU ref count after attaching to ICC bus"
> -> this series ad

Re: [Qemu-devel] [PATCH v5 1/6] target-arm: Store SPSR_EL1 state in banked_spsr[1] (SPSR_svc)

[Peter Maydell]

On 24 March 2015 at 14:32, Greg Bellows  wrote:
> On Mon, Mar 23, 2015 at 12:05 PM, Alex Bennée  wrote:
>> From: Peter Maydell 

>> @@ -523,7 +523,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
>>  aarch64_save_sp(env, arm_current_el(env));
>>  env->elr_el[new_el] = env->pc;
>>  } else {
>> -env->banked_spsr[0] = cpsr_read(env);
>> +env->banked_spsr[aarch64_banked_spsr_index(new_el)] = 
>> cpsr_read(env);
>
> Are the other banks (2-5) only used for KVM?  It seems we go out of
> our way to manage this larger SPSR array then not use all of the slots
> in QEMU itself.

They're used in AArch32 (where they are the SPSR for various
32 bit modes). In AArch64 you can access those registers via
MSR/MRS (we probably haven't implemented those yet because they
are only accessible at EL2 and above) so hypervisors can do
worldswitches. But for exception entry and return (which is
what this code is) we only use SPSR_EL0/SPSR_EL1/SPSR_EL2/SPSR_EL3
which is a subset of the AArch32 SPSRs.

-- PMM

Re: [Qemu-devel] RFC: memory API changes

[Paolo Bonzini]

On 24/03/2015 14:47, Peter Maydell wrote:
> On 23 March 2015 at 12:24, Peter Maydell  wrote:
>> (This is part of the work I'm doing for transaction attributes.)
> 
> OK, here's try 2, based on feedback on the first proposal:
> 
>  * address_space_rw &c remain with their current names, but
>take an extra MemTxAttrs argument and return MemTxResult
>rather than bool. (The latter conveniently doesn't require changes to
>callsites because conversion to bool gives the same true-on-error
>semantics as before.)
>[maybe readbuf/writebuf would be clearer than read/write,
>but it didn't seem sufficiently obvious a win to make the change]
>  * the ld/st_*phys functions are renamed as:
>  ldl_be_phys -> address_space_ldl_be &c
>and all take MemTxAttrs, *MemTxResult
>  * rather than MEMTXATTRS_UNSPECIFIED, use TXATTRS_NONE, so the
>extra arguments (TXATTRS_NONE, NULL) aren't too unwieldy
>  * prototypes in memory.h
>  * no default-to-no-attrs/etc versions of ld/st*_ phys
>(if in specific devices/buses it's the best thing we should
>have bus-specific dma accessors, as we do for pci)

I would keep them since they're really heavily used with cs->as as the
first argument.  But definitely move them to a different header than
cpu-common.h, and perhaps make them takes a CPUState instead of an
AddressSpace (which might help solving "where do we call the
unassigned-access hooks" in the future).

In any case, the removal or segregation of ld/st*_phys should be a
separate series for ease of review.

Apart from this, I'm on board.

Paolo

>  * mechanically convert all uses of cpu_physical_memory_* to
>address_space_*(&address_space_memory, ...)
> 
> [This leaves the "where do we call the unassigned-access
> hooks" problem for a different patchset.]
> 
> Is there anything in there people strongly dislike, or
> should I start writing coccinelle patches for this?
> 
> -- PMM
> 

Re: [Qemu-devel] [PATCH for-2.3 0/3] Contain drive_get() misuse

[Paolo Bonzini]

On 24/03/2015 13:48, Markus Armbruster wrote:
> Use of -drive if=floppy with onboard pc87312 (machine "prep") shouldn't
> be affected.  Likewise for connecting onboard m25p80-generic derivatives
> with if=mtd drives, or onboard ssi-sd with if=sd.

Exactly.

> Weird usage similar to the one you caught in time for sdhci-pci (--drive
> if=sd --device sdhci-pci) would break.  It's possible when the target
> has the device, and the machine type has a suitable bus.
> 
> * pc87312
> 
>   Depends on CONFIG_PC87312, set in {ppc,ppc64}-softmmu.mak.
> 
>   Requires an ISA bus.  I believe "prep" is the only machine providing
>   one.

You can add one with -device i82378.  Actually used in
tests/endianness-test.c, hence I guess supported.

>   If Andreas agrees, I can set cannot_instantiate_with_device_add_yet
>   for pc87312 now.

Could do that, could also decide that "-device i82378 -device pc87312"
is a valid way to add all the legacy crap to a PCI machine.  In which
case supporting "-drive if=floppy" is a weird feature but it's also hard
to call it a bug.

The difference with other devices is that you can only add it once.
It's a big difference.

> * ssi-sd
> 
>   I guess we want to document that --device ssi-sd will at some point
>   cease to auto-connect to the next available if=sd drive and require
>   the usual drive property instead.  Okay?
> 
> * m25p80-generic
>  
>   Document just like ssi-sd.

Ack for these two.  Boards can still hook -drive if={sd,mtd} to them,
but (hypothetical) users would have to switch to -drive if=none.

Paolo

> [...]
>> Acked-by: Paolo Bonzini 
> 
> Thanks!
> 

Re: [Qemu-devel] [v3][PATCH 2/2] libxl: introduce gfx_passthru_kind

[Ian Campbell]

On Mon, 2015-03-23 at 09:17 +0800, Tiejun Chen wrote:
> Although we already have 'gfx_passthru' in b_info, this doesn' suffice

^t

> after we want to handle IGD specifically. Now we define a new field of
> type, gfx_passthru_kind, to indicate we're trying to pass IGD. Actually
> this means we can benefit this to support other specific devices just
> by extending gfx_passthru_kind. And then we can cooperate with
> gfx_passthru to address IGD cases as follows:
> 
> gfx_passthru = 0=> sets build_info.u.gfx_passthru to false
> gfx_passthru = 1=> sets build_info.u.gfx_passthru to true and
>build_info.u.gfx_passthru_kind to DEFAULT
> gfx_passthru = "igd"=> sets build_info.u.gfx_passthru to false
   true

You had it right in the code.

>and build_info.u.gfx_passthru_kind to IGD
> 
> Here if gfx_passthru_kind = DEFAULT, we will call
> libxl__is_igd_vga_passthru() to check if we're hitting that table to need
> to pass that option to qemu. But if gfx_passthru_kind = "igd" we always
> force to pass that.
> 
> And "-gfx_passthru" is just introduced to work for qemu-xen-traditional
> so we should get this away from libxl__build_device_model_args_new() in
> the case of qemu upstream.
> 
> Signed-off-by: Tiejun Chen 
> ---
>  docs/man/xl.cfg.pod.5| 11 +++
>  tools/libxl/libxl.h  |  6 ++
>  tools/libxl/libxl_dm.c   | 36 +---
>  tools/libxl/libxl_internal.h |  4 
>  tools/libxl/libxl_types.idl  |  6 ++
>  tools/libxl/xl_cmdimpl.c | 14 --
>  6 files changed, 68 insertions(+), 9 deletions(-)
> 
> diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
> index 408653f..c29bcbf 100644
> --- a/docs/man/xl.cfg.pod.5
> +++ b/docs/man/xl.cfg.pod.5
> @@ -671,7 +671,7 @@ through to this VM. See L above.
>  devices passed through to this VM. See L
>  above.
>  
> -=item B
> +=item B

I think B is more accurate.

>  Enable graphics device PCI passthrough. This option makes an assigned
>  PCI graphics card become primary graphics card in the VM. The QEMU
> @@ -699,9 +699,12 @@ working graphics passthrough. See the 
> XenVGAPassthroughTestedAdapters
>  L wiki page
>  for currently supported graphics cards for gfx_passthru.
>  
> -gfx_passthru is currently only supported with the qemu-xen-traditional
> -device-model. Upstream qemu-xen device-model currently does not have
> -support for gfx_passthru.
> +gfx_passthru is currently supported both with the qemu-xen-traditional
> +device-model and upstream qemu-xen device-model. Note with the
> +qemu-xen-traditional device-model this option is just treated as BOOLEAN
> +actually, but with upstream qemu-xen device-model this option is extended
> +to pass a specific device name to force work. Currently just 'igd' is
> +defined to support Intel graphics device.

How about:

When given as a boolean the B option either
disables gfx passthru or enables autodetection.

When given as a string the B option describes the
type of passthru to enable.

Valid options are:

=over 4

=item B

Disables gfx_passthru.

=item B, B

Enables gfx_passthru and autodetects the type of device which is
being used.

=item "igd"

Enables gfx_passthru of the Intel Graphics Device.

=back

Note: When used with the qemu-xen-traditional device model only
IGD passthru is supported.

(do check my pod syntax, I'm mostly making it up!)

The note at the end makes me thing that perhaps something ought to check
this constraint in the qemu-xen-traditional case. It might be easiest to
do it in libxl__build_device_model_args_old using the
libxl__detect_gfx_passthru_kind helper you have added

e.g. where libxl__build_device_model_args_old has:
if (libxl_defbool_val(b_info->u.hvm.gfx_passthru)) {
flexarray_append(dm_args, "-gfx_passthru");
}
would become something like:
if (libxl_defbool_val(b_info->u.hvm.gfx_passthru)) {
if (libxl__detect_gfx_passthru_kind(gc, guest_config) !=
LIBXL_GFX_PASSTHRU_KIND_IGD) {
LOG("only IGD gfx_passthru is supported with 
qemu-xen-traditional");
return NULL;
}
flexarray_append(dm_args, "-gfx_passthru");
}

Or something like that. What do you think?

> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index c97c62d..26a01cb 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -3632,6 +3632,10 @@ static inline void libxl__update_config_vtpm(libxl__gc 
> *gc,
>   */
>

Re: [Qemu-devel] PCI passthrough of 40G ethernet interface (Openstack/KVM)

[Shannon Nelson]

On Tue, Mar 24, 2015 at 7:13 AM, jacob jacob  wrote:
> After update to latest firmware and using version 1.2.37 of i40e
> driver, things are looking better with PCI passthrough.
>
> ]# ethtool -i eth3
> driver: i40e
> version: 1.2.37
> firmware-version: f4.33.31377 a1.2 n4.42 e1930
> bus-info: :00:07.0
> supports-statistics: yes
> supports-test: yes
> supports-eeprom-access: yes
> supports-register-dump: yes
> supports-priv-flags: yes

I'm glad the updates helped as we expected.

>
> There are still issues running dpdk 1.8.0 from the VM using the pci
> passthrough devices and looks like it puts the devices in a bad state.
> i40e driver will not bind after this happens and a host reboot is
> required to recover.

Did you make sure to unbind the i40e device from pci-stub after you
were done with using it in the VM?

> I'll post further updates as i make progress.
> Thanks for all the help.
>

Good luck,
sln

Re: [Qemu-devel] RFC: memory API changes

[Peter Maydell]

On 24 March 2015 at 14:45, Paolo Bonzini  wrote:
> On 24/03/2015 14:47, Peter Maydell wrote:
>> On 23 March 2015 at 12:24, Peter Maydell  wrote:
>>  * no default-to-no-attrs/etc versions of ld/st*_ phys
>>(if in specific devices/buses it's the best thing we should
>>have bus-specific dma accessors, as we do for pci)
>
> I would keep them since they're really heavily used with cs->as as the
> first argument.  But definitely move them to a different header than
> cpu-common.h, and perhaps make them takes a CPUState instead of an
> AddressSpace (which might help solving "where do we call the
> unassigned-access hooks" in the future).

No, I think we want to be able to do simple "load a word
from an arbitrary AddressSpace" functions. And these should
definitely include the parameters for attributes and result
information, because otherwise it's way too easy to just
forget completely about those when you're writing a device.
What I don't think there's much call for is "load/store a
word, but completely ignore the possibility that might fail".

At some point we might want to have functions that take a
CPUState, yes. (We probably *don't* want those to do the
"call the unassigned access hook", because longjumping out of
functions is really hard to deal with, and we should I think
restrict that to code paths that come directly or near directly
from translated code. For instance if we take an exception trying
to read code during translation at the moment we do some completely
lunatic things that happen to work, and it would be much better
to just return a failure...) But that's separate cleanup IMHO.

In other words I'd rather try to restrict this to sorting out
the APIs that work generically on AddressSpaces. The amount
of change involved is already starting to feel very unwieldy.

> In any case, the removal or segregation of ld/st*_phys should be a
> separate series for ease of review.

Who wants to remove ld/st*_phys? Not me...

-- PMM

Re: [Qemu-devel] [PATCH for-2.3] sdhci: add "drive" property

[Markus Armbruster]

Peter Crosthwaite  writes:

> On Mon, Mar 23, 2015 at 7:05 PM, Markus Armbruster  wrote:
>> Paolo Bonzini  writes:
>>
>>> On 23/03/2015 10:10, Markus Armbruster wrote:
[...]
 I believe the proper solution for your problem is qdevifying the SD
 card.
>>>
>>> The question is whether there is a use for qdevifying the SD card.
>>
>> Okay, that's a fair question.
>>
>>> Each SD/MMC controller will have exactly zero or one SD cards, but the
>>> hw/sd/sd.c interface already treats "BlockBackend ejected" as "zero SD
>>> cards":
>>>
>>> if (!sd->blk || !blk_is_inserted(sd->blk) || !sd->enable) {
>>> return 0;
>>> }
>>>
>>> Unlike SCSI, the SD card code:
>>>
>>> 1) doesn't need multiplexing (a la scsi-hd/scsi-cd/scsi-generic)
>>>
>>> 2) doesn't have a bus to talk on (real-world SD cards are just connected
>>> with GPIO pins; hw/sd/sd.c abstracts the bitbanging protocol but still
>>> there is only one device to talk to)
>>>
>>> So in the end I think it's easier to treat hw/sd/sd.c as the common code
>>> for all hw/sd/* devices, like e.g. hw/display/vga.c.
>>
>> To pick a block device precedent: like floppy.
>>
>> I don't like that the floppy controller and its drives are fused.
>> However, the fusing has been *much* less grief than the usb-storage
>> hack: basically just a weird user interface to configure the drives,
>> namely --global instead of --device.
>>
>> If sd.c is common code rather than a device model in its own right,
>> perhaps SDState should be unboxed in SDHCIState, just like the FDrive
>> are unboxed in FDCtrl.  The "drive" property can then be connected
>> straight to SDState member blk.
>>
>
> sd.c for me is definitely a device in its own right, and SDHCI is
> quite separate. We already have multiple SD controllers connecting to
> SD cards. Ideally, SD should be bussified (or the modern equivalient
> using Links). I actually have code in my my tree that connects an
> SDHCI to two cards with muxing logic handled by another peripheral.
>
> There are a wider class of SD devices that can use the SD bus beyond
> SD cards (although we don't model any today). Another thing to
> consider is MMC support which is a subtle variant on SD card. We need
> the QOMification of sd.c to setup user-settable props or an
> inheritance hierarchy for the different flavors of SD/MMC cards. I
> don't think rolling into the controller is a good idea as then the
> controller needs to take ownership of any card configuration.
>
> I think the correct way forward is the qom/qdevification.
>
> Note that SD has a SPI mode, every SD card is in theory a valid SSI
> device. We could unify SD under SSI to achieve both QOMification and
> busification.
>
> I would then expect the block setup of sd.c to be very similar to
> hw/block/m25p80.c (SPI flash).

Do you think you'll be able to work on this?

It would be nice to have -device sdhci-pci working in 2.4.

Re: [Qemu-devel] [PATCH 2/6] throttle: Add throttle group infrastructure

[Stefan Hajnoczi]

On Tue, Mar 10, 2015 at 05:30:46PM +0200, Alberto Garcia wrote:
> +typedef struct ThrottleGroup {
> +char *name; /* This is constant during the lifetime of the group */

Is this also protected by throttle_groups_lock?

I guess throttle_groups_lock must be held in order to read this field -
otherwise there is a risk that the object is freed unless you have
already incremented the refcount.

> +
> +QemuMutex lock;

What does this lock protect?  I guess 'ts', 'head', and 'tokens' fields.
Please document it.

> +ThrottleState ts;
> +QLIST_HEAD(, BlockDriverState) head;
> +BlockDriverState *tokens[2];


pgpvJ1y8R3SaF.pgp
Description: PGP signature

Re: [Qemu-devel] [PATCH 1/6] throttle: Extract timers from ThrottleState into a separate structure

[Stefan Hajnoczi]

On Tue, Mar 10, 2015 at 05:30:45PM +0200, Alberto Garcia wrote:
> From: Benoît Canet 
> 
> Group throttling will share ThrottleState between multiple bs.
> As a consequence the ThrottleState will be accessed by multiple aio
> context.
> 
> Timers are tied to their aio context so they must go out of the
> ThrottleState structure.
> 
> This commit paves the way for each bs of a common ThrottleState to
> have its own timer.
> 
> Signed-off-by: Benoit Canet 
> Signed-off-by: Alberto Garcia 
> ---
>  block.c   | 35 
>  include/block/block_int.h |  1 +
>  include/qemu/throttle.h   | 38 ++
>  tests/test-throttle.c | 82 
> ++-
>  util/throttle.c   | 73 -
>  5 files changed, 135 insertions(+), 94 deletions(-)

Reviewed-by: Stefan Hajnoczi 


pgpmAINAXQ69v.pgp
Description: PGP signature

Re: [Qemu-devel] PCI passthrough of 40G ethernet interface (Openstack/KVM)

[jacob jacob]

On Tue, Mar 24, 2015 at 10:53 AM, Shannon Nelson
 wrote:
> On Tue, Mar 24, 2015 at 7:13 AM, jacob jacob  wrote:
>> After update to latest firmware and using version 1.2.37 of i40e
>> driver, things are looking better with PCI passthrough.
>>
>> ]# ethtool -i eth3
>> driver: i40e
>> version: 1.2.37
>> firmware-version: f4.33.31377 a1.2 n4.42 e1930
>> bus-info: :00:07.0
>> supports-statistics: yes
>> supports-test: yes
>> supports-eeprom-access: yes
>> supports-register-dump: yes
>> supports-priv-flags: yes
>
> I'm glad the updates helped as we expected.
>
>>
>> There are still issues running dpdk 1.8.0 from the VM using the pci
>> passthrough devices and looks like it puts the devices in a bad state.
>> i40e driver will not bind after this happens and a host reboot is
>> required to recover.
>
> Did you make sure to unbind the i40e device from pci-stub after you
> were done with using it in the VM?

The issue is running dpdk from within the vm itself. Is it possible
that the igb_uio driver needs additional updates/functionality to be
at parity with 1.2.37 version of i40e driver?

>
>> I'll post further updates as i make progress.
>> Thanks for all the help.
>>
>
> Good luck,
> sln

Re: [Qemu-devel] RFC: memory API changes

[Paolo Bonzini]

On 24/03/2015 15:53, Peter Maydell wrote:
>> > In any case, the removal or segregation of ld/st*_phys should be a
>> > separate series for ease of review.
> Who wants to remove ld/st*_phys? Not me...

Well, you want to rename them _and_ add new arguments.  Basically at the
end they don't exist anymore as we know them now. :)

Paolo

Re: [Qemu-devel] RFC: memory API changes

[Peter Maydell]

On 24 March 2015 at 15:08, Paolo Bonzini  wrote:
> On 24/03/2015 15:53, Peter Maydell wrote:
>>> > In any case, the removal or segregation of ld/st*_phys should be a
>>> > separate series for ease of review.
>> Who wants to remove ld/st*_phys? Not me...
>
> Well, you want to rename them _and_ add new arguments.  Basically at the
> end they don't exist anymore as we know them now. :)

I guess :-)  So what exactly would you like to see as a
separate series?

-- PMM

Re: [Qemu-devel] [PATCH 3/6] throttle: Add throttle group infrastructure tests

[Stefan Hajnoczi]

On Tue, Mar 10, 2015 at 05:30:47PM +0200, Alberto Garcia wrote:
> Signed-off-by: Alberto Garcia 
> ---
>  tests/test-throttle.c | 37 +++--
>  1 file changed, 35 insertions(+), 2 deletions(-)

Reviewed-by: Stefan Hajnoczi 


pgpP_PgdRsuJB.pgp
Description: PGP signature

Re: [Qemu-devel] [PATCH for-2.3 0/3] Contain drive_get() misuse

[Markus Armbruster]

Paolo Bonzini  writes:

> On 24/03/2015 13:48, Markus Armbruster wrote:
>> Use of -drive if=floppy with onboard pc87312 (machine "prep") shouldn't
>> be affected.  Likewise for connecting onboard m25p80-generic derivatives
>> with if=mtd drives, or onboard ssi-sd with if=sd.
>
> Exactly.
>
>> Weird usage similar to the one you caught in time for sdhci-pci (--drive
>> if=sd --device sdhci-pci) would break.  It's possible when the target
>> has the device, and the machine type has a suitable bus.
>> 
>> * pc87312
>> 
>>   Depends on CONFIG_PC87312, set in {ppc,ppc64}-softmmu.mak.
>> 
>>   Requires an ISA bus.  I believe "prep" is the only machine providing
>>   one.
>
> You can add one with -device i82378.  Actually used in
> tests/endianness-test.c, hence I guess supported.

Point taken.

>>   If Andreas agrees, I can set cannot_instantiate_with_device_add_yet
>>   for pc87312 now.
>
> Could do that, could also decide that "-device i82378 -device pc87312"
> is a valid way to add all the legacy crap to a PCI machine.  In which
> case supporting "-drive if=floppy" is a weird feature but it's also hard
> to call it a bug.
>
> The difference with other devices is that you can only add it once.
> It's a big difference.

$ qemu-system-ppc64 -M bamboo -S -device i82378 -device pc87312 -device pc87312
qemu-system-ppc64: -device pc87312: Property 'isa-parallel.chardev' can't take 
value 'parallel0', it's in use
Aborted (core dumped)

drive_get() & friends were designed for board code, and never meant for
realize().  They got used it in a few, but that were honest mistakes.
Attempting to promote mistakes into a design of sorts just to avoid the
consequences of reverting the mistakes is not a bright idea.  Especially
not when it complicates our conventions for device models.  People are
confused enough about them as it is, without alternate ways to do stuff
that only work in special circumstances.

Let's not go there.

I really can't see why we should tie ourselves into knots to avoid an
incompatible change here.  I seriously doubt anyone will notice if drop
the mistaken automatic backend pickup so that "-drive if=floppy -device
i82378 -device pc87312" no longer picks up the floppy.

>> * ssi-sd
>> 
>>   I guess we want to document that --device ssi-sd will at some point
>>   cease to auto-connect to the next available if=sd drive and require
>>   the usual drive property instead.  Okay?
>> 
>> * m25p80-generic
>>  
>>   Document just like ssi-sd.
>
> Ack for these two.  Boards can still hook -drive if={sd,mtd} to them,
> but (hypothetical) users would have to switch to -drive if=none.

Want me to add something to
http://wiki.qemu.org/ChangeLog/2.3#Future_incompatible_changes
?

[...]

Re: [Qemu-devel] [Xen-devel] [PATCH] SeaBios/vTPM: Enable Xen stubdom vTPM for HVM virtual machine

[Stefan Berger]

On 03/23/2015 10:20 PM, Xu, Quan wrote:



-Original Message-
From: Stefan Berger [mailto:stef...@linux.vnet.ibm.com]
Sent: Tuesday, March 24, 2015 4:01 AM
To: Xu, Quan; Ian Campbell
Cc: ke...@koconnor.net; qemu-devel@nongnu.org;
stefano.stabell...@eu.citrix.com; xen-de...@lists.xen.org
Subject: Re: [Qemu-devel] [Xen-devel] [PATCH] SeaBios/vTPM: Enable Xen
stubdom vTPM for HVM virtual machine

On 03/23/2015 08:03 AM, Xu, Quan wrote:

-Original Message-
From: Stefan Berger [mailto:stef...@linux.vnet.ibm.com]
Sent: Monday, March 23, 2015 6:57 PM
To: Xu, Quan; Ian Campbell
Cc: ke...@koconnor.net; xen-de...@lists.xen.org;
qemu-devel@nongnu.org; stefano.stabell...@eu.citrix.com
Subject: Re: [Xen-devel] [PATCH] SeaBios/vTPM: Enable Xen stubdom
vTPM for HVM virtual machine

On 03/22/2015 09:47 PM, Xu, Quan wrote:

-Original Message-
From: Stefan Berger [mailto:stef...@linux.vnet.ibm.com]
Sent: Friday, March 20, 2015 7:44 PM
To: Ian Campbell; Xu, Quan
Cc: ke...@koconnor.net; xen-de...@lists.xen.org;
qemu-devel@nongnu.org; stefano.stabell...@eu.citrix.com
Subject: Re: [Xen-devel] [PATCH] SeaBios/vTPM: Enable Xen stubdom
vTPM for HVM virtual machine

On 03/19/2015 08:56 AM, Ian Campbell wrote:

On Tue, 2015-03-10 at 08:16 -0400, Quan Xu wrote:

@@ -151,6 +152,8 @@ device_hardware_setup(void)
 esp_scsi_setup();
 megasas_setup();
 pvscsi_setup();
+if (runningOnXen())
+vtpm4hvm_setup();

Is there anything which is actually Xen specific about the driver
in tpm.[ch]? Would it be better to just probe for it, perhaps
gates by a Kconfig option which enables TPM support.

I also think the probing should be done. That code can also be
recycled from what I posted earlier. It's gated by a Kconfig
option, so it doesn't

fill up the 128k ROM.

Stefan


Agree, I will do it ASAP.

I reposted v9 of my series of patches. I will probably post v10 today.
Please try that one then since these patches should cover Xen, QEMU
(using a driver that only I can test at the moment), and to some
extent bare metal system.


  Stefan


Great!  Could you also archive v10 to your github?
then I can also test it and go through these source code.

I put it here now:

https://github.com/stefanberger/seabios-tpm

  Stefan

Thanks.
MS windows guest VM are maybe tricky issues. In my early-stage SeaBios patch,
I deal with TPM TCPA and SSDT in SeaBios, but MS windows guest VM is blue 
screens(Linux guest virtual machines are working).
It works when I deal with TPM TCPA and SSDT in hvmloader for Windows guest VM.


Can you be a bit more specific as to what gets it to work or which 
modifications you have to make in SeaBIOS to make it work?



   Stefan

Re: [Qemu-devel] [PATCH 2/6] throttle: Add throttle group infrastructure

[Alberto Garcia]

On Tue, Mar 24, 2015 at 03:03:07PM +, Stefan Hajnoczi wrote:

> > +typedef struct ThrottleGroup {
> > +char *name; /* This is constant during the lifetime of the group */
> 
> Is this also protected by throttle_groups_lock?
> 
> I guess throttle_groups_lock must be held in order to read this
> field - otherwise there is a risk that the object is freed unless
> you have already incremented the refcount.

The creation and destruction of ThrottleGroup objects are protected by
throttle_groups_lock. That includes handling the memory for that field
and its contents.

Once the ThrottleGroup is created the 'name' field doesn't need any
additional locking since it remains constant during the lifetime of
the group.

The only way to read it from the outside is throttle_group_get_name()
and that's safe (until you release the reference to the group, that
is).

> > +QemuMutex lock;
> 
> What does this lock protect?  I guess 'ts', 'head', and 'tokens'
> fields.  Please document it.

I thought it was obvious in the code but since you ask I guess it's
not :) I can add an additional comment.

Berto

Re: [Qemu-devel] [PATCH 0/2] CVE-2015-1779: fix denial of service in VNC websockets

[Gerd Hoffmann]

On Mo, 2015-03-23 at 22:58 +, Daniel P. Berrange wrote:
> The VNC websockets protocol decoder has two places where it did
> not correctly limit its resource usage when processing data from
> the client. This can be abused by a malicious client to cause QEMU
> to consume all system memory, unless it is otherwise limited by
> ulimits and/or cgroups. These problems can be triggered in the
> websockets layer before the VNC protocol actually starts, so no
> client authentication will have taken place at this point.

Hmm, with patch 1/2 applied novnc disconnects frequently.  Boot messages
on the text (framebuffer) console seems to work fine.  But after logging
in via gdm and trying to do stuff in gnome shell problems are starting.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 4/6] throttle: Add throttle group support

[Stefan Hajnoczi]

On Tue, Mar 10, 2015 at 05:30:48PM +0200, Alberto Garcia wrote:
> @@ -179,10 +179,11 @@ static void bdrv_throttle_write_timer_cb(void *opaque)
>  }
>  
>  /* should be called before bdrv_set_io_limits if a limit is set */
> -void bdrv_io_limits_enable(BlockDriverState *bs)
> +void bdrv_io_limits_enable(BlockDriverState *bs, const char *group)
>  {
>  assert(!bs->io_limits_enabled);
> -throttle_init(&bs->throttle_state);
> +
> +throttle_group_register_bs(bs, group ? group : bdrv_get_device_name(bs));

Please don't allow a NULL group argument.  blockdev_init() should pick
the group name instead of requiring bdrv_io_limits_enable() to generate
a unique name.

This way we avoid silently adding all BDS without a BlockBackend to a ""
throttling group.  I think that's a bug waiting to happen :).

> @@ -201,29 +219,7 @@ static void bdrv_io_limits_intercept(BlockDriverState 
> *bs,
>   unsigned int bytes,
>   bool is_write)
>  {
> -/* does this io must wait */
> -bool must_wait = throttle_schedule_timer(&bs->throttle_state,
> - &bs->throttle_timers,
> - is_write);
> -
> -/* if must wait or any request of this type throttled queue the IO */
> -if (must_wait ||
> -!qemu_co_queue_empty(&bs->throttled_reqs[is_write])) {
> -qemu_co_queue_wait(&bs->throttled_reqs[is_write]);
> -}
> -
> -/* the IO will be executed, do the accounting */
> -throttle_account(&bs->throttle_state, is_write, bytes);
> -
> -
> -/* if the next request must wait -> do nothing */
> -if (throttle_schedule_timer(&bs->throttle_state, &bs->throttle_timers,
> -is_write)) {
> -return;
> -}
> -
> -/* else queue next request for execution */
> -qemu_co_queue_next(&bs->throttled_reqs[is_write]);
> +throttle_group_io_limits_intercept(bs, bytes, is_write);
>  }

bdrv_io_limits_intercept() is no longer useful.  Can you replace
bdrv_io_limits_intercept() calls with
throttle_group_io_limits_intercept() to eliminate this indirection?

>  
>  size_t bdrv_opt_mem_align(BlockDriverState *bs)
> @@ -2050,15 +2046,16 @@ static void bdrv_move_feature_fields(BlockDriverState 
> *bs_dest,
>  bs_dest->enable_write_cache = bs_src->enable_write_cache;
>  
>  /* i/o throttled req */
> -memcpy(&bs_dest->throttle_state,
> -   &bs_src->throttle_state,
> -   sizeof(ThrottleState));
> +bs_dest->throttle_state = bs_src->throttle_state,
> +bs_dest->io_limits_enabled  = bs_src->io_limits_enabled;
> +bs_dest->throttled_reqs[0]  = bs_src->throttled_reqs[0];
> +bs_dest->throttled_reqs[1]  = bs_src->throttled_reqs[1];
> +memcpy(&bs_dest->round_robin,
> +   &bs_src->round_robin,
> +   sizeof(bs_dest->round_robin));

The bdrv_swap() code is tricky to think about...I think this is okay
because bs_dest and bs_src linked list pointers will be unchanged
(although they are now pointing to a different block driver instance).

Just highlighting this in case anyone else spots a problem with the
pointer swapping.

> @@ -145,6 +147,131 @@ static BlockDriverState 
> *throttle_group_next_bs(BlockDriverState *bs)
>  return next;
>  }
>  
> +/* Return the next BlockDriverState in the round-robin sequence with
> + * pending I/O requests.
> + *
> + * @bs:the current BlockDriverState
> + * @is_write:  the type of operation (read/write)
> + * @ret:   the next BlockDriverState with pending requests, or bs
> + * if there is none.

Assumes tg->lock is held.  It's helpful to document this.


pgpkS9ITuO2gR.pgp
Description: PGP signature

Re: [Qemu-devel] [PATCH for-2.3 0/3] Contain drive_get() misuse

[Paolo Bonzini]

> I really can't see why we should tie ourselves into knots to avoid an
> incompatible change here.  I seriously doubt anyone will notice if drop
> the mistaken automatic backend pickup so that "-drive if=floppy -device
> i82378 -device pc87312" no longer picks up the floppy.

Ok, it wasn't too hard to convince me.  Let's document the three of them
together.  I'll do it tomorrow if you haven't beaten me to it.

Paolo

> >> * ssi-sd
> >> 
> >>   I guess we want to document that --device ssi-sd will at some point
> >>   cease to auto-connect to the next available if=sd drive and require
> >>   the usual drive property instead.  Okay?
> >> 
> >> * m25p80-generic
> >>  
> >>   Document just like ssi-sd.
> >
> > Ack for these two.  Boards can still hook -drive if={sd,mtd} to them,
> > but (hypothetical) users would have to switch to -drive if=none.
> 
> Want me to add something to
> http://wiki.qemu.org/ChangeLog/2.3#Future_incompatible_changes
> ?
> 
> [...]
> 

Re: [Qemu-devel] RFC: memory API changes

[Paolo Bonzini]

> On 24 March 2015 at 15:08, Paolo Bonzini  wrote:
> > On 24/03/2015 15:53, Peter Maydell wrote:
> >>> > In any case, the removal or segregation of ld/st*_phys should be a
> >>> > separate series for ease of review.
> >> Who wants to remove ld/st*_phys? Not me...
> >
> > Well, you want to rename them _and_ add new arguments.  Basically at the
> > end they don't exist anymore as we know them now. :)
> 
> I guess :-)  So what exactly would you like to see as a
> separate series?

Adding the arguments / renaming the functions, for those callers
of ld/st*_phys that use cs->as as the first argument.

Paolo

Re: [Qemu-devel] [PATCH 4/6] throttle: Add throttle group support

[Stefan Hajnoczi]

On Tue, Mar 10, 2015 at 05:30:48PM +0200, Alberto Garcia wrote:
> +/* Return the next BlockDriverState in the round-robin sequence with
> + * pending I/O requests.
> + *
> + * @bs:the current BlockDriverState
> + * @is_write:  the type of operation (read/write)
> + * @ret:   the next BlockDriverState with pending requests, or bs
> + * if there is none.
> + */
> +static BlockDriverState *next_throttle_token(BlockDriverState *bs,
> + bool is_write)
> +{
> +ThrottleGroup *tg = container_of(bs->throttle_state, ThrottleGroup, ts);
> +BlockDriverState *token, *start;
> +
> +start = token = tg->tokens[is_write];
> +
> +/* get next bs round in round robin style */
> +token = throttle_group_next_bs(token);
> +while (token != start  &&
> +   qemu_co_queue_empty(&token->throttled_reqs[is_write])) {

It's not safe to access bs->throttled_reqs[].  There are many of other
places that access bs->throttled_reqs[] without holding tg->lock (e.g.
block.c).

throttled_reqs needs to be protected by tg->lock in order for this to
work.

> +/* Check if an I/O request needs to be throttled, wait and set a timer
> + * if necessary, and schedule the next request using a round robin
> + * algorithm.
> + *
> + * @bs:the current BlockDriverState
> + * @bytes: the number of bytes for this I/O
> + * @is_write:  the type of operation (read/write)
> + */
> +void throttle_group_io_limits_intercept(BlockDriverState *bs,
> +unsigned int bytes,
> +bool is_write)

This function must be called from coroutine context because it uses
qemu_co_queue_wait() and may yield.  Please mark the function with
coroutine_fn (see include/block/coroutine.h).  This serves as
documentation.

> +{
> +bool must_wait;
> +BlockDriverState *token;
> +
> +ThrottleGroup *tg = container_of(bs->throttle_state, ThrottleGroup, ts);
> +qemu_mutex_lock(&tg->lock);
> +
> +/* First we check if this I/O has to be throttled. */
> +token = next_throttle_token(bs, is_write);
> +must_wait = throttle_group_schedule_timer(token, is_write);
> +
> +/* Wait if there's a timer set or queued requests of this type */
> +if (must_wait || !qemu_co_queue_empty(&bs->throttled_reqs[is_write])) {
> +qemu_mutex_unlock(&tg->lock);
> +qemu_co_queue_wait(&bs->throttled_reqs[is_write]);

Here bs->throttled_reqs[] is used without holding tg->lock.  It can race
with token->throttled_reqs[] users.


pgplz7EDaDgog.pgp
Description: PGP signature

Re: [Qemu-devel] [PATCH v3 0/6] Block Throttle Group Support

[Stefan Hajnoczi]

On Tue, Mar 10, 2015 at 05:30:44PM +0200, Alberto Garcia wrote:
> Here's what's new:

Thanks for the cleanup!  The locking isn't safe yet but the series is in
pretty good shape overall.  Please see comments on individual patches
for details.

Stefan


pgpBl24v56at7.pgp
Description: PGP signature

Re: [Qemu-devel] RFC: memory API changes

[Peter Maydell]

On 24 March 2015 at 16:23, Paolo Bonzini  wrote:
>> On 24 March 2015 at 15:08, Paolo Bonzini  wrote:
>> > On 24/03/2015 15:53, Peter Maydell wrote:
>> >>> > In any case, the removal or segregation of ld/st*_phys should be a
>> >>> > separate series for ease of review.
>> >> Who wants to remove ld/st*_phys? Not me...
>> >
>> > Well, you want to rename them _and_ add new arguments.  Basically at the
>> > end they don't exist anymore as we know them now. :)
>>
>> I guess :-)  So what exactly would you like to see as a
>> separate series?
>
> Adding the arguments / renaming the functions

OK. (This will need the patch that actually at least defines
the MemTxAttr and MemTxResult types, obviously.)

>, for those callers
> of ld/st*_phys that use cs->as as the first argument.

...but I don't understand this caveat. I want to add arguments
and rename the functions for *all* callers of ld/st*_phys.
I don't want to specialcase the ones which happen to be
operating on cs->as.

-- PMM

Re: [Qemu-devel] [RFC 0/6] memory: make dirty_memory[] accesses atomic

[Stefan Hajnoczi]

On Mon, Mar 23, 2015 at 12:09:55PM +0100, Paolo Bonzini wrote:
> 
> 
> On 28/11/2014 13:44, Stefan Hajnoczi wrote:
> > This is an example of what I mean.
> > 
> > I'm not going to work on making TCG thread-safe in this series, and
> > there is no dangerous race condition in this code if we leave it as is.
> > 
> > But I'm not 100% sure of all cases, so I'll audit them again.
> 
> I looked at this again, and I agree that this series is not making
> anything worse.
> 
> We have to start somewhere, so I'm thinking of queuing this series for
> 2.4.  Making dirty bitmaps thread-safe with respect to memory hotplug
> can be done on top---especially since we already have a plan for that.

Hi Paolo,
Thanks for bringing this series up again.

I'm still happy with it.  Let me know if you'd like me to rebase.

Stefan


pgpALtSLKpWvA.pgp
Description: PGP signature

Re: [Qemu-devel] [PATCH RFC for-2.3 1/1] block: New command line option --no-format-probing

[Markus Armbruster]

Paolo Bonzini  writes:

> On 23/03/2015 21:19, Markus Armbruster wrote:
>> Paolo Bonzini  writes:
>> 
>>> On 23/03/2015 18:48, Eric Blake wrote:
> Why can't libvirt just add ,format=raw instead of leaving out the
> format key altogether?

 Libvirt DOES add format=raw.  This patch is an extra insurance
 policy to guarantee that libvirt does not have any code paths that
 omit the explicit format (as we have had a couple of CVEs in
 libvirt over the years where that was the case).
>>>
>>> And where's the extra insurance policy to guarantee that QEMU does not
>>> have any code paths that ignore the new command line option?
>> 
>> Right here (in the non-RFC patch, not this one):
>> 
>> @@ -751,6 +752,11 @@ static int find_image_format(BlockDriverState *bs, 
>> const char *filename,
>>  return ret;
>>  }
>>  
>> +if (bdrv_image_probing_disabled) {
>> +error_setg(errp, "Format not specified and image probing disabled");
>> +return -EINVAL;
>> +}
>> +
>>  ret = bdrv_pread(bs, 0, buf, sizeof(buf));
>>  if (ret < 0) {
>>  error_setg_errno(errp, -ret, "Could not read image for determining 
>> its "
>> 
>> The option sets bdrv_image_probing_disabled in a straightforward manner,
>> and bdrv_image_probing_disabled guards the probing code in an equally
>> straightforward manner.
>
> But what about migration from newer to older QEMU?  Libvirt even
> supports QEMU versions where the only way to specify disks is "-hda
> XYZ", so it is _impossible_ to honor the format=raw specifier.

If you migrate to a QEMU that doesn't understand the new option, libvirt
simply won't set it.  You lose the protection against libvirt bugs it
provides.  Guest won't notice.

If you somehow manage to find a QEMU old enough to make libvirt use
format-incapable interfaces, you'll be using insecure format probing on
the destination.  My patch makes no difference.  Good luck migrating to
such an old QEMU.

> Also, libvirt can start qemu-nbd and doesn't force format=raw in that
> case.  So the protection is far from complete.  This reinforces my
> opinion that the false sense of safety provided by this patch is worse
> than the "insurance" against future CVEs

The question whether the feature should be added to utilities is valid.
Me neglecting to add it in this patch can be a reason to reject this
patch, but hardly a reason for throwing out the idea wholesale.

>  (also, have there been any
> actual libvirt CVEs about this after 2010?  near misses don't count IMHO).

See Eric's testimony.

As to "near misses don't count": for me, what counts is actual users
telling me about their difficulties using QEMU securely.  Secure usage
shouldn't be hard.

>> How to get p0wned anyway:
>> 
>> 1. Use your raw image with format=raw.  Malicious guest writes qcow2
>> header.
>> 
>> 2. Use the image again without a format.  Probe returns qcow2, no
>> warning is printed.
>> 
>> Plausible attack?  Maybe not.  Worth stopping cold anyway?  I think so,
>> as long as it's easy.
>> 
>> My new option is a different kind of mitigation.  It's for users who
>> want more airtight protection, prefer writes to sector 0 just work,
>> funny or not, and are willing to always specify the format.  At least
>> one such user exists: libvirt.
>> 
>> Without this patch:
>> 
>> * Alternate use of raw images with and without format is still insecure;
>>   Kevin's mitigation can't protect you then.
>
> That's PEBKAC.

Secure usage shouldn't be hard.  When it is, then PEBKAC is blaming the
victim.

Today, secure usage requires you to always specify the format, on the
command line, in monitor commands, in every image referencing another
image (e.g. COW referencing backing file), and so forth.  There's images
all the way down.

Users tell us this is hard to get right and keep right, especially as
they race to keep up with our rapidly evolving block subsystem.

As long as they get it right, my proposed option doesn't make a
difference.  What it does is giving them a choice of failure modes for
when they fail:

1. The current failure mode, which maximizes backward compatibility and
   ease of use, except secure usage is inconveniently hard.  This is the
   default.

2. An alternate mode that fails early, hard and safe on insecure use.
   This is what libvirt wants to use.

> Paolo
>
>> * If you somehow miss specifying a format, and probing detects raw, you
>>   get a warning on stderr.  If your guest writes something funny to
>>   sector 0, the write fails.
>> 
>> With this patch:
>> 
>> * If you somehow miss specifying a format, open fails.  This is what
>>   libvirt wants.
>> 
>>>   There certainly hasn't been enough discussion
>>> for this to get into 2.3.
>> 
>> Isn't that what were doing now?  :)

I chatted with Kevin, and we agree it's too late for 2.3 now, given the
ongoing discussion, but we'd like to pursue this further in 2.4.

[Qemu-devel] [PATCH 3/3] spice: learn to hide cursor

[Marc-André Lureau]

---
 ui/spice-display.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/ui/spice-display.c b/ui/spice-display.c
index a85d6aa..7d025be 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -269,7 +269,8 @@ static void qemu_spice_create_update(SimpleSpiceDisplay 
*ssd)
 
 static SimpleSpiceCursor*
 qemu_spice_create_cursor_update(SimpleSpiceDisplay *ssd,
-QEMUCursor *c)
+QEMUCursor *c,
+int on)
 {
 size_t size = c ? c->width * c->height * 4 : 0;
 SimpleSpiceCursor *update;
@@ -297,6 +298,8 @@ qemu_spice_create_cursor_update(SimpleSpiceDisplay *ssd,
 cursor->data_size = size;
 cursor->chunk.data_size   = size;
 memcpy(cursor->chunk.data, c->data, size);
+} else if (!on) {
+ccmd->type = QXL_CURSOR_HIDE;
 } else {
 ccmd->type = QXL_CURSOR_MOVE;
 ccmd->u.position.x = ssd->ptr_x + ssd->hot_x;
@@ -721,7 +724,7 @@ static void display_mouse_set(DisplayChangeListener *dcl,
 if (ssd->ptr_move) {
 g_free(ssd->ptr_move);
 }
-ssd->ptr_move = qemu_spice_create_cursor_update(ssd, NULL);
+ssd->ptr_move = qemu_spice_create_cursor_update(ssd, NULL, on);
 qemu_mutex_unlock(&ssd->lock);
 }
 
@@ -740,7 +743,7 @@ static void display_mouse_define(DisplayChangeListener *dcl,
 if (ssd->ptr_define) {
 g_free(ssd->ptr_define);
 }
-ssd->ptr_define = qemu_spice_create_cursor_update(ssd, c);
+ssd->ptr_define = qemu_spice_create_cursor_update(ssd, c, 0);
 qemu_mutex_unlock(&ssd->lock);
 }
 
-- 
2.1.0

[Qemu-devel] [PATCH 1/3] spice: fix mouse cursor position

[Marc-André Lureau]

---
 ui/spice-display.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ui/spice-display.c b/ui/spice-display.c
index c888650..fb22b65 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -717,7 +717,7 @@ static void display_mouse_set(DisplayChangeListener *dcl,
 
 qemu_mutex_lock(&ssd->lock);
 ssd->ptr_x = x;
-ssd->ptr_y = x;
+ssd->ptr_y = y;
 if (ssd->ptr_move) {
 g_free(ssd->ptr_move);
 }
-- 
2.1.0

[Qemu-devel] [PATCH 0/3] A few Spice cursor fixes for non-QXL

[Marc-André Lureau]

Hi

Since 5643fc0, qemu learned to set Spice cursor when using
non-QXL card, this is a few improvements:

Marc-André Lureau (3):
  spice: fix mouse cursor position
  spice: set pointer position on hotspot
  spice: learn to hide cursor

 include/ui/spice-display.h |  3 ++-
 ui/spice-display.c | 21 +
 2 files changed, 15 insertions(+), 9 deletions(-)

-- 
2.1.0

[Qemu-devel] [PATCH 2/3] spice: set pointer position on hotspot

[Marc-André Lureau]

The Spice protocol uses cursor position on hotspot: the client is
applying hotspot offset when drawing the cursor.
---
 include/ui/spice-display.h |  3 ++-
 ui/spice-display.c | 10 ++
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/include/ui/spice-display.h b/include/ui/spice-display.h
index 1672889..2c3959f 100644
--- a/include/ui/spice-display.h
+++ b/include/ui/spice-display.h
@@ -97,7 +97,8 @@ struct SimpleSpiceDisplay {
 /* cursor (without qxl): displaychangelistener -> spice server */
 SimpleSpiceCursor *ptr_define;
 SimpleSpiceCursor *ptr_move;
-uint16_t ptr_x, ptr_y;
+int16_t ptr_x, ptr_y;
+int16_t hot_x, hot_y;
 
 /* cursor (with qxl): qxl local renderer -> displaychangelistener */
 QEMUCursor *cursor;
diff --git a/ui/spice-display.c b/ui/spice-display.c
index fb22b65..a85d6aa 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -284,8 +284,8 @@ qemu_spice_create_cursor_update(SimpleSpiceDisplay *ssd,
 
 if (c) {
 ccmd->type = QXL_CURSOR_SET;
-ccmd->u.set.position.x = ssd->ptr_x;
-ccmd->u.set.position.y = ssd->ptr_y;
+ccmd->u.set.position.x = ssd->ptr_x + ssd->hot_x;
+ccmd->u.set.position.y = ssd->ptr_y + ssd->hot_y;
 ccmd->u.set.visible= true;
 ccmd->u.set.shape  = (uintptr_t)cursor;
 cursor->header.unique = ssd->unique++;
@@ -299,8 +299,8 @@ qemu_spice_create_cursor_update(SimpleSpiceDisplay *ssd,
 memcpy(cursor->chunk.data, c->data, size);
 } else {
 ccmd->type = QXL_CURSOR_MOVE;
-ccmd->u.position.x = ssd->ptr_x;
-ccmd->u.position.y = ssd->ptr_y;
+ccmd->u.position.x = ssd->ptr_x + ssd->hot_x;
+ccmd->u.position.y = ssd->ptr_y + ssd->hot_y;
 }
 ccmd->release_info.id = (uintptr_t)(&update->ext);
 
@@ -731,6 +731,8 @@ static void display_mouse_define(DisplayChangeListener *dcl,
 SimpleSpiceDisplay *ssd = container_of(dcl, SimpleSpiceDisplay, dcl);
 
 qemu_mutex_lock(&ssd->lock);
+ssd->hot_x = c->hot_x;
+ssd->hot_y = c->hot_y;
 if (ssd->ptr_move) {
 g_free(ssd->ptr_move);
 ssd->ptr_move = NULL;
-- 
2.1.0

Re: [Qemu-devel] [PATCH v2 0/2] ahci: test varying sector offsets

[John Snow]

Ping: I'll pull both this series and the 'ahci: rerror/werror=stop 
resume tests' series into my ide-next branch if just these two patches 
get a re-review.


They were excised from a pullreq due to glib compatibility issues, so 
the following series has no changes.


--js

On 03/13/2015 03:22 PM, John Snow wrote:

This is a re-send of patches 7 & 8 from an earlier series,
"[PATCH v2 0/8] ahci: add more IO tests" which ultimately got bounced
back because I used some glib functions that were too new.

v2:
- Patchew caught a pathing problem with the qemu-img binary;
   the relative path produced by the Makefile does not prepend
   "./", so I was relying on the /distro's/ qemu-img by accident.
   Fix that by using realpath().

v1:
- Removed "./" from the execution CLI. Now you can set an absolute or
   relative path for QTEST_QEMU_IMG and it will work either way. The default
   as generated by the Makefile will be a relative path.

- Removed the g_spawn_check_exit_status glib call from mkimg(). See the
   in-line comments in patch 1/2 for correctness justification.

John Snow (2):
   qtest/ahci: add qcow2 support to ahci-test
   qtest/ahci: test different disk sectors

  tests/Makefile|  1 +
  tests/ahci-test.c | 84 +--
  tests/libqos/ahci.c   | 10 +++---
  tests/libqos/ahci.h   |  4 +--
  tests/libqos/libqos.c | 44 +++
  tests/libqos/libqos.h |  2 ++
  6 files changed, 116 insertions(+), 29 deletions(-)

Re: [Qemu-devel] [Xen-devel] Question about scsi emulation

[Stefano Stabellini]

On Tue, 24 Mar 2015, Juergen Gross wrote:
> On 03/24/2015 01:20 AM, Yaoli Zheng wrote:
> > Thank you for the advice!
> > It will be grateful if someone can provide any guide how to config  MegaSAS
> > HBA or how to use xen pvscsi in XEN.
> > There seems no option in XEN for these two driver emulation and no document
> > found online.
> 
> The pvscsi driver is rather new in the Linux kernel. Support for pvscsi
> in Xen tools (xl) is just being worked on:
> 
> http://lists.xen.org/archives/html/xen-devel/2015-03/msg00030.html
> 

Regarding MegaSAS, it should be just a matter of passing the right
command line arguments to QEMU so that it will emulate it, same as for
QEMU without Xen. You can use device_model_args in the VM config file to
pass custom command line arguments from xl to QEMU.


> > 
> > Thanks!
> > 
> > -Original Message-
> > From: Stefano Stabellini [mailto:stefano.stabell...@eu.citrix.com]
> > Sent: Monday, March 23, 2015 11:18 AM
> > To: Stefan Hajnoczi
> > Cc: Yaoli Zheng; qemu-devel@nongnu.org; xen-de...@lists.xen.org
> > Subject: Re: [Xen-devel] [Qemu-devel] Question about scsi emulation
> > 
> > On Mon, 23 Mar 2015, Stefan Hajnoczi wrote:
> > > On Wed, Mar 18, 2015 at 02:16:47PM -0700, Yaoli Zheng wrote:
> > > > We have problem using qemu emulated scsi driver(the old lsi). Wonder if
> > > > any of other device model we can try for emulating scsi, and how we can
> > > > get and config it in Xen? Having been told virtio-scsi is alternative
> > > > one, but have no idea how to get it work in Xen.
> > > 
> > > The MegaSAS HBA might be another option.  Not sure whether MegaSAS or
> > > virtio-scsi are supported under Xen though.
> > 
> > Making MegaSAS HBA work on Xen should be easy.
> > 
> > Rather than virtio-scsi I would consider the new Xen pvscsi frontend/backend
> > drivers, now upstream in Linux (drivers/scsi/xen-scsifront.c and
> > drivers/xen/xen-scsiback.c). Usually Xen PV drivers perform much better than
> > virtio devices on Xen.
> > 
> > That said, probably most things would be better than the old lsi.
> > 
> > ___
> > Xen-devel mailing list
> > xen-de...@lists.xen.org
> > http://lists.xen.org/xen-devel
> > 
> 

Re: [Qemu-devel] [PATCH 4/6] throttle: Add throttle group support

[Alberto Garcia]

On Tue, Mar 24, 2015 at 04:31:45PM +, Stefan Hajnoczi wrote:

> > +/* get next bs round in round robin style */
> > +token = throttle_group_next_bs(token);
> > +while (token != start  &&
> > +   qemu_co_queue_empty(&token->throttled_reqs[is_write])) {
> 
> It's not safe to access bs->throttled_reqs[].  There are many of
> other places that access bs->throttled_reqs[] without holding
> tg->lock (e.g. block.c).
>
> throttled_reqs needs to be protected by tg->lock in order for this
> to work.

Good catch, but I don't think that's the solution. throttled_reqs
cannot be protected by that lock because it must release it before
calling qemu_co_queue_wait(&bs->throttled_reqs[is_write]). Otherwise
it will cause a deadlock.

My assumption was that throttled_reqs would be protected by the BDS's
AioContext lock, but I overlooked the fact that this part of the
algorithm needs to access other BDSs' queues so we indeed have a
problem.

I will give it a thought to see what I can come up with. Since we only
need to check whether other BDSs have pending requests maybe I can
implement this with a flag.

Thanks!

Berto

Re: [Qemu-devel] RFC: memory API changes

[Paolo Bonzini]

On 24/03/2015 17:35, Peter Maydell wrote:
> On 24 March 2015 at 16:23, Paolo Bonzini  wrote:
>>> On 24 March 2015 at 15:08, Paolo Bonzini  wrote:
 On 24/03/2015 15:53, Peter Maydell wrote:
>>> In any case, the removal or segregation of ld/st*_phys should be a
>>> separate series for ease of review.
> Who wants to remove ld/st*_phys? Not me...

 Well, you want to rename them _and_ add new arguments.  Basically at the
 end they don't exist anymore as we know them now. :)
>>>
>>> I guess :-)  So what exactly would you like to see as a
>>> separate series?
>>
>> Adding the arguments / renaming the functions
> 
> OK. (This will need the patch that actually at least defines
> the MemTxAttr and MemTxResult types, obviously.)
> 
>> , for those callers
>> of ld/st*_phys that use cs->as as the first argument.
> 
> ...but I don't understand this caveat. I want to add arguments
> and rename the functions for *all* callers of ld/st*_phys.
> I don't want to specialcase the ones which happen to be
> operating on cs->as.

The ones that operate on cs->as could become (for some CPUs at least)
special-cased accessors like the bus ones; for example building the
MemTxAttrs according to internal CPU state.

ld/st*_phys actually started as CPU-specific accessors, and most uses
are still of that kind, so it makes sense to me that we special-case
them.  Maybe it limits churn, maybe it doesn't.  But if it doesn't, it's
not like anything is lost.

Paolo

Re: [Qemu-devel] RFC: memory API changes

[Peter Maydell]

On 24 March 2015 at 17:51, Paolo Bonzini  wrote:
> On 24/03/2015 17:35, Peter Maydell wrote:
>> On 24 March 2015 at 16:23, Paolo Bonzini  wrote:
 On 24 March 2015 at 15:08, Paolo Bonzini  wrote:
>>> , for those callers
>>> of ld/st*_phys that use cs->as as the first argument.
>>
>> ...but I don't understand this caveat. I want to add arguments
>> and rename the functions for *all* callers of ld/st*_phys.
>> I don't want to specialcase the ones which happen to be
>> operating on cs->as.
>
> The ones that operate on cs->as could become (for some CPUs at least)
> special-cased accessors like the bus ones; for example building the
> MemTxAttrs according to internal CPU state.

Sure, individual targets could do something like this if they
wanted (compare the arm_ldl_code functions), once these renames
have gone in.

> ld/st*_phys actually started as CPU-specific accessors, and most uses
> are still of that kind, so it makes sense to me that we special-case
> them.  Maybe it limits churn, maybe it doesn't.  But if it doesn't, it's
> not like anything is lost.

I think this is where we disagree. I see ld/st*_phys as being
really generic -- they take an AddressSpace, after all, and
part of the same family with address_space_read &c. If you
don't leave them as generic, then you end up having to use
the really awkward _read/_write for simple accesses and
then manage the byteswapping yourself. That's why I want
to rename them into address_space_* : to indicate that they
are all part of the same family, and you can use
address_space_read if you want to read an arbitrary byte
buffer, or address_space_ldl_be if you want to read a
big endian 32 bit word, and so on.

(The only reason they started out CPU specific is because
we didn't have any concept at all of having more than
one address space, so there wasn't any need to say which
one you meant when you were doing a load.)

To me it makes much more sense that if a DMA controller
like pl080 wants to do an LE word read from the AS which
its bus master is connected to, that it can just do
 word = ldl_le_phys(my_as, addr, ...);

I'd expect pretty much any bus master to want to do this
kind of thing, in fact. It just happens that most of the
bus masters we have in QEMU right now are CPUs...

-- PMM

[Qemu-devel] [PATCH v4 3/4] configure: silence glib unknown attribute __alloc_size__

[John Snow]

The glib headers use GCC attributes.  Unfortunately the __GNUC__ and
__GNUC_MINOR__ version macros are also defined by clang, but clang
doesn't support the same attributes as GCC.

clang 3.5.0 does not support the __alloc_size__ attribute:

  
https://github.com/llvm-mirror/clang/commit/c047507a9a79e89fc8339e074fa72822a7e7ea73

The following warning is produced:

  gstrfuncs.h:257:44: warning: unknown attribute '__alloc_size__' ignored 
[-Wunknown-attributes]
G_GNUC_MALLOC G_GNUC_ALLOC_SIZE(2);
  gmacros.h:67:45: note: expanded from macro 'G_GNUC_ALLOC_SIZE'
#define G_GNUC_ALLOC_SIZE(x) __attribute__((__alloc_size__(x)))

This patch checks whether glib headers cause warnings and disables
-Wunknown-attributes if it is able to.

Signed-off-by: Stefan Hajnoczi 
Signed-off-by: John Snow 
---
 configure | 12 
 1 file changed, 12 insertions(+)

diff --git a/configure b/configure
index 6f4bf4f..87a7f40 100755
--- a/configure
+++ b/configure
@@ -2789,6 +2789,18 @@ if ! $pkg_config --atleast-version=2.38 glib-2.0; then
 glib_subprocess=no
 fi
 
+# Silence clang 3.5.0 warnings about glib attribute __alloc_size__ usage
+cat > $TMPC << EOF
+#include 
+int main(void) { return 0; }
+EOF
+if ! compile_prog "$glib_cflags -Werror" "$glib_libs" ; then
+if cc_has_warning_flag "-Wno-unknown-attributes"; then
+glib_cflags="-Wno-unknown-attributes $glib_cflags"
+CFLAGS="-Wno-unknown-attributes $CFLAGS"
+fi
+fi
+
 ##
 # SHA command probe for modules
 if test "$modules" = yes; then
-- 
2.1.0

[Qemu-devel] [PATCH v4 4/4] configure: Add workaround for ccache and clang

[John Snow]

Test if ccache is interfering with semantic analysis of macros,
disable its habit of trying to compile already pre-processed
versions of code if so. ccache attempts to save time by compiling
pre-processed versions of code, but this disturbs clang's static
analysis enough to produce false positives.

ccache allows us to disable this feature, opting instead to
compile the original version instead of its preprocessed version.
This makes ccache much slower for cache misses, but at least it
becomes usable with QEMU/clang.

This workaround only activates for users using ccache AND clang,
and only if their configuration is observed to be producing warnings.
You may need to clear your ccache for builds started without -Werror,
as those may continue to produce warnings from the cache.

Thanks to Peter Eisentraut for his writeup on the issue:
http://peter.eisentraut.org/blog/2014/12/01/ccache-and-clang-part-3/

Signed-off-by: John Snow 
---
 configure | 34 +-
 1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index 87a7f40..36274cc 100755
--- a/configure
+++ b/configure
@@ -103,7 +103,8 @@ update_cxxflags() {
 }
 
 compile_object() {
-  do_cc $QEMU_CFLAGS -c -o $TMPO $TMPC
+  local_cflags="$1"
+  do_cc $QEMU_CFLAGS $local_cflags -c -o $TMPO $TMPC
 }
 
 compile_prog() {
@@ -4169,6 +4170,33 @@ if compile_prog "" "" ; then
 getauxval=yes
 fi
 
+
+# check if ccache is interfering with
+# semantic analysis of macros
+
+ccache_cpp2=no
+cat > $TMPC << EOF
+static const int Z = 1;
+#define fn() ({ Z; })
+#define TAUT(X) ((X) == Z)
+#define PAREN(X, Y) (X == Y)
+#define ID(X) (X)
+int main(int argc, char *argv[])
+{
+int x = 0, y = 0;
+x = ID(x);
+x = fn();
+fn();
+if (PAREN(x, y)) return 0;
+if (TAUT(Z)) return 0;
+return 0;
+}
+EOF
+
+if ! compile_object "-Werror"; then
+ccache_cpp2=yes
+fi
+
 ##
 # End of CC checks
 # After here, no more $cc or $ld runs
@@ -5461,6 +5489,10 @@ if test "$numa" = "yes"; then
   echo "CONFIG_NUMA=y" >> $config_host_mak
 fi
 
+if test "$ccache_cpp2" = "yes"; then
+  echo "export CCACHE_CPP2=y" >> $config_host_mak
+fi
+
 # build tree in object directory in case the source is not in the current 
directory
 DIRS="tests tests/tcg tests/tcg/cris tests/tcg/lm32 tests/libqos 
tests/qapi-schema tests/tcg/xtensa tests/qemu-iotests"
 DIRS="$DIRS fsdev"
-- 
2.1.0

[Qemu-devel] [PATCH v4 2/4] configure: factor out supported flag check

[John Snow]

Factor out the function that checks if a compiler
flag is supported or not.

Signed-off-by: John Snow 
---
 configure | 33 +++--
 1 file changed, 19 insertions(+), 14 deletions(-)

diff --git a/configure b/configure
index 7a8637e..6f4bf4f 100755
--- a/configure
+++ b/configure
@@ -435,6 +435,12 @@ EOF
   compile_object
 }
 
+write_c_skeleton() {
+cat > $TMPC < $TMPC << EOF
-int main(void) { return 0; }
-EOF
+  write_c_skeleton;
   if compile_prog "" "-liberty" ; then
 LIBS="-liberty $LIBS"
   fi
@@ -1438,10 +1442,7 @@ if test -z "$werror" ; then
 fi
 
 # check that the C compiler works.
-cat > $TMPC < $TMPC << EOF
-int main(void) { return 0; }
-EOF
-for flag in $gcc_flags; do
+
+cc_has_warning_flag() {
+write_c_skeleton;
+
 # Use the positive sense of the flag when testing for -Wno-wombat
 # support (gcc will happily accept the -Wno- form of unknown
 # warning options).
-optflag="$(echo $flag | sed -e 's/^-Wno-/-W/')"
-if compile_prog "-Werror $optflag" "" ; then
-   QEMU_CFLAGS="$QEMU_CFLAGS $flag"
+optflag="$(echo $1 | sed -e 's/^-Wno-/-W/')"
+compile_prog "-Werror $optflag" ""
+}
+
+for flag in $gcc_flags; do
+if cc_has_warning_flag $flag --keep-tmpc; then
+QEMU_CFLAGS="$QEMU_CFLAGS $flag"
 fi
 done
 
-- 
2.1.0

[Qemu-devel] [PATCH v4 0/4] configure: clang 3.5.0 build fixes

[John Snow]

QEMU does not compile cleanly under clang 3.5.0.  These patches eliminate the
avalanche of warnings and make the build usable.

v4:
- Enable ccache workaround for configurations without -Werror,
  to suppress warnings as well.
- Removed optimization from cc_has_warning_flag()
- Renamed ccache variable to ccache_cpp2

v3:
- Drop tricore patch, it's already being worked on elsewhere.
- Factor out flag check test.

v2:
 - Stole the series from Stefan
 - No changes to the -nopie patch, which I think was fine.
 - Fixed the -Wno-unknown-attributes patch.
 - Added a tricore fix for -Wabsolute-value
 - Added a workaround to help suppress ccache warnings

The result is that you *should* be able to use clang 3.5.0 *with* ccache and
-Werror and produce all targets.

John Snow (3):
  configure: factor out supported flag check
  configure: silence glib unknown attribute __alloc_size__
  configure: Add workaround for ccache and clang

Stefan Hajnoczi (1):
  configure: handle clang -nopie argument warning

 configure | 81 ++-
 1 file changed, 65 insertions(+), 16 deletions(-)

-- 
2.1.0

[Qemu-devel] [PATCH v4 1/4] configure: handle clang -nopie argument warning

[John Snow]

From: Stefan Hajnoczi 

gcc 4.9.2 treats -nopie as an error:

  cc: error: unrecognized command line option ‘-nopie’

clang 3.5.0 treats -nopie as a warning:

  clang: warning: argument unused during compilation: '-nopie'

The causes ./configure to fail with clang:

  ERROR: configure test passed without -Werror but failed with -Werror.

Make the -nopie test use -Werror so that compile_prog works for both gcc
and clang.

Signed-off-by: Stefan Hajnoczi 
Signed-off-by: John Snow 
---
 configure | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure b/configure
index 589798e..7a8637e 100755
--- a/configure
+++ b/configure
@@ -1589,7 +1589,7 @@ EOF
 fi
   fi
 
-  if compile_prog "-fno-pie" "-nopie"; then
+  if compile_prog "-Werror -fno-pie" "-nopie"; then
 CFLAGS_NOPIE="-fno-pie"
 LDFLAGS_NOPIE="-nopie"
   fi
-- 
2.1.0

[Qemu-devel] [PATCH for-2.3 0/1] gtk: fix NULL dereferences

[Hervé Poussineau]

Hi,

Attached patch fixes some problems I encountered while testing MIPS Magnum 
emulation.
(qemu-system-mips64el:1399): Gdk-CRITICAL **: IA__gdk_drawable_get_size: 
assertion `GDK_IS_DRAWABLE (drawable)' failed
(qemu-system-mips64el:1714): Gdk-CRITICAL **: IA__gdk_window_set_cursor: 
assertion `GDK_IS_WINDOW (window)' failed

Hervé Poussineau (1):
  gtk: do not call gtk_widget_get_window if drawing area is not
initialized

 ui/gtk.c |   12 
 1 file changed, 12 insertions(+)

-- 
1.7.10.4

[Qemu-devel] [PATCH for-2.3 1/1] gtk: do not call gtk_widget_get_window if drawing area is not initialized

[Hervé Poussineau]

This prevents gtk_widget_get_window to return a NULL pointer.

Signed-off-by: Hervé Poussineau 
---
 ui/gtk.c |   12 
 1 file changed, 12 insertions(+)

diff --git a/ui/gtk.c b/ui/gtk.c
index 29bdc19..d6e01c2 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -294,6 +294,10 @@ static void gd_update_cursor(VirtualConsole *vc)
 return;
 }
 
+if (!gtk_widget_get_realized(vc->gfx.drawing_area)) {
+return;
+}
+
 window = gtk_widget_get_window(GTK_WIDGET(vc->gfx.drawing_area));
 if (s->full_screen || qemu_input_is_absolute() || s->ptr_owner == vc) {
 gdk_window_set_cursor(window, s->null_cursor);
@@ -458,6 +462,10 @@ static void gd_update(DisplayChangeListener *dcl,
 
 trace_gd_update(vc->label, x, y, w, h);
 
+if (!gtk_widget_get_realized(vc->gfx.drawing_area)) {
+return;
+}
+
 if (vc->gfx.convert) {
 pixman_image_composite(PIXMAN_OP_SRC, vc->gfx.ds->image,
NULL, vc->gfx.convert,
@@ -540,6 +548,10 @@ static void gd_cursor_define(DisplayChangeListener *dcl,
 GdkPixbuf *pixbuf;
 GdkCursor *cursor;
 
+if (!gtk_widget_get_realized(vc->gfx.drawing_area)) {
+return;
+}
+
 pixbuf = gdk_pixbuf_new_from_data((guchar *)(c->data),
   GDK_COLORSPACE_RGB, true, 8,
   c->width, c->height, c->width * 4,
-- 
1.7.10.4

[Qemu-devel] [PATCH v2 0/5] Implement RH bit handling and lowpri arbitration

[James Sullivan]

(Combination of
<1427149572-11378-1-git-send-email-sullivan.jame...@gmail.com> and
<1427152883-7049-1-git-send-email-sullivan.jame...@gmail.com>)

Changes in v2:
* Merged in low priority IRQ delivery implementation to RH bit
handling implementation, since both rely on the same helper
functions for priority arbitration.
* Corrected use of MSI data register => addr register when setting
msi_redir_hint in apic_send_msi().

This set of patches adds the following features to QEMU:
* Low priority delivery arbitration. Currently the first present CPU
is always selected when lowpri delivery mode is used, and no
arbitration is performed. Implemented arbitration in
apic_bus_deliver() by adding the following functions:
1) apic_get_arb_pri(APICCommonState *s)
2) apic_compare_prio(APICCommonState *s1, APICCommonState *s2);
3) apic_lowest_prio(const uint32_t *deliver_bitmask)
* RH Bit handling for MSI messages. See below.

Currently, there is no handling of the MSI RH bit. This patch implements 
the following logic:

* DM=0, RH=*  : Physical destination mode. Interrupt is delivered to
the LAPIC with the matching APIC ID. (Subject to
the usual restrictions, i.e. no broadcast dest)
* DM=1, RH=0  : Logical destination mode without redirection. Interrupt
is delivered to all LAPICs in the logical group 
specified by the IRQ's destination map and delivery
mode.
* DM=1, RH=1  : Logical destination mode with redirection. Interrupt
is delivered only to the lowest priority LAPIC in
the 
logical group specified by the dest map and the
delivery mode. Delivery semantics are otherwise
specified by the delivery_mode of the IRQ, which
is unchanged.

These changes reflect those made in the KVM in
http://www.spinics.net/lists/kvm/msg114915.html ("kvm: x86: Implement
handling of RH=1 for MSI delivery in KVM"), which are under review but
are largely settled in terms of functionality.


James Sullivan (5):
  apic: Implement LAPIC low priority arbitration functions
  apic: Implement low priority arbitration for IRQ delivery
  apic: Added helper function apic_match_dest,
apic_match_[physical,logical]_dest
  apic: Set and pass in RH bit for MSI interrupts
  apic: Implement handling of RH=1 for MSI interrupt delivery

 hw/intc/apic.c | 137 -
 hw/intc/ioapic.c   |   2 +-
 include/hw/i386/apic.h |   3 +-
 trace-events   |   2 +-
 4 files changed, 105 insertions(+), 39 deletions(-)

-- 
2.3.3

[Qemu-devel] [PATCH v2 2/5] apic: Implement low priority arbitration for IRQ delivery

[James Sullivan]

Currently, there is no arbitration among processors for low priority IRQ
delivery. Added support for low priority arbitration to
apic_bus_deliver(), using the functions introduced in
[74c1222c5b579970fafdd6a8e919fbb2c88219c3] ("apic: Implement LAPIC low
priority arbitration functions").

Signed-off-by: James Sullivan 
---
 hw/intc/apic.c | 20 
 1 file changed, 4 insertions(+), 16 deletions(-)

diff --git a/hw/intc/apic.c b/hw/intc/apic.c
index b372513..47d2fb1 100644
--- a/hw/intc/apic.c
+++ b/hw/intc/apic.c
@@ -249,22 +249,10 @@ static void apic_bus_deliver(const uint32_t 
*deliver_bitmask,
 
 switch (delivery_mode) {
 case APIC_DM_LOWPRI:
-/* XXX: search for focus processor, arbitration */
-{
-int i, d;
-d = -1;
-for(i = 0; i < MAX_APIC_WORDS; i++) {
-if (deliver_bitmask[i]) {
-d = i * 32 + apic_ffs_bit(deliver_bitmask[i]);
-break;
-}
-}
-if (d >= 0) {
-apic_iter = local_apics[d];
-if (apic_iter) {
-apic_set_irq(apic_iter, vector_num, trigger_mode);
-}
-}
+/* XXX: search for focus processor */
+apic_iter = apic_lowest_prio(deliver_bitmask);
+if (apic_iter) {
+apic_set_irq(apic_iter , vector_num, trigger_mode);
 }
 return;
 
-- 
2.3.3