Re: [Qemu-devel] [PATCH] qemu-kvm: Add CPUID support for VIA CPU

[Jan Kiszka]

On 2011-05-06 03:06, brill...@viatech.com.cn wrote:
> Hi, Jan
> Thank you very much for your advice. That's helpful for me.
> 
>> Hi,
>>
>> the subject's tag (qemu-kvm) is misleading. This is actually targeting 
>> the uq/master patch queue, i.e. the upstream kvm staging area.
>>
> 
> If I want to submit a patch for the qemu-kvm-git,  should I use  
> "[QEMU-DEVEL][Patch]..." as the subject? Or there are other rules for 
> qemu-kvm upstream? If yes, would you like to tell me?. Thanks!

If you really have to target qemu-kvm only, then you tagging is fine.
But this patch does not qualify for such exclusiveness. Rather, your
feature should go into upstream's KVM first and will then be merged back
into qemu-kvm on next update.

> 
>> On 2011-05-05 05:03, brill...@viatech.com.cn wrote:
>>> When KVM is running on VIA CPU with host cpu's model, the
>> feautures of
>>> VIA CPU will be passed into kvm guest by calling the CPUID
>> instruction
>>> for Centaur.
>>>
>>> Signed-off-by: BrillyWu
>>> Signed-off-by: KaryJin
>>> ---
>>>  target-i386/cpu.h   |7 +++
>>>  target-i386/cpuid.c |   48
>>> +++-
>>
>> You patch is unfortunately line-wrapped.
> 
> Yes, I will be careful the next time.
> 
>>> @@ -721,6 +725,9 @@ typedef struct CPUX86State {
>>>  uint32_t cpuid_ext3_features;
>>>  uint32_t cpuid_apic_id;
>>>  int cpuid_vendor_override;
>>> +/*Store the results of Centaur's CPUID instructions*/
>>
>> Please format comments like this /* comment text */, ie. with blanks 
>> after/before the /* / */.
> OK, I will check it.
>>
>>> +1050,15 @@ void cpu_x86_cpuid(CPUX86State *env, uin
>>> uint32_t *ecx, uint32_t *edx)  {
>>>  /* test if maximum index reached */
>>> -if (index & 0x8000) {
>>> +if ((index & 0xC000) == 0xC000) {
>>> +   /* Handle the Centaur's CPUID instruction.*
>>> +   * If cpuid_xlevel2 is "0", then put into the*
>>> +   * default case. */
>>> +   if (env->cpuid_xlevel2 == 0)
>>> +   index = 0xF000;
>>> +   else if (index > env->cpuid_xlevel2)
>>> +   index = env->cpuid_xlevel2;
>>
>> Please validate your patch before posting with scripts/checkpatch.pl.
> 
> OK, I will do it. 
> I found that space is used to code indent other than tab, should I follow it 
> or use tab instead in my patch?
> If I use space, there are some warnings when using scripts/checkpatch.pl to 
> validate the patch. Can I ignore them?

Generally, the advices checkpatch provides are valid and shall be
applied. If you feel like you came across a corner case where the script
reports nonsense, please post your findings to qemu-devel.

Jan

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

Re: [Qemu-devel] Bug #757654: UHCI fails to signal stall response patch

[Jan Vesely]

hi,
using the modified (single line) patch only works half-way, it sets
the value in status register (guess that's what that line does :))
but hw interrupt is not generated.

I tried adding uhci_update_irq and this patch:

diff --git a/hw/usb-uhci.c b/hw/usb-uhci.c
index a65e0b3..1e9c1e7 100644
--- a/hw/usb-uhci.c
+++ b/hw/usb-uhci.c
@@ -702,11 +702,15 @@ out:
 case USB_RET_STALL:
 td->ctrl |= TD_CTRL_STALL;
 td->ctrl &= ~TD_CTRL_ACTIVE;
+s->status |= UHCI_STS_USBERR;
+uhci_update_irq(s);
 return 1;

 case USB_RET_BABBLE:
 td->ctrl |= TD_CTRL_BABBLE | TD_CTRL_STALL;
 td->ctrl &= ~TD_CTRL_ACTIVE;
+s->status |= UHCI_STS_USBERR;
+uhci_update_irq(s);
 /* frame interrupted */
 return -1;

works.

jan

On Thu, May 5, 2011 at 2:05 PM, Gerd Hoffmann  wrote:
>> diff --git a/hw/usb-uhci.c b/hw/usb-uhci.c
>> index 346db3e..a51d89b 100644
>> --- a/hw/usb-uhci.c
>> +++ b/hw/usb-uhci.c
>> @@ -732,11 +732,21 @@ out:
>>      case USB_RET_STALL:
>>          td->ctrl |= TD_CTRL_STALL;
>>          td->ctrl&= ~TD_CTRL_ACTIVE;
>> +        s->status |= UHCI_STS_USBERR;
>
> Just this line should be enougth.
>
>>      case USB_RET_BABBLE:
>>          td->ctrl |= TD_CTRL_BABBLE | TD_CTRL_STALL;
>>          td->ctrl&= ~TD_CTRL_ACTIVE;
>> +        s->status |= UHCI_STS_USBERR;
>
> Likewise.
>
> Tried that?
>
> cheers,
>  Gerd
>
>

Re: [Qemu-devel] BUG: 0.14.0 -device usb-host supports only one device

[erik . rull]

Hi Gerd,

sorry, but that parameter causes a non-complete boot of my Windows VM (I
have removed that) when using the ehci-patch.

Additionally, the ehci patch does not support that parameter type, there
you must add it via -usbdevice host:auto:*.*

Any ideas how to proceed?
I need the ehci patch because it removes the bluescreen issue when plugging
in USB 2.0 printers even if USB 2.0 is disabled in the host bios and the
USB 1.1 transfer performance is less that 64k/sec without the patch.

Best regards,

Erik

Re: [Qemu-devel] "General protection fault" exiting qemu with current linux-git.

[Rob Landley]

On 05/05/2011 07:34 PM, Rob Landley wrote:
> I'm running current linux-git (5895198c56d131cc696556) and current
> qemu-git (d2d979c628e4b2c4a3c I think) with the attached kernel .config,
> and it gave me a GPF on exit from the emulator.
> 
> I'm not sure which project is responsible, but I thought I'd throw it
> out there...
> 
> Rob

Here it is again with function names in the stack trace:

md: stopping all md devices.
e1000 :00:03.0: PCI INT A disabled
Restarting system.
machine restart
general protection fault: fff2 [#1] SMP
last sysfs file: /sys/devices/virtual/cpuid/cpu0/dev
CPU 0
Pid: 667, comm: oneit Not tainted 2.6.39-rc6+ #7 Bochs Bochs
RIP: 0010:[]  []
native_stop_other_cpus+0x77/0x80
RSP: 0018:880006c0be08  EFLAGS: 0246
RAX: 00ff RBX: 0246 RCX: 
RDX: 81020e50 RSI: 00ff RDI: 00f0
RBP: 816ceeb8 R08:  R09: 
R10:  R11: 0a74726174736572 R12: 0001
R13:  R14: 0004 R15: 004009e4
FS:  () GS:880007c0() knlGS:
CS:  0010 DS:  ES:  CR0: 8005003b
CR2: 00609910 CR3: 07b3b000 CR4: 06f0
DR0:  DR1:  DR2: 
DR3:  DR6:  DR7: 
Process oneit (pid: 667, threadinfo 880006c0a000, task 880007a5cb00)
Stack:
 fee1dead 28121969 01234567 8101c4cf
 004009e4 8101c214 fee1dead 81053ae7
    
Call Trace:
 [] ? native_machine_shutdown+0x5f/0x80
 [] ? native_machine_restart+0x24/0x40
 [] ? sys_reboot+0x1a7/0x230
 [] ? schedule_tail+0x21/0xa0
 [] ? ret_from_fork+0x13/0x80
 [] ? system_call_fastpath+0x16/0x1b
Code: 0f 1f 40 00 74 1b 48 89 c3 bf c7 10 00 00 e8 b1 ca 18 00 48 8b 7d
00 f3 48 0f b8 c7 83 f8 01 77 d3 9c 5b fa e8 db 0b 00 00 53 9d <5b> 5d
41 5c c3 0f 1f 40 00 48 83 ec 18 48 8b 05 7d 72 47 00 48
RIP  [] native_stop_other_cpus+0x77/0x80
 RSP 
---[ end trace 12a9e2cf16977a27 ]---

Re: [Qemu-devel] [PATCH 0/3]: QMP: Introduce inject-nmi command

[Markus Armbruster]

Blue Swirl  writes:

> On Mon, May 2, 2011 at 6:57 PM, Luiz Capitulino  
> wrote:
>> On Sat, 30 Apr 2011 09:33:15 +0300
>> Blue Swirl  wrote:
>>
>>> On Sat, Apr 30, 2011 at 1:40 AM, Luiz Capitulino  
>>> wrote:
>>> > This series introduces the inject-nmi command for QMP, which sends an
>>> > NMI to _all_ guest's CPUs.
>>> >
>>> > Also note that this series changes the human monitor nmi command to use
>>> > the QMP implementation, which means that it now has a DIFFERENT behavior.
>>> > Please, check patch 3/3 for details.
>>>
>>> As discussed earlier, please change the QMP version for future
>>> expandability so that instead of single command 'inject-nmi', 'inject'
>>> takes parameter 'nmi'. HMP command 'nmi' can remain for now, but
>>> 'inject' should be added.
>>
>> I'm not sure I agree with this, because we risky overloading 'inject' the
>> same way we did with the 'change' command.
>>
>> What's 'inject' supposed to do in the future?
>
> Inject other IRQs, for example inject nmi could become an alias to
> something like
> inject /apic@fee0:l1int
> which would be a shorthand for
> raise /apic@fee0:l1int
> lower /apic@fee0:l1int
>
> I think we only need a registration framework for IRQs and other signals.

Yes, we could use nicer infrastructure for modeling IRQs.  No, we
shouldn't reject Lai's work because it doesn't get us there.  Perfect is
the enemy of good.

Pick one:

1. We take inject-nmi now.  Should we get a more general inject command
like the one you envisage later, we can deprecate inject-nmi, and remove
it after a suitable grace time.  Big deal.  We get the special problem
solved now, without really compromising future solutions for the general
problem.

2. We reject inject-nmi now.  The itch Lai tries to scratch remains
unscratched until we get a more general inject command.

2a. Rejection "motivates" Lai to solve the general problem[*].  Or maybe
it motivates somebody else.  We get the general problem solved sooner.
And maybe I get a pony for my birthday, too.

2b. The general problem remains unsolved along with the special problem.
We get nothing.


[*] He's been trying to give us NMI injection via QMP for five months,
so what's a few months more to him.

[Qemu-devel] winCE on linux via qemu

[Rafal Fabich]

Good morning,

I would like to ask if there is any possibility to run WinCE applications in 
the embedded linux environment based on ARM processors by using the QEMU 
emulator?

Looking forward to hearing from you.

Best regards,
Rafal Fabich

[Qemu-devel] KVM migration with different source and dest paths

[Onkar Mahajan]

Is it possible to migrate KVM guest to different paths like this:

path of the image of guest01 on host A
/home/joe/guest01.img

path of the image of guest01 on host B
/home/bill/image/temp/guest01.img


is this possible ?

if it is any pointers as to how to do this ?


Regards,
Onnm

[Qemu-devel] winCE on linux via qemu

[Rafal Fabich]

Good morning,

I would like to ask if there is any possibility to run WinCE applications in 
the embedded linux environment based on ARM processors by using the QEMU 
emulator?

Looking forward to hearing from you.

Best regards,
Rafal Fabich

Re: [Qemu-devel] [PATCH] qemu-kvm: Add CPUID support for VIA CPU

[BrillyWu]

> > 
> > If I want to submit a patch for the qemu-kvm-git,  should I
> use  "[QEMU-DEVEL][Patch]..." as the subject? Or there are other rules 
> for qemu-kvm upstream? If yes, would you like to tell me?. Thanks!
> 
> If you really have to target qemu-kvm only, then you tagging is fine.
> But this patch does not qualify for such exclusiveness. 
> Rather, your feature should go into upstream's KVM first and will then 
> be merged back into qemu-kvm on next update.

I have submit a patch into upstream's KVM for supporting these features before, 
and the patch has been applied in kvm-git.
Do you mean that I should not submit this patch until the KVM's patch is merged 
back?
 
> > 
> >> On 2011-05-05 05:03, brill...@viatech.com.cn wrote:
> >>> When KVM is running on VIA CPU with host cpu's model, the
> >> feautures of
> >>> VIA CPU will be passed into kvm guest by calling the CPUID
> >> instruction
> >>> for Centaur.
> >>>
> >>> Signed-off-by: BrillyWu
> >>> Signed-off-by: KaryJin
> >>> ---
> >>>  target-i386/cpu.h   |7 +++
> >>>  target-i386/cpuid.c |   48
> >>> +++-
> >>
> >> You patch is unfortunately line-wrapped.
> > 
> > Yes, I will be careful the next time.
> > 
> >>> @@ -721,6 +725,9 @@ typedef struct CPUX86State {
> >>>  uint32_t cpuid_ext3_features;
> >>>  uint32_t cpuid_apic_id;
> >>>  int cpuid_vendor_override;
> >>> +/*Store the results of Centaur's CPUID instructions*/
> >>
> >> Please format comments like this /* comment text */, ie. 
> with blanks
> >> after/before the /* / */.
> > OK, I will check it.
> >>
> >>> +1050,15 @@ void cpu_x86_cpuid(CPUX86State *env, uin
> >>> uint32_t *ecx, uint32_t *edx)  {
> >>>  /* test if maximum index reached */
> >>> -if (index & 0x8000) {
> >>> +if ((index & 0xC000) == 0xC000) {
> >>> + /* Handle the Centaur's CPUID instruction.*
> >>> + * If cpuid_xlevel2 is "0", then put into the*
> >>> + * default case. */
> >>> + if (env->cpuid_xlevel2 == 0)
> >>> + index = 0xF000;
> >>> + else if (index > env->cpuid_xlevel2)
> >>> + index = env->cpuid_xlevel2;
> >>
> >> Please validate your patch before posting with
> scripts/checkpatch.pl.
> > 
> > OK, I will do it. 
> > I found that space is used to code indent other than tab,
> should I follow it or use tab instead in my patch?
> > If I use space, there are some warnings when using
> scripts/checkpatch.pl to validate the patch. Can I ignore them?
> 
> Generally, the advices checkpatch provides are valid and shall be 
> applied. If you feel like you came across a corner case where the 
> script reports nonsense, please post your findings to qemu-devel.
> 

I use the tab instead of space as code indent, and the checkpatch reports no 
warnings.
Thanks!

> Jan
> 
> --
> Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence 
> Center Embedded Linux
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in the 
> body of a message to majord...@vger.kernel.org More majordomo info at  
> http://vger.kernel.org/majordomo-info.html
> 

[Qemu-devel] [PATCH qemu-block 0/2] cleanup progress code

[Jes . Sorensen]

From: Jes Sorensen 

Hi,

Two small patches cleaning up the progress printing code - adding
documentation and removing some unneeded paranthesis. Also know as the
'happy markus' patch series

This is relative to the block branch.

Jes

Jes Sorensen (2):
  Add documentation for qemu_progres_print()
  qemu-img.c: Remove superfluous parenthesis

 qemu-img.c  |6 +++---
 qemu-progress.c |8 
 2 files changed, 11 insertions(+), 3 deletions(-)

-- 
1.7.4.4

[Qemu-devel] [PATCH 1/2] Add documentation for qemu_progres_print()

[Jes . Sorensen]

From: Jes Sorensen 

Signed-off-by: Jes Sorensen 
---
 qemu-progress.c |8 
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/qemu-progress.c b/qemu-progress.c
index a4894c0..70928d6 100644
--- a/qemu-progress.c
+++ b/qemu-progress.c
@@ -111,6 +111,14 @@ void qemu_progress_end(void)
 state.end();
 }
 
+/*
+ * Add delta to current state, and print the output if the current
+ * state has progressed more than min_skip since the last value was
+ * printed. 'max' specifies the relative percentage, ie. a function
+ * can count for 30% of the total work, and still count from 0-100, by
+ * setting max to 30. If max is set to zero, the percent argument
+ * becomes an absolute value for current state.
+ */
 void qemu_progress_print(float percent, int max)
 {
 float current;
-- 
1.7.4.4

[Qemu-devel] [PATCH 2/2] qemu-img.c: Remove superfluous parenthesis

[Jes . Sorensen]

From: Jes Sorensen 

Signed-off-by: Jes Sorensen 
---
 qemu-img.c |6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index e825123..1da5484 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -785,7 +785,7 @@ static int img_convert(int argc, char **argv)
 
 nb_sectors = total_sectors;
 local_progress = (float)100 /
-(nb_sectors / MIN(nb_sectors, (cluster_sectors)));
+(nb_sectors / MIN(nb_sectors, cluster_sectors));
 
 for(;;) {
 int64_t bs_num;
@@ -856,7 +856,7 @@ static int img_convert(int argc, char **argv)
 sector_num = 0; // total number of sectors converted so far
 nb_sectors = total_sectors - sector_num;
 local_progress = (float)100 /
-(nb_sectors / MIN(nb_sectors, (IO_BUF_SIZE / 512)));
+(nb_sectors / MIN(nb_sectors, IO_BUF_SIZE / 512));
 
 for(;;) {
 nb_sectors = total_sectors - sector_num;
@@ -1331,7 +1331,7 @@ static int img_rebase(int argc, char **argv)
 bdrv_get_geometry(bs, &num_sectors);
 
 local_progress = (float)100 /
-(num_sectors / MIN(num_sectors, (IO_BUF_SIZE / 512)));
+(num_sectors / MIN(num_sectors, IO_BUF_SIZE / 512));
 for (sector = 0; sector < num_sectors; sector += n) {
 
 /* How many sectors can we handle with the next read? */
-- 
1.7.4.4

[Qemu-devel] Add option to disable Cocoa on Mac OS X

[Ben Leslie]

Hi all,

Are there any objections to adding a --disable-cocoa configure option?
For simulating ARM microcontrollers I have no desire or need for graphics.

Thanks,

Benno

Re: [Qemu-devel] [PATCH 5/6] PPC: Implement e500 (FSL) MMU

[Alexander Graf]

On 03.05.2011, at 21:25, Scott Wood wrote:

> On Mon, 2 May 2011 17:03:21 +0200
> Alexander Graf  wrote:
> 
>> Most of the code to support e500 style MMUs is already in place, but
>> we're missing on some of the special TLB0-TLB1 handling code and slightly
>> different TLB modification.
>> 
>> This patch adds support for the FSL style MMU.
>> 
>> Signed-off-by: Alexander Graf 
> 
> You beat me to (part of) it... :-)
> 
> How is this going to interact with the KVM MMU API, where it was suggested
> that qemu use that representation as its native structure?  Should we just
> change the representation at that point, for both types of booke mmu (so 4xx
> would be represented with MAS registers internally, even though a
> different interface is exposed to the guest)?

Ugh - I completely forgot about that one. I guess we'll just have to redo the 
internal TLB resolution code at that point, yes. I somehow prefer to work my 
way from working solution to working solution though, so I don't think I'll 
move it over to a MAS based approach just yet.

> 
>> @@ -678,8 +887,9 @@ struct CPUPPCState {
>> int nb_BATs;
>> target_ulong DBAT[2][8];
>> target_ulong IBAT[2][8];
>> -/* PowerPC TLB registers (for 4xx and 60x software driven TLBs) */
>> +/* PowerPC TLB registers (for 4xx, e500 and 60x software driven TLBs) */
>> int nb_tlb;  /* Total number of TLB  
>> */
>> +int nb_tlbs[4];  /* Number of respective TLB entries (e500) 
>>  */
> 
> Looks like "number of tlbs", not "number of tlb entries"... Was less
> confusing when there was only one tlb array.

I certainly agree, but this way I don't have to change non-e500 code :). I 
actually started off creating a new TLB struct with 4 entries (the 4 TLBs 
allowed by the 2.06 spec), but quickly gave up as it's a _lot_ of code using 
these O_o.

Either way, it's all different now anyways :).

>> diff --git a/target-ppc/helper.c b/target-ppc/helper.c
>> index 5e4030b..261c1a9 100644
>> --- a/target-ppc/helper.c
>> +++ b/target-ppc/helper.c
>> @@ -1153,48 +1153,144 @@ void store_40x_sler (CPUPPCState *env, uint32_t val)
>> env->spr[SPR_405_SLER] = val;
>> }
>> 
>> +static inline int mmubooke_check_tlb (CPUState *env, ppcemb_tlb_t *tlb,
>> +  target_phys_addr_t *raddr, int *prot,
>> +  target_ulong address, int rw,
>> +  int access_type, int i)
>> +{
>> +int ret, _prot;
>> +
>> +if (ppcemb_tlb_check(env, tlb, raddr, address,
>> + env->spr[SPR_BOOKE_PID],
>> + !env->nb_pids, i) >= 0) {
>> +goto found_tlb;
>> +}
>> +
>> +if ((env->nb_pids > 1) && env->spr[SPR_BOOKE_PID1] &&
>> +ppcemb_tlb_check(env, tlb, raddr, address,
>> + env->spr[SPR_BOOKE_PID1], 0, i) >= 0) {
>> +goto found_tlb;
>> +}
>> +
>> +if ((env->nb_pids > 2) && env->spr[SPR_BOOKE_PID2] &&
>> +ppcemb_tlb_check(env, tlb, raddr, address,
>> + env->spr[SPR_BOOKE_PID2], 0, i) >= 0) {
>> +goto found_tlb;
>> +}
> 
> Maybe PID1/PID2 should be moved into ppcemb_tlb_check, and eliminate the
> nb_pids checks by putting zero in PID1/PID2 on chips that don't have it?
> I'm assuming this is performance sensitive for non-KVM.

PID1/PID2 are already 0 on chips that don't have them, so we could eliminate 
the check on nb_pids, yup. The small branch here really isn't too bad on 
performance though, so it's more of a readability thing than performance. We're 
currently killing performance anyways by flushing qemu's internal TLB on every 
MSR_IR/DR switch.

> 
>> +static int mmue500_get_physical_address (CPUState *env, mmu_ctx_t *ctx,
>> + target_ulong address, int rw,
>> + int access_type)
>> +{
>> +ppcemb_tlb_t *tlb;
>> +target_phys_addr_t raddr;
>> +int i, ret;
>> +uint32_t ea = (address >> MAS2_EPN_SHIFT) & 0x7f;
>> +
>> +ret = -1;
>> +raddr = (target_phys_addr_t)-1ULL;
>> +
>> +/* check TLB1 - hits often because the kernel is here */
> 
> I'd optimize for userspace instead -- that's where the real work is more
> likely to happen.  Plus, compare what's likely to be 5-6 iterations before
> finding the entry for a kernel large-page entry if we check TLB0 first,
> versus 17+ iterations (65+ on e500mc) for finding a TLB0 entry if TLB1 is
> checked first -- based on Linux's TLB1 usage patterns.

Good point!

> 
>> +for (i = env->nb_tlbs[0]; i < env->nb_tlb; i++) {
>> +tlb = &env->tlb[i].tlbe;
>> +ret = mmubooke_check_tlb(env, tlb, &raddr, &ctx->prot, address, rw,
>> + access_type, i);
>> +if (!ret) {
>> +goto found_tlb;
>> }
>> }
>> -if (ret >= 0)
>> +
>> +/* check possible TLB0 entries */
>> +f

Re: [Qemu-devel] [PATCH] qemu-kvm: Add CPUID support for VIA CPU

[Jan Kiszka]

On 2011-05-06 11:37, brill...@viatech.com.cn wrote:
> 
>>>
>>> If I want to submit a patch for the qemu-kvm-git,  should I
>> use  "[QEMU-DEVEL][Patch]..." as the subject? Or there are other rules 
>> for qemu-kvm upstream? If yes, would you like to tell me?. Thanks!
>>
>> If you really have to target qemu-kvm only, then you tagging is fine.
>> But this patch does not qualify for such exclusiveness. 
>> Rather, your feature should go into upstream's KVM first and will then 
>> be merged back into qemu-kvm on next update.
> 
> I have submit a patch into upstream's KVM for supporting these features 
> before, and the patch has been applied in kvm-git.

As far as I can see, nothing has been applied to any tree, neither
qemu.git nor qemu-kvm.git.

> Do you mean that I should not submit this patch until the KVM's patch is 
> merged back?

You still need to submit a fixed version of this patch against
qemu-kvm.git, uq/master branch (which is qemu.git effectively). Then
Marcelo or Avi can pick it up and push it to upstream. Once it's merged
there, qemu-kvm.git will update from upstream, and you will have your
patch applied to both trees.

Jan

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

Re: [Qemu-devel] [PATCH qemu-block 0/2] cleanup progress code

[Markus Armbruster]

jes.soren...@redhat.com writes:

> From: Jes Sorensen 
>
> Hi,
>
> Two small patches cleaning up the progress printing code - adding
> documentation and removing some unneeded paranthesis. Also know as the
> 'happy markus' patch series

Happy ACK :)

Re: [Qemu-devel] Add option to disable Cocoa on Mac OS X

[Tristan Gingold]

On May 6, 2011, at 11:48 AM, Ben Leslie wrote:

> Hi all,
> 
> Are there any objections to adding a --disable-cocoa configure option?
> For simulating ARM microcontrollers I have no desire or need for graphics.

Seconded.  I think I have once posted such a patch.

Tristan.

Re: [Qemu-devel] [PATCH 1/2] Add documentation for qemu_progres_print()

[Jes Sorensen]

On 05/06/11 12:40, Brad Hards wrote:
> On Fri, 6 May 2011 07:39:10 PM jes.soren...@redhat.com wrote:
>> +/*
>> + * Add delta to current state, and print the output if the current
>> + * state has progressed more than min_skip since the last value was
>> + * printed. 'max' specifies the relative percentage, ie. a function
>> + * can count for 30% of the total work, and still count from 0-100, by
>> + * setting max to 30. If max is set to zero, the percent argument
>> + * becomes an absolute value for current state.
>> + */
>>  void qemu_progress_print(float percent, int max)
> I hate to critique anyone adding docs, but this makes no sense at all to me 
> without reading the code. Is "percent" the amount we are adding (i.e. the 
> delta) or the result (i.e. absolute progress)? Or does it vary according to 
> the value of max?

What you add is a delta, which is relative to the max. We can change the
argument name of the function to be delta instead if that makes it
easier to follow.

Cheers,
Jes

[Qemu-devel] [Bug 458521] Re: kvm crash when using virtio for network, hardy guest

[John Johansen]

I went back and retested this and it has already been fixed and released
(commit fcbc05a1be0a7600153e78207dcb8b62fe753a4a), it was just not
properly closed.

mapl,
If you are running an updated hardy guest running the 2.6.24-29 kernel this bug 
should not be a problem.  Can you please provide information on which kernel 
you are using.  If you are still encountering a problem with an updated kernel 
this is a new bug and we will need to gather more information to reproduce.

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

Title:
  kvm crash when using virtio for network, hardy guest

Status in QEMU:
  Fix Released
Status in Release Notes for Ubuntu:
  Fix Released
Status in “linux” package in Ubuntu:
  Fix Released
Status in “qemu-kvm” package in Ubuntu:
  Fix Released
Status in “linux” source package in Hardy:
  In Progress
Status in “qemu-kvm” source package in Hardy:
  Invalid
Status in “linux” source package in Karmic:
  Fix Released
Status in “qemu-kvm” source package in Karmic:
  Fix Released

Bug description:
  Binary package hint: kvm

  I was running a kvm host with 5 virtual machines using jaunty. 1
  running centos 5.4, 3 running karmic and 1 running hardy.

  I upgraded the host to karmic to test it and one of my virtual
  machines(the vm with hardy) crashs after some seconds of boot! I stack
  traced the kvm process and found a "virtio-net truncating packet"
  before process crash.

  No error in any log file. The VM just crash.

  I changed the model of network interface(the disk is still virtio) in
  VM.xml to rtl8139, redefined and started. The problem goes away.

  I don't know if it's a kvm bug or libvirt(regarding the bridge
  network) bug but definitely is a bug. All other VMs are running OK
  with virtio for both network and disk.

  ===
  Karmic Release Notes:

  KVM Guest Crashes when Guest is Hardy and using Virtio Networking

  Ubuntu 8.04 LTS (Hardy) KVM guests using virtio networking may crash,
  when running on top of Ubuntu 9.10 (Karmic) hosts.

  As a workaround, such guests should use either e1000 or rtl839 as the
  networking model.  A fix for the bug is currently in progress and
  should be addressed in an update to the qemu-kvm package in Karmic.

  ===

  ===
  SRU Justification

  This bug is a regression from the kvm-84 package in 9.04.  Karmic users 
hosting 8.04 KVM guests and using virtio networking *will* crash their VM when 
the network connection is saturated.  As the virtual machine crashes without 
sync buffers, there could very well be data lost in the guest filesystem or 
memory.
  I have posted the patch for review, and acked upstream:
   * http://lists.gnu.org/archive/html/qemu-devel/2009-10/msg02495.html
  I am bundling this fix with two other minor fixes, for Bug #452323  and Bug 
#453441.

  TEST CASE:
   1) Download and uncompress
* http://rookery.canonical.com/~kirkland/hardy.img.bz2
   2) Download a bridged networking script
* http://rookery.canonical.com/~kirkland/bridge.sh
   3) Start the vm with virtio and bridged networking
* sudo /usr/bin/kvm -m 512 -net nic,model=virtio -net tap,script=bridge.sh 
-drive file=hardy.img,if=virtio,index=0,boot=on
   4) The VM's username and password are both "ubuntu".
   5) In the guest, have nc listen on a port
* nc -lp 1234 > /dev/null
   6) On your host, flood the guest with network traffic on that port
* cat /dev/urandom | nc -w 3 guest_ip 1234
   7) Without this fix, the guest will crash immediately, saying:
*   virtio-net truncating packet
   8) With the fix, you will be able to send the guest with data over the 
virtio network ad nauseum.  I get about 6MB/s throughput.

  ===

Re: [Qemu-devel] [Xen-devel] GSoC Introduction: Add Xen PV block device support to SeaBIOS

[Ian Campbell]

On Fri, 2011-05-06 at 11:25 +0100, Daniel Castro wrote:
> Good Day to all,
> 
> My Name is Daniel Castro I am going to be your google coder for this summer 
> in:
> 
> Add Xen PV block device support to SeaBIOS

Thanks Daniel and Welcome!

I'll be mentoring Daniel in this project.

(I've taken the liberty of CCing the SeaBIOS and qemu lists for their
information).

Cheers,
Ian.

> 
> About Me:
> This is my first time in Google Summer of Code and my first time
> contributing to an open source project. I hope I will keep
> contributing for many years to come to Xen Hypervisor. I was born and
> raised in Colombia and currently a PhD candidate in South Korea. I
> worked with Citrix XenServer for several years, but never worked with
> the open source version, and I have big expectations for Xen.
> 
> The Project:
> Currently Xen is using for HVM Virtual machines a version of BIOS
> called  ROMBIOS. Xen is going to introduce SeaBIOS (and upstream
> qemu), which happens to also be the qemu upstream and KVM BIOS of
> Choice. Then a modification to SeaBIOS hard drive abstract layer will
> be done to use the PV Drivers instead of the emulated device used in
> qemu-dm.
> Once SeaBIOS is in place my work starts as follows:
> We are going to provide PV frontend drivers for SeaBIOS in order to
> discourage the use of the emulated device. Removing the emulated
> device also removes the subtle confusion that can arise from booting
> from one device (emulated) but (apparently) running from another (PV
> frontend).  Around July we will present the community with a patch(s)
> that will enable to test this new functionality. Test for each kind of
> VM  (Windows HVM VMs) will be done and a compatibility matrix will be
> presented as part of the usual documentation.
> All activities and reports will be posted on:
> http://wiki.xensource.com/xenwiki/SeaBIOS
> 
> All questions or/and suggestions are very welcome,
> Thank you,
> Daniel
> 

Re: [Qemu-devel] [PATCH 1/2] Add documentation for qemu_progres_print()

[Brad Hards]

On Fri, 6 May 2011 07:39:10 PM jes.soren...@redhat.com wrote:
> +/*
> + * Add delta to current state, and print the output if the current
> + * state has progressed more than min_skip since the last value was
> + * printed. 'max' specifies the relative percentage, ie. a function
> + * can count for 30% of the total work, and still count from 0-100, by
> + * setting max to 30. If max is set to zero, the percent argument
> + * becomes an absolute value for current state.
> + */
>  void qemu_progress_print(float percent, int max)
I hate to critique anyone adding docs, but this makes no sense at all to me 
without reading the code. Is "percent" the amount we are adding (i.e. the 
delta) or the result (i.e. absolute progress)? Or does it vary according to 
the value of max?

Brad

[Qemu-devel] [PATCH 3/7] PPC: Add GS MSR definition

[Alexander Graf]

The BookE specification defines MSR bit 28 as Guest State. Add it
to the list of MSR macros.

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

diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 303f8ce..c6b2255 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -420,6 +420,7 @@ struct ppc_slb_t {
 #define MSR_CM   31 /* Computation mode for BookE hflags */
 #define MSR_ICM  30 /* Interrupt computation mode for BookE  */
 #define MSR_THV  29 /* hypervisor state for 32 bits PowerPC   hflags */
+#define MSR_GS   28 /* guest state for BookE */
 #define MSR_UCLE 26 /* User-mode cache lock enable for BookE */
 #define MSR_VR   25 /* altivec availablex hflags */
 #define MSR_SPE  25 /* SPE enable for BookE x hflags */
@@ -457,6 +458,7 @@ struct ppc_slb_t {
 #define msr_cm   ((env->msr >> MSR_CM)   & 1)
 #define msr_icm  ((env->msr >> MSR_ICM)  & 1)
 #define msr_thv  ((env->msr >> MSR_THV)  & 1)
+#define msr_gs   ((env->msr >> MSR_GS)   & 1)
 #define msr_ucle ((env->msr >> MSR_UCLE) & 1)
 #define msr_vr   ((env->msr >> MSR_VR)   & 1)
 #define msr_spe  ((env->msr >> MSR_SPE)  & 1)
-- 
1.6.0.2

[Qemu-devel] [PATCH 6/7] PPC MPC7544DS: Use new TLB helper function

[Alexander Graf]

Now that we have some nice helpers that can find us a TLB entry, let's
use that on the machine initialization code, so we don't need to know
about the internals of the TLB array.

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

diff --git a/hw/ppce500_mpc8544ds.c b/hw/ppce500_mpc8544ds.c
index 44d6440..4120977 100644
--- a/hw/ppce500_mpc8544ds.c
+++ b/hw/ppce500_mpc8544ds.c
@@ -179,7 +179,7 @@ static void mmubooke_create_initial_mapping(CPUState *env,
  target_ulong va,
  target_phys_addr_t pa)
 {
-ppcemb_tlb_t *tlb = &env->tlb[512].tlbe;
+ppcemb_tlb_t *tlb = booke206_get_tlbe(env, 1, 0, 0);
 
 tlb->attr = 0;
 tlb->prot = PAGE_VALID | ((PAGE_READ | PAGE_WRITE | PAGE_EXEC) << 4);
-- 
1.6.0.2

[Qemu-devel] [PATCH 7/7] PPC: Qdev'ify e500 pci

[Alexander Graf]

The e500 PCI controller isn't qdev'ified yet. This leads to severe issues
when running with -drive.

To be able to use a virtio disk with an e500 VM, let's convert the PCI
controller over to qdev.

Signed-off-by: Alexander Graf 

---

v2 -> v3:

  - rebase to current code base
  - fix endian issue
  - use sysbus helpers

v3 -> v4:

  - drop base_addr
---
 hw/ppce500_pci.c |  111 +++---
 1 files changed, 72 insertions(+), 39 deletions(-)

diff --git a/hw/ppce500_pci.c b/hw/ppce500_pci.c
index 83a20e4..f3db0a7 100644
--- a/hw/ppce500_pci.c
+++ b/hw/ppce500_pci.c
@@ -73,11 +73,10 @@ struct pci_inbound {
 };
 
 struct PPCE500PCIState {
+PCIHostState pci_state;
 struct pci_outbound pob[PPCE500_PCI_NR_POBS];
 struct pci_inbound pib[PPCE500_PCI_NR_PIBS];
 uint32_t gasket_time;
-PCIHostState pci_state;
-PCIDevice *pci_dev;
 };
 
 typedef struct PPCE500PCIState PPCE500PCIState;
@@ -250,7 +249,6 @@ static const VMStateDescription vmstate_ppce500_pci = {
 .minimum_version_id = 1,
 .minimum_version_id_old = 1,
 .fields  = (VMStateField[]) {
-VMSTATE_PCI_DEVICE_POINTER(pci_dev, PPCE500PCIState),
 VMSTATE_STRUCT_ARRAY(pob, PPCE500PCIState, PPCE500_PCI_NR_POBS, 1,
  vmstate_pci_outbound, struct pci_outbound),
 VMSTATE_STRUCT_ARRAY(pib, PPCE500PCIState, PPCE500_PCI_NR_PIBS, 1,
@@ -262,58 +260,93 @@ static const VMStateDescription vmstate_ppce500_pci = {
 
 PCIBus *ppce500_pci_init(qemu_irq pci_irqs[4], target_phys_addr_t registers)
 {
-PPCE500PCIState *controller;
+DeviceState *dev;
+PCIBus *b;
+PCIHostState *h;
+PPCE500PCIState *s;
 PCIDevice *d;
-int index;
 static int ppce500_pci_id;
+SysBusDevice *sb;
+
+dev = qdev_create(NULL, "e500-pcihost");
+sb = sysbus_from_qdev(dev);
+h = FROM_SYSBUS(PCIHostState, sb);
+s = DO_UPCAST(PPCE500PCIState, pci_state, h);
+
+b = pci_register_bus(&s->pci_state.busdev.qdev, NULL, mpc85xx_pci_set_irq,
+ mpc85xx_pci_map_irq, pci_irqs, PCI_DEVFN(0x11, 0), 4);
 
-controller = qemu_mallocz(sizeof(PPCE500PCIState));
+s->pci_state.bus = b;
+qdev_init_nofail(dev);
+d = pci_create_simple(b, 0, "e500-host-bridge");
 
-controller->pci_state.bus = pci_register_bus(NULL, "pci",
- mpc85xx_pci_set_irq,
- mpc85xx_pci_map_irq,
- pci_irqs, PCI_DEVFN(0x11, 0),
- 4);
-d = pci_register_device(controller->pci_state.bus,
-"host bridge", sizeof(PCIDevice),
-0, NULL, NULL);
+sysbus_mmio_map(sb, 0, registers + PCIE500_CFGADDR);
+sysbus_mmio_map(sb, 1, registers + PCIE500_CFGDATA);
+sysbus_mmio_map(sb, 2, registers + PCIE500_REG_BASE);
 
-pci_config_set_vendor_id(d->config, PCI_VENDOR_ID_FREESCALE);
-pci_config_set_device_id(d->config, PCI_DEVICE_ID_MPC8533E);
-pci_config_set_class(d->config, PCI_CLASS_PROCESSOR_POWERPC);
+/* XXX load/save code not tested. */
+vmstate_register(&d->qdev, ppce500_pci_id++, &vmstate_ppce500_pci, s);
+
+return b;
+}
+
+static int e500_pcihost_initfn(SysBusDevice *dev)
+{
+PCIHostState *h;
+PPCE500PCIState *s;
+/* XXX qdev var */
+int index;
 
-controller->pci_dev = d;
+h = FROM_SYSBUS(PCIHostState, sysbus_from_qdev(dev));
+s = DO_UPCAST(PPCE500PCIState, pci_state, h);
 
 /* CFGADDR */
-index = pci_host_conf_register_mmio(&controller->pci_state,
-DEVICE_BIG_ENDIAN);
+index = pci_host_conf_register_mmio(&s->pci_state, DEVICE_BIG_ENDIAN);
 if (index < 0)
-goto free;
-cpu_register_physical_memory(registers + PCIE500_CFGADDR, 4, index);
+return -1;
+sysbus_init_mmio(dev, 4, index);
 
 /* CFGDATA */
-index = pci_host_data_register_mmio(&controller->pci_state,
-DEVICE_BIG_ENDIAN);
+index = pci_host_data_register_mmio(&s->pci_state, DEVICE_LITTLE_ENDIAN);
 if (index < 0)
-goto free;
-cpu_register_physical_memory(registers + PCIE500_CFGDATA, 4, index);
+return -1;
+sysbus_init_mmio(dev, 4, index);
 
 index = cpu_register_io_memory(e500_pci_reg_read,
-   e500_pci_reg_write, controller,
-   DEVICE_NATIVE_ENDIAN);
+   e500_pci_reg_write, s, DEVICE_BIG_ENDIAN);
 if (index < 0)
-goto free;
-cpu_register_physical_memory(registers + PCIE500_REG_BASE,
-   PCIE500_REG_SIZE, index);
+return -1;
+sysbus_init_mmio(dev, PCIE500_REG_SIZE, index);
+return 0;
+}
 
-/* XXX load/save code not tested. */
-vmstate_register(&d->qdev, ppce500_p

[Qemu-devel] [PATCH 0/7] PPC: Add FSL (e500) MMU emulation v4

[Alexander Graf]

In a global effort to get rid of KVM-only targets, this is the next
important piece of the puzzle: e500 emulation :).

We had support for running an e500 KVM guest for a while now, but the
code could not be tested without a real e500 machine, because it required
KVM to work. This patchset adds emulation for the e500 MMU, enabling
anyone to use the MPC8544DS emulation.

v1 -> v2:

  - fix linux-user build
  - optimize tlb invalidate & search

v2 -> v3:

  - add qdev patch (enables -drive if=virtio)
  - fix mpc initial tlb size comment
  - enable cpu reset

v3 -> v4:

  - remove tlb_nbs
  - rename BOOKE_FSL to BOOKE206
  - generalize implementation to be extensible for future MMUs
  - check if tsize should be set
  - check if iprot should be set
  - set RPN on tlb miss
  - implement tlb_invalidate_all
  - use specific helper to set PIDs
  - create generic booke206 tlb flushing function
  - drop base_addr
  - new patch: MPC7544DS: Use new TLB helper function

Alexander Graf (7):
  PPC: Make MPC8544DS obey -cpu switch
  PPC: Make MPC8544DS emulation work w/o KVM
  PPC: Add GS MSR definition
  PPC: Add another 64 bits to instruction feature mask
  PPC: Implement e500 (FSL) MMU
  PPC MPC7544DS: Use new TLB helper function
  PPC: Qdev'ify e500 pci

 hw/ppce500_mpc8544ds.c  |   92 +++---
 hw/ppce500_pci.c|  111 ++--
 target-ppc/cpu.h|  307 ++-
 target-ppc/helper.c |  277 +++
 target-ppc/helper.h |6 +
 target-ppc/op_helper.c  |  296 +
 target-ppc/translate.c  |  116 +++--
 target-ppc/translate_init.c |  282 ---
 8 files changed, 1282 insertions(+), 205 deletions(-)

[Qemu-devel] [PATCH 1/7] PPC: Make MPC8544DS obey -cpu switch

[Alexander Graf]

The MPC8544DS board emulation code ignored the user defined -cpu switch.
This patch enables it to only provide a sane default, not force an e500v2
CPU inside.

Signed-off-by: Alexander Graf 
---
 hw/ppce500_mpc8544ds.c |6 +-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/hw/ppce500_mpc8544ds.c b/hw/ppce500_mpc8544ds.c
index e111dda..1b8a1c4 100644
--- a/hw/ppce500_mpc8544ds.c
+++ b/hw/ppce500_mpc8544ds.c
@@ -178,7 +178,11 @@ static void mpc8544ds_init(ram_addr_t ram_size,
 qemu_irq *irqs, *mpic, *pci_irqs;
 
 /* Setup CPU */
-env = cpu_ppc_init("e500v2_v30");
+if (cpu_model == NULL) {
+cpu_model = "e500v2_v30";
+}
+
+env = cpu_ppc_init(cpu_model);
 if (!env) {
 fprintf(stderr, "Unable to initialize CPU!\n");
 exit(1);
-- 
1.6.0.2

Re: [Qemu-devel] Add option to disable Cocoa on Mac OS X

[Alexander Graf]

On 06.05.2011, at 12:46, Tristan Gingold wrote:

> 
> On May 6, 2011, at 11:48 AM, Ben Leslie wrote:
> 
>> Hi all,
>> 
>> Are there any objections to adding a --disable-cocoa configure option?
>> For simulating ARM microcontrollers I have no desire or need for graphics.
> 
> Seconded.  I think I have once posted such a patch.

Andreas?


Alex

[Qemu-devel] [PATCH 4/7] PPC: Add another 64 bits to instruction feature mask

[Alexander Graf]

To enable quick runtime detection of instruction groups to the currently
selected CPU emulation, we have a feature mask of what exactly the respective
instruction supports.

This feature mask is 64 bits long and we just successfully exceeded those 64
bits. To add more features, we need to think of something.

The easiest solution that came to my mind was to simply add another 64 bits
that we can also match on. Since the comparison is only done on start of the
qemu process to generate an internal opcode calling table, we should be fine
on any performance penalties here.

Signed-off-by: Alexander Graf 
---
 target-ppc/cpu.h|1 +
 target-ppc/translate.c  |   25 +++--
 target-ppc/translate_init.c |  123 +++
 3 files changed, 110 insertions(+), 39 deletions(-)

diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index c6b2255..2a7431c 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -722,6 +722,7 @@ struct CPUPPCState {
 int bfd_mach;
 uint32_t flags;
 uint64_t insns_flags;
+uint64_t insns_flags2;
 
 #if defined(TARGET_PPC64) && !defined(CONFIG_USER_ONLY)
 target_phys_addr_t vpa;
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 99f572a..95813f2 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -201,6 +201,8 @@ struct opc_handler_t {
 uint32_t inval;
 /* instruction type */
 uint64_t type;
+/* extended instruction type */
+uint64_t type2;
 /* handler */
 void (*handler)(DisasContext *ctx);
 #if defined(DO_PPC_STATISTICS) || defined(PPC_DUMP_CPU)
@@ -314,10 +316,16 @@ static inline void gen_sync_exception(DisasContext *ctx)
 }
 
 #define GEN_HANDLER(name, opc1, opc2, opc3, inval, type)  \
-GEN_OPCODE(name, opc1, opc2, opc3, inval, type)
+GEN_OPCODE(name, opc1, opc2, opc3, inval, type, PPC_NONE)
+
+#define GEN_HANDLER_E(name, opc1, opc2, opc3, inval, type, type2) \
+GEN_OPCODE(name, opc1, opc2, opc3, inval, type, type2)
 
 #define GEN_HANDLER2(name, onam, opc1, opc2, opc3, inval, type)   \
-GEN_OPCODE2(name, onam, opc1, opc2, opc3, inval, type)
+GEN_OPCODE2(name, onam, opc1, opc2, opc3, inval, type, PPC_NONE)
+
+#define GEN_HANDLER2_E(name, onam, opc1, opc2, opc3, inval, type, type2)  \
+GEN_OPCODE2(name, onam, opc1, opc2, opc3, inval, type, type2)
 
 typedef struct opcode_t {
 unsigned char opc1, opc2, opc3;
@@ -457,7 +465,7 @@ static inline target_ulong MASK(uint32_t start, uint32_t 
end)
 /* PowerPC instructions table*/
 
 #if defined(DO_PPC_STATISTICS)
-#define GEN_OPCODE(name, op1, op2, op3, invl, _typ)   \
+#define GEN_OPCODE(name, op1, op2, op3, invl, _typ, _typ2)\
 { \
 .opc1 = op1,  \
 .opc2 = op2,  \
@@ -466,12 +474,13 @@ static inline target_ulong MASK(uint32_t start, uint32_t 
end)
 .handler = {  \
 .inval   = invl,  \
 .type = _typ, \
+.type2 = _typ2,   \
 .handler = &gen_##name,   \
 .oname = stringify(name), \
 },\
 .oname = stringify(name), \
 }
-#define GEN_OPCODE2(name, onam, op1, op2, op3, invl, _typ)\
+#define GEN_OPCODE2(name, onam, op1, op2, op3, invl, _typ, _typ2) \
 { \
 .opc1 = op1,  \
 .opc2 = op2,  \
@@ -480,13 +489,14 @@ static inline target_ulong MASK(uint32_t start, uint32_t 
end)
 .handler = {  \
 .inval   = invl,  \
 .type = _typ, \
+.type2 = _typ2,   \
 .handler = &gen_##name,   \
 .oname = onam,\
 },\
 .oname = onam,\
 }
 #else
-#define GEN_OPCODE(name, op1, op2, op3, invl, _typ)

[Qemu-devel] [PATCH 2/7] PPC: Make MPC8544DS emulation work w/o KVM

[Alexander Graf]

The MPC8544DS board emulation was only used with KVM so far, so some
parts of the code didn't provide proper values for non-KVM execution.

This patch makes the machine work without KVM enabled. To actually use
this, you also need proper e500v2 MMU emulation.

Signed-off-by: Alexander Graf 

---

v2 -> v3:

  - fix mpc initial tlb size comment
  - enable cpu reset
---
 hw/ppce500_mpc8544ds.c |   86 +++
 1 files changed, 71 insertions(+), 15 deletions(-)

diff --git a/hw/ppce500_mpc8544ds.c b/hw/ppce500_mpc8544ds.c
index 1b8a1c4..44d6440 100644
--- a/hw/ppce500_mpc8544ds.c
+++ b/hw/ppce500_mpc8544ds.c
@@ -28,6 +28,7 @@
 #include "kvm_ppc.h"
 #include "device_tree.h"
 #include "openpic.h"
+#include "ppc.h"
 #include "ppce500.h"
 #include "loader.h"
 #include "elf.h"
@@ -50,6 +51,12 @@
 #define MPC8544_PCI_IO 0xE100
 #define MPC8544_PCI_IOLEN  0x1
 
+static struct boot_info
+{
+uint32_t dt_base;
+uint32_t entry;
+} boot_info;
+
 #ifdef CONFIG_FDT
 static int mpc8544_copy_soc_cell(void *fdt, const char *node, const char *prop)
 {
@@ -82,7 +89,7 @@ static int mpc8544_load_device_tree(target_phys_addr_t addr,
 {
 int ret = -1;
 #ifdef CONFIG_FDT
-uint32_t mem_reg_property[] = {0, ramsize};
+uint32_t mem_reg_property[] = {0, cpu_to_be32(ramsize)};
 char *filename;
 int fdt_size;
 void *fdt;
@@ -103,15 +110,19 @@ static int mpc8544_load_device_tree(target_phys_addr_t 
addr,
 if (ret < 0)
 fprintf(stderr, "couldn't set /memory/reg\n");
 
-ret = qemu_devtree_setprop_cell(fdt, "/chosen", "linux,initrd-start",
-initrd_base);
-if (ret < 0)
-fprintf(stderr, "couldn't set /chosen/linux,initrd-start\n");
+if (initrd_size) {
+ret = qemu_devtree_setprop_cell(fdt, "/chosen", "linux,initrd-start",
+initrd_base);
+if (ret < 0) {
+fprintf(stderr, "couldn't set /chosen/linux,initrd-start\n");
+}
 
-ret = qemu_devtree_setprop_cell(fdt, "/chosen", "linux,initrd-end",
-(initrd_base + initrd_size));
-if (ret < 0)
-fprintf(stderr, "couldn't set /chosen/linux,initrd-end\n");
+ret = qemu_devtree_setprop_cell(fdt, "/chosen", "linux,initrd-end",
+(initrd_base + initrd_size));
+if (ret < 0) {
+fprintf(stderr, "couldn't set /chosen/linux,initrd-end\n");
+}
+}
 
 ret = qemu_devtree_setprop_string(fdt, "/chosen", "bootargs",
   kernel_cmdline);
@@ -145,6 +156,13 @@ static int mpc8544_load_device_tree(target_phys_addr_t 
addr,
 
 mpc8544_copy_soc_cell(fdt, buf, "clock-frequency");
 mpc8544_copy_soc_cell(fdt, buf, "timebase-frequency");
+} else {
+const uint32_t freq = 4;
+
+qemu_devtree_setprop_cell(fdt, "/cpus/PowerPC,8544@0",
+  "clock-frequency", freq);
+qemu_devtree_setprop_cell(fdt, "/cpus/PowerPC,8544@0",
+  "timebase-frequency", freq);
 }
 
 ret = rom_add_blob_fixed(BINARY_DEVICE_TREE_FILE, fdt, fdt_size, addr);
@@ -156,6 +174,35 @@ out:
 return ret;
 }
 
+/* Create -kernel TLB entries for BookE, linearly spanning 256MB.  */
+static void mmubooke_create_initial_mapping(CPUState *env,
+ target_ulong va,
+ target_phys_addr_t pa)
+{
+ppcemb_tlb_t *tlb = &env->tlb[512].tlbe;
+
+tlb->attr = 0;
+tlb->prot = PAGE_VALID | ((PAGE_READ | PAGE_WRITE | PAGE_EXEC) << 4);
+tlb->size = 256 * 1024 * 1024;
+tlb->EPN = va & TARGET_PAGE_MASK;
+tlb->RPN = pa & TARGET_PAGE_MASK;
+tlb->PID = 0;
+}
+
+static void mpc8544ds_cpu_reset(void *opaque)
+{
+CPUState *env = opaque;
+struct boot_info *bi = env->load_info;
+
+cpu_reset(env);
+
+/* Set initial guest state. */
+env->gpr[1] = (16<<20) - 8;
+env->gpr[3] = bi->dt_base;
+env->nip = bi->entry;
+mmubooke_create_initial_mapping(env, 0, 0);
+}
+
 static void mpc8544ds_init(ram_addr_t ram_size,
  const char *boot_device,
  const char *kernel_filename,
@@ -188,6 +235,13 @@ static void mpc8544ds_init(ram_addr_t ram_size,
 exit(1);
 }
 
+/* XXX register timer? */
+ppc_emb_timers_init(env, 4, PPC_INTERRUPT_DECR);
+ppc_dcr_init(env, NULL, NULL);
+
+/* Register reset handler */
+qemu_register_reset(mpc8544ds_cpu_reset, env);
+
 /* Fixup Memory size on a alignment boundary */
 ram_size &= ~(RAM_SIZES_ALIGN - 1);
 
@@ -265,6 +319,9 @@ static void mpc8544ds_init(ram_addr_t ram_size,
 
 /* If we're loading a kernel directly, we must load the device tree too. */
 if (kernel_filename) {
+#ifndef CONFIG_FDT
+cpu_abort(env, "Compiled without FDT sup

[Qemu-devel] [PATCH 5/7] PPC: Implement e500 (FSL) MMU

[Alexander Graf]

Most of the code to support e500 style MMUs is already in place, but
we're missing on some of the special TLB0-TLB1 handling code and slightly
different TLB modification.

This patch adds support for the FSL style MMU.

Signed-off-by: Alexander Graf 

---

v1 -> v2:

  - fix linux-user build
  - optimize tlb invalidate & search

v3 -> v4:

  - remove tlb_nbs
  - rename BOOKE_FSL to BOOKE206
  - generalize implementation to be extensible for future MMUs
  - check if tsize should be set
  - check if iprot should be set
  - set RPN on tlb miss
  - implement tlb_invalidate_all
  - use specific helper to set PIDs
  - create generic booke206 tlb flushing function
  - make e500 check more explicit
  - rename functions to booke206
  - rework page size finder

  Special thanks go to Scott here for pointing out all the issues :)
---
 target-ppc/cpu.h|  304 ++-
 target-ppc/helper.c |  277 +++
 target-ppc/helper.h |6 +
 target-ppc/op_helper.c  |  296 +
 target-ppc/translate.c  |   91 -
 target-ppc/translate_init.c |  163 ---
 6 files changed, 1024 insertions(+), 113 deletions(-)

diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 2a7431c..20d995a 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -108,8 +108,8 @@ enum powerpc_mmu_t {
 POWERPC_MMU_MPC8xx = 0x0007,
 /* BookE MMU model */
 POWERPC_MMU_BOOKE  = 0x0008,
-/* BookE FSL MMU model */
-POWERPC_MMU_BOOKE_FSL  = 0x0009,
+/* BookE 2.06 MMU model*/
+POWERPC_MMU_BOOKE206   = 0x0009,
 /* PowerPC 601 MMU model (specific BATs format)*/
 POWERPC_MMU_601= 0x000A,
 #if defined(TARGET_PPC64)
@@ -608,6 +608,224 @@ enum {
 #define vscr_sat   (((env->vscr) >> VSCR_SAT)  & 0x1)
 
 /*/
+/* BookE e500 MMU registers */
+
+#define MAS0_NV_SHIFT  0
+#define MAS0_NV_MASK   (0xfff << MAS0_NV_SHIFT)
+
+#define MAS0_WQ_SHIFT  12
+#define MAS0_WQ_MASK   (3 << MAS0_WQ_SHIFT)
+/* Write TLB entry regardless of reservation */
+#define MAS0_WQ_ALWAYS (0 << MAS0_WQ_SHIFT)
+/* Write TLB entry only already in use */
+#define MAS0_WQ_COND   (1 << MAS0_WQ_SHIFT)
+/* Clear TLB entry */
+#define MAS0_WQ_CLR_RSRV   (2 << MAS0_WQ_SHIFT)
+
+#define MAS0_HES_SHIFT 14
+#define MAS0_HES   (1 << MAS0_HES_SHIFT)
+
+#define MAS0_ESEL_SHIFT16
+#define MAS0_ESEL_MASK (0xfff << MAS0_ESEL_SHIFT)
+
+#define MAS0_TLBSEL_SHIFT  28
+#define MAS0_TLBSEL_MASK   (3 << MAS0_TLBSEL_SHIFT)
+#define MAS0_TLBSEL_TLB0   (0 << MAS0_TLBSEL_SHIFT)
+#define MAS0_TLBSEL_TLB1   (1 << MAS0_TLBSEL_SHIFT)
+#define MAS0_TLBSEL_TLB2   (2 << MAS0_TLBSEL_SHIFT)
+#define MAS0_TLBSEL_TLB3   (3 << MAS0_TLBSEL_SHIFT)
+
+#define MAS0_ATSEL_SHIFT   31
+#define MAS0_ATSEL (1 << MAS0_ATSEL_SHIFT)
+#define MAS0_ATSEL_TLB 0
+#define MAS0_ATSEL_LRATMAS0_ATSEL
+
+#define MAS1_TSIZE_SHIFT   8
+#define MAS1_TSIZE_MASK(0xf << MAS1_TSIZE_SHIFT)
+
+#define MAS1_TS_SHIFT  12
+#define MAS1_TS(1 << MAS1_TS_SHIFT)
+
+#define MAS1_IND_SHIFT 13
+#define MAS1_IND   (1 << MAS1_IND_SHIFT)
+
+#define MAS1_TID_SHIFT 16
+#define MAS1_TID_MASK  (0x3fff << MAS1_TID_SHIFT)
+
+#define MAS1_IPROT_SHIFT   30
+#define MAS1_IPROT (1 << MAS1_IPROT_SHIFT)
+
+#define MAS1_VALID_SHIFT   31
+#define MAS1_VALID 0x8000
+
+#define MAS2_EPN_SHIFT 12
+#define MAS2_EPN_MASK  (0xf << MAS2_EPN_SHIFT)
+
+#define MAS2_ACM_SHIFT 6
+#define MAS2_ACM   (1 << MAS2_ACM_SHIFT)
+
+#define MAS2_VLE_SHIFT 5
+#define MAS2_VLE   (1 << MAS2_VLE_SHIFT)
+
+#define MAS2_W_SHIFT   4
+#define MAS2_W (1 << MAS2_W_SHIFT)
+
+#define MAS2_I_SHIFT   3
+#define MAS2_I (1 << MAS2_I_SHIFT)
+
+#define MAS2_M_SHIFT   2
+#define MAS2_M (1 << MAS2_M_SHIFT)
+
+#define MAS2_G_SHIFT   1
+#define MAS2_G (1 << MAS2_G_SHIFT)
+
+#define MAS2_E_SHIFT   0
+#define MAS2_E (1 << MAS2_E_SHIFT)
+
+#define MAS3_RPN_SHIFT 12
+#define MAS3_RPN_MASK  (0xf << MAS3_RPN_SHIFT)
+
+#define MAS3_U0 0x0200
+#define MAS3_U1 0x0100
+#define MAS3_U2 0x0080
+#define MAS3_U3 0x0040
+#define MAS3_UX 0x0020
+#define MAS3_SX 0x0010
+#define MAS3_UW 0x0008
+#define MAS3_SW 0x0004
+#define MAS3_UR 0x0002
+#define MAS3_SR 0x0001
+#define MAS3_SPSIZE_SHIFT   1
+#define MAS3_SPSIZE_MASK(0x3e << MAS3_SPSIZE_SHIFT)
+
+#define MAS4_TLBS

Re: [Qemu-devel] virtio-scsi spec, first public draft

[Stefan Hajnoczi]

On Thu, May 5, 2011 at 3:50 PM, Paolo Bonzini  wrote:
> On 05/05/2011 04:29 PM, Hannes Reinecke wrote:
>>>
>>> I chose 1 requestq per target so that, with MSI-X support, each
>>> target can be associated to one MSI-X vector.
>>>
>>> If you want a large number of units, you can subdivide targets into
>>> logical units, or use multiple adapters if you prefer. We can have
>>> 20-odd SCSI adapters, each with 65534 targets. I think we're way
>>> beyond the practical limits even before LUN support is added to QEMU.
>>
>> But this will make queue full tracking harder.
>> If we have one queue per LUN the SCSI stack is able to track QUEUE FULL
>> states and will adjust the queue depth accordingly.
>> When we have only one queue per target we cannot track QUEUE FULL
>> anymore and have to rely on the static per-host 'can_queue' setting.
>> Which doesn't work as well, especially in a virtualized environment
>> where the queue full conditions might change at any time.
>
> So you want one virtqueue per LUN?  I had it in the first version, but then
> you had to associate a (target, 8-byte LUN) pair to each virtqueue manually.
>  That was very hairy, so I changed it to one target per queue.
>
>> But read on:
>>
>>> For comparison, Windows supports up to 1024 targets per adapter
>>> (split across 8 channels); IBM vSCSI provides up to 128; VMware
>>> supports a maximum of 15 SCSI targets per adapter and 4 adapters per
>>> VM.
>>>
>> We don't have to impose any hard limits here. The virtio scsi transport
>> would need to be able to detect the targets, and we would be using
>> whatever targets have been found.
>
> Yes, that's what I wrote above.  Right now "detect the targets" means "send
> INQUIRY for LUN0 and/or REPORT LUNS to each virtqueue", thanks to the 1:1
> relationship.  In my first version it would mean:
>
> - associate each target's LUN0 to a virtqueue
> - if needed, send INQUIRY for LUN0 and/or REPORT LUNS
> - if needed, deassociate the LUN0 and the virtqueue
>
> Really, it was ugly.  It also brings a lot more the question, such as what
> to do if a virtqueue has pending requests at deassociation time.
>
>>> Yes, just add the first LUN to it (it will be LUN0 which must be
>>> there anyway). The target's existence will be reported on the
>>> control receiveq.
>>>
>> ?? How is this supposed to work?
>> How can I detect the existence of a virtqueue ?
>
> Config space tells you how many virtqueue exist.  That gives how many
> targets you can address at most.  If some of them are empty at the beginning
> of the guest's life, their LUN0 will fail to answer INQUIRY and REPORT LUNS.
>
> (It is the same for vmw_pvscsi by the way, except simpler: the maximum # of
> targets is not configurable, and there is just one queue + one interrupt).

Okay, this explains how you plan to handle targets appearing - you
want to set a maximum number of targets.  I was wondering how we would
add virtqueues dynamically (and why the control vqs are placed last at
n,n+1 instead of 0,1).  Like Hannes said, why introduce a limit here
if we don't have to?

I'm really not sure I understand the win of creating lots of
virtqueues.  I just want a pipe out onto the SCSI bus so I can talk to
all devices in the SCSI domain.  Creating separate virtqueues
increases complexity in the driver and emulation IMO.

What is the MSI-X win you mentioned?  I guess if an application on
vcpu0 is accessing target0 a lot then interrupt handling can be
handled on vcpu0 while other vcpus handle interrupts for other SCSI
targets?  I remember VMware pv scsi has a trick here, each request can
contain the vcpu number which influences interrupt routing somehow.

Stefan

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

[Boris Derzhavets]

Uncompress regular Natty qemu-kvm package :-

qemu-kvm_0.14.0+noroms-0ubuntu4.diff.gz
qemu-kvm_0.14.0+noroms-0ubuntu4.dsc
qemu-kvm_0.14.0+noroms.orig.tar.gz

# dpkg-source -x qemu-kvm_0.14.0+noroms-0ubuntu4.dsc
# cd qemu-kvm_0.14.0+noroms-0ubuntu4

Edit  debian/rules and comment following  line ( per Serge Hallyn)

#   $(MAKE) -f debian/rules unpatch

to get patching to work !!

That's a reason why hack "spice-qxl-locking-fix-for-qemu-kvm.patch"
didn't work originally.

Change ./configure part of debian/rules adding "--enable-spice" :-

config-host.mak: $(QUILT_STAMPFN)
dh_testdir
./configure \
--target-list="x86_64-softmmu i386-softmmu x86_64-linux-user 
i386-linux-user" \
--prefix=/usr \
--interp-prefix=/etc/qemu-binfmt/%M \
--disable-blobs \
--disable-strip \
--enable-spice \
$(conf_arch)
Save

Edit debian/patches/series:-

larger_default_ram_size.patch
Detect-and-use-GCC-atomic-builtins-for-locking.patch
# spice-qxl-locking-fix-for-qemu-kvm.patch
0001-qxl-spice-display-move-pipe-to-ssd.patch
0002-qxl-implement-get_command-in-vga-mode-without-locks.patch
0003-qxl-spice-remove-qemu_mutex_-un-lock_iothread-around.patch
0004-hw-qxl-render-drop-cursor-locks-replace-with-pipe.patch

Save and copy required patches to debian/patches.

Edit debian/control like in SPICE2 PPA:-

Build-Depends: bcc,
   bzip2,
   debhelper (>= 7),
   device-tree-compiler [powerpc],
   iasl,
   libaio-dev,
   libasound2-dev,
   libcelt051-dev,
   libcurl4-gnutls-dev,
   libgnutls-dev,
   libncurses5-dev,
   libpci-dev,
   libpixman-1-dev,
   libpulse-dev,
   libsasl2-dev,
   libsdl1.2-dev (>= 1.2.14),
   libssl-dev,
   libx11-dev,
   nasm,
   perl,
   pkg-config,
   quilt (>= 0.40),
   libspice-server-dev,
   spice-protocol-dev,
   sysv-rc (>= 2.86.ds1-14.1ubuntu2),
   texi2html,
   texinfo,
   uuid-dev,
   zlib1g-dev
Save.

# touch -f debian/chagelog
# cd ..
# dpkg-buildpackage -rfakeroot -b -us -uc
# cd ..

Remove regular qemu-kvm package via Synaptic Manager
Install been built just now :-

root@boris-System-P5Q3:~/QEMU14# dpkg -i 
./qemu_0.14.0+noroms-0ubuntu4_amd64.deb \
> qemu-common_0.14.0+noroms-0ubuntu4_all.deb


root@boris-System-P5Q3:~/QEMU14# ls -l /usr/bin/kvm*
lrwxrwxrwx 1 root root18 2011-05-06 15:24 /usr/bin/kvm -> qemu-system-x86_64
lrwxrwxrwx 1 root root 8 2011-05-06 15:24 /usr/bin/kvm-img -> qemu-img
lrwxrwxrwx 1 root root 8 2011-05-06 15:24 /usr/bin/kvm-nbd -> qemu-nbd
-rwxr-xr-x 1 root root 11792 2011-02-22 16:34 /usr/bin/kvm_stat

root@boris-System-P5Q3:~/QEMU14# ls -l /usr/bin/qemu*
-rwxr-xr-x 1 root root 3520568 2011-05-06 15:08 /usr/bin/qemu
-rwxr-xr-x 1 root root 2014976 2011-05-06 15:08 /usr/bin/qemu-i386
-rwxr-xr-x 1 root root 270 2011-05-06 14:04 /usr/bin/qemu-ifdown
-rwxr-xr-x 1 root root 138 2011-02-22 16:34 /usr/bin/qemu-ifup
-rwxr-xr-x 1 root root  350640 2011-05-06 15:08 /usr/bin/qemu-img
-rwxr-xr-x 1 root root  362928 2011-05-06 15:08 /usr/bin/qemu-io
-rwxr-xr-x 1 root root  338384 2011-05-06 15:08 /usr/bin/qemu-nbd
lrwxrwxrwx 1 root root   4 2011-05-06 15:24 /usr/bin/qemu-system-i386 -> 
qemu
-rwxr-xr-x 1 root root 3590968 2011-05-06 15:08 /usr/bin/qemu-system-x86_64
-rwxr-xr-x 1 root root 2055128 2011-05-06 15:08 /usr/bin/qemu-x86_64

Done.

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

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

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

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

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

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

[Qemu-devel] [PATCH 3/7] target-arm: Signal InvalidOp for Neon GE and GT compares of QNaN

[Peter Maydell]

If the input to a Neon float comparison is a quiet NaN, the ARM ARM
specifies that we should raise InvalidOp if the comparison is GE or GT
but not for EQ. (Signaling NaNs raise InvalidOp regardless). This means
only EQ should use the _quiet version of the comparison function.

We implement this by cleaning up the comparison helpers to call the
appopriate versions of the softfloat simple comparison functions
(float32_le and friends) rather than the generic float32_compare functions.
This makes them simple enough that they are clearer opencoded rather
than macroised.

Signed-off-by: Peter Maydell 
---
 target-arm/neon_helper.c |   40 ++--
 1 files changed, 18 insertions(+), 22 deletions(-)

diff --git a/target-arm/neon_helper.c b/target-arm/neon_helper.c
index f5b173a..9165519 100644
--- a/target-arm/neon_helper.c
+++ b/target-arm/neon_helper.c
@@ -1802,41 +1802,37 @@ uint32_t HELPER(neon_mul_f32)(uint32_t a, uint32_t b)
 return float32_val(float32_mul(make_float32(a), make_float32(b), NFS));
 }
 
-/* Floating point comparisons produce an integer result.  */
-#define NEON_VOP_FCMP(name, ok) \
-uint32_t HELPER(neon_##name)(uint32_t a, uint32_t b) \
-{ \
-switch (float32_compare_quiet(make_float32(a), make_float32(b), NFS)) { \
-ok return ~0; \
-default: return 0; \
-} \
+/* Floating point comparisons produce an integer result.
+ * Note that EQ doesn't signal InvalidOp for QNaNs but GE and GT do.
+ * Softfloat routines return 0/1, which we convert to the 0/-1 Neon requires.
+ */
+uint32_t HELPER(neon_ceq_f32)(uint32_t a, uint32_t b)
+{
+return -float32_eq_quiet(make_float32(a), make_float32(b), NFS);
+}
+
+uint32_t HELPER(neon_cge_f32)(uint32_t a, uint32_t b)
+{
+return -float32_le(make_float32(b), make_float32(a), NFS);
 }
 
-NEON_VOP_FCMP(ceq_f32, case float_relation_equal:)
-NEON_VOP_FCMP(cge_f32, case float_relation_equal: case float_relation_greater:)
-NEON_VOP_FCMP(cgt_f32, case float_relation_greater:)
+uint32_t HELPER(neon_cgt_f32)(uint32_t a, uint32_t b)
+{
+return -float32_lt(make_float32(b), make_float32(a), NFS);
+}
 
 uint32_t HELPER(neon_acge_f32)(uint32_t a, uint32_t b)
 {
 float32 f0 = float32_abs(make_float32(a));
 float32 f1 = float32_abs(make_float32(b));
-switch (float32_compare_quiet(f0, f1, NFS)) {
-case float_relation_equal:
-case float_relation_greater:
-return ~0;
-default:
-return 0;
-}
+return -float32_le(f1, f0, NFS);
 }
 
 uint32_t HELPER(neon_acgt_f32)(uint32_t a, uint32_t b)
 {
 float32 f0 = float32_abs(make_float32(a));
 float32 f1 = float32_abs(make_float32(b));
-if (float32_compare_quiet(f0, f1, NFS) == float_relation_greater) {
-return ~0;
-}
-return 0;
+return -float32_lt(f1, f0, NFS);
 }
 
 #define ELEM(V, N, SIZE) (((V) >> ((N) * (SIZE))) & ((1ull << (SIZE)) - 1))
-- 
1.7.1

Re: [Qemu-devel] Allow ARMv7M to be started without a kernel

[Alexander Graf]

On 06.05.2011, at 01:50, Rob Landley wrote:

> On 05/05/2011 06:26 PM, Alexander Graf wrote:
>>> As an aside: I think QEMU should have an option which is "just load
>>> a plain ELF or raw binary, with no funny Linux-kernel-specific 
>>> behaviour" rather than overloading -kernel to mean "if it's a raw
>>> image it's Linux and if it's an ELF file it's not".
>> 
>> Traditionally, -bios has been that one. -kernel is more of a real
>> bootloader replacement, including all the weirdness a bootloader does
>> :).
> 
> Except that neither "qemu-system-x86_64 -bios vmlinux" nor
> qemu-system-x86_64 -kernel vmlinux" will load an ELF kernel on x86-64.
> 
> The code to do this _exists_ within qemu, it's just not hooked up
> consistently on all targets.  We have a universal cross-platform image
> format, and we have support in qemu for loading that format, and for
> some reason it's only enabled on certain targets.  I've never understood
> why...

Unfortunately, booting isn't that easy :). The kernel needs some more data than 
just itself and an entry point to properly load. On some architectures, it 
requires a device tree (and pointer to it in a register), it might require page 
tables / TLB entries to be set up, if you want -initrd and -append to work you 
also need to tell the kernel where to find those.

So while ELF is a nice container for binaries, there are still some pieces 
missing to actually make it a kernel loader. But that's what multiboot was 
invented for. I haven't seen too much effort going on around multiboot on 
non-x86, but it basically fills exactly the gap you're wondering about. It uses 
ELF and adds a simple boot protocol for all the other fancy stuff a kernel 
needs.


Alex

Re: [Qemu-devel] virtio-scsi spec, first public draft

[Paolo Bonzini]

On 05/06/2011 02:31 PM, Stefan Hajnoczi wrote:

Okay, this explains how you plan to handle targets appearing - you
want to set a maximum number of targets.  I was wondering how we would
add virtqueues dynamically (and why the control vqs are placed last at
n,n+1 instead of 0,1).


You don't, it's not in the spec.  On the other hand, I don't think a 
limit on the number of targets is imposing, and the limit that virtio 
places is more theoretical than practical.


(Control virtqueues are last simply to avoid +2 and -2 all over the place).


I'm really not sure I understand the win of creating lots of
virtqueues.  I just want a pipe out onto the SCSI bus so I can talk to
all devices in the SCSI domain.  Creating separate virtqueues
increases complexity in the driver and emulation IMO.


In the driver, probably.  Emulation shouldn't change much, there's so 
little to do in the end in a PV HBA emulation if you have a proper SCSI 
subsystem and the protocol is a simple transport or reasonably close.



What is the MSI-X win you mentioned?  I guess if an application on
vcpu0 is accessing target0 a lot then interrupt handling can be
handled on vcpu0 while other vcpus handle interrupts for other SCSI
targets?


Yes, possibly.  But I think the main benefit is in resiliency.  If one 
target malfunctions and timeouts, other targets still work normally 
until the SCSI layer decides to reset that target.



I remember VMware pv scsi has a trick here, each request can
contain the vcpu number which influences interrupt routing somehow.


I don't think it works under Linux though, it depends on how the OS sets 
up the APICs.


Paolo

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

[Boris Derzhavets]

Sorry, for typo in package name ( qemu-kvm) . I've upgraded version in
debian/changelog.

root@boris-System-P5Q3:~/QEMU14# ls -l *.deb |grep -v "0ubuntu4_"

-rw-r--r-- 1 root root   19494 2011-05-06 16:45 
kvm_84+dfsg-0ubuntu16+0.14.0+noroms+0ubuntu5_amd64.deb
-rw-r--r-- 1 root root   18958 2011-05-06 16:45 
qemu_0.14.0+noroms-0ubuntu5_amd64.deb
-rw-r--r-- 1 root root   37758 2011-05-06 16:41 
qemu-common_0.14.0+noroms-0ubuntu5_all.deb
-rw-r--r-- 1 root root 3512986 2011-05-06 16:45 
qemu-kvm_0.14.0+noroms-0ubuntu5_amd64.deb


root@boris-System-P5Q3:~/QEMU14# dpkg -i 
./qemu-kvm_0.14.0+noroms-0ubuntu5_amd64.deb \
> qemu_0.14.0+noroms-0ubuntu5_amd64.deb \
> qemu-common_0.14.0+noroms-0ubuntu5_all.deb

(Reading database ... 141175 files and directories currently installed.)
Preparing to replace qemu-kvm 0.14.0+noroms-0ubuntu4 (using 
.../qemu-kvm_0.14.0+noroms-0ubuntu5_amd64.deb) ...
Unpacking replacement qemu-kvm ...
Preparing to replace qemu 0.14.0+noroms-0ubuntu4 (using 
qemu_0.14.0+noroms-0ubuntu5_amd64.deb) ...
Unpacking replacement qemu ...
Preparing to replace qemu-common 0.14.0+noroms-0ubuntu4 (using 
qemu-common_0.14.0+noroms-0ubuntu5_all.deb) ...
Unpacking replacement qemu-common ...
Setting up qemu-common (0.14.0+noroms-0ubuntu5) ...
Setting up qemu-kvm (0.14.0+noroms-0ubuntu5) ...
Processing triggers for ureadahead ...
ureadahead will be reprofiled on next reboot
Processing triggers for man-db ...
Setting up qemu (0.14.0+noroms-0ubuntu5) ...

# dpkg -l | grep qemu
ii  qemu  0.14.0+noroms-0ubuntu5
 dummy transitional package from qemu to qemu-kvm
ii  qemu-common 0.14.0+noroms-0ubuntu5 qemu 
common functionality (bios, documentation, etc)
ii  qemu-kvm 0.14.0+noroms-0ubuntu5 
Full virtualization on i386 and amd64 har

root@boris-System-P5Q3:~/QEMU14# ls -l /usr/bin/qemu*

-rwxr-xr-x 1 root root 3520568 2011-05-06 16:45 /usr/bin/qemu
-rwxr-xr-x 1 root root 2014976 2011-05-06 16:45 /usr/bin/qemu-i386
-rwxr-xr-x 1 root root 270 2011-05-06 14:04 /usr/bin/qemu-ifdown
-rwxr-xr-x 1 root root 138 2011-02-22 16:34 /usr/bin/qemu-ifup
-rwxr-xr-x 1 root root  350640 2011-05-06 16:45 /usr/bin/qemu-img
-rwxr-xr-x 1 root root  362928 2011-05-06 16:45 /usr/bin/qemu-io
-rwxr-xr-x 1 root root  338384 2011-05-06 16:45 /usr/bin/qemu-nbd
lrwxrwxrwx 1 root root   4 2011-05-06 16:48 /usr/bin/qemu-system-i386 -> 
qemu
-rwxr-xr-x 1 root root 3590968 2011-05-06 16:45 /usr/bin/qemu-system-x86_64
-rwxr-xr-x 1 root root 2055128 2011-05-06 16:45 /usr/bin/qemu-x86_64

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

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

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

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

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

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

[Qemu-devel] [Bug 458521] Re: kvm crash when using virtio for network, hardy guest

[Jamie Strandboge]

** Changed in: linux (Ubuntu Hardy)
   Status: In Progress => Fix Released

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

Title:
  kvm crash when using virtio for network, hardy guest

Status in QEMU:
  Fix Released
Status in Release Notes for Ubuntu:
  Fix Released
Status in “linux” package in Ubuntu:
  Fix Released
Status in “qemu-kvm” package in Ubuntu:
  Fix Released
Status in “linux” source package in Hardy:
  Fix Released
Status in “qemu-kvm” source package in Hardy:
  Invalid
Status in “linux” source package in Karmic:
  Fix Released
Status in “qemu-kvm” source package in Karmic:
  Fix Released

Bug description:
  Binary package hint: kvm

  I was running a kvm host with 5 virtual machines using jaunty. 1
  running centos 5.4, 3 running karmic and 1 running hardy.

  I upgraded the host to karmic to test it and one of my virtual
  machines(the vm with hardy) crashs after some seconds of boot! I stack
  traced the kvm process and found a "virtio-net truncating packet"
  before process crash.

  No error in any log file. The VM just crash.

  I changed the model of network interface(the disk is still virtio) in
  VM.xml to rtl8139, redefined and started. The problem goes away.

  I don't know if it's a kvm bug or libvirt(regarding the bridge
  network) bug but definitely is a bug. All other VMs are running OK
  with virtio for both network and disk.

  ===
  Karmic Release Notes:

  KVM Guest Crashes when Guest is Hardy and using Virtio Networking

  Ubuntu 8.04 LTS (Hardy) KVM guests using virtio networking may crash,
  when running on top of Ubuntu 9.10 (Karmic) hosts.

  As a workaround, such guests should use either e1000 or rtl839 as the
  networking model.  A fix for the bug is currently in progress and
  should be addressed in an update to the qemu-kvm package in Karmic.

  ===

  ===
  SRU Justification

  This bug is a regression from the kvm-84 package in 9.04.  Karmic users 
hosting 8.04 KVM guests and using virtio networking *will* crash their VM when 
the network connection is saturated.  As the virtual machine crashes without 
sync buffers, there could very well be data lost in the guest filesystem or 
memory.
  I have posted the patch for review, and acked upstream:
   * http://lists.gnu.org/archive/html/qemu-devel/2009-10/msg02495.html
  I am bundling this fix with two other minor fixes, for Bug #452323  and Bug 
#453441.

  TEST CASE:
   1) Download and uncompress
* http://rookery.canonical.com/~kirkland/hardy.img.bz2
   2) Download a bridged networking script
* http://rookery.canonical.com/~kirkland/bridge.sh
   3) Start the vm with virtio and bridged networking
* sudo /usr/bin/kvm -m 512 -net nic,model=virtio -net tap,script=bridge.sh 
-drive file=hardy.img,if=virtio,index=0,boot=on
   4) The VM's username and password are both "ubuntu".
   5) In the guest, have nc listen on a port
* nc -lp 1234 > /dev/null
   6) On your host, flood the guest with network traffic on that port
* cat /dev/urandom | nc -w 3 guest_ip 1234
   7) Without this fix, the guest will crash immediately, saying:
*   virtio-net truncating packet
   8) With the fix, you will be able to send the guest with data over the 
virtio network ad nauseum.  I get about 6MB/s throughput.

  ===

[Qemu-devel] [PATCH 7/7] target-arm: Signal Underflow when denormal flushed to zero on output

[Peter Maydell]

On ARM the architecture mandates that when an output denormal is flushed to
zero we must set the FPSCR UFC (underflow) bit, so map softfloat's
float_flag_output_denormal accordingly.

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

diff --git a/target-arm/helper.c b/target-arm/helper.c
index de00468..149fc82 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -2355,7 +2355,7 @@ static inline int vfp_exceptbits_from_host(int host_bits)
 target_bits |= 2;
 if (host_bits & float_flag_overflow)
 target_bits |= 4;
-if (host_bits & float_flag_underflow)
+if (host_bits & (float_flag_underflow | float_flag_output_denormal))
 target_bits |= 8;
 if (host_bits & float_flag_inexact)
 target_bits |= 0x10;
-- 
1.7.1

[Qemu-devel] [PATCH 1/7] target-arm: Don't set FP exceptions in recip, recip_sqrt estimate fns

[Peter Maydell]

The functions which do the core estimation algorithms for the VRSQRTE
and VRECPE instructions should not set floating point exception flags,
so use a local fp status for doing these calculations.

Signed-off-by: Peter Maydell 
---
 target-arm/helper.c |   12 ++--
 1 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index 62ae72e..5ff6a9b 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -2749,7 +2749,11 @@ float32 HELPER(rsqrts_f32)(float32 a, float32 b, 
CPUState *env)
  */
 static float64 recip_estimate(float64 a, CPUState *env)
 {
-float_status *s = &env->vfp.standard_fp_status;
+/* These calculations mustn't set any fp exception flags,
+ * so we use a local copy of the fp_status.
+ */
+float_status dummy_status = env->vfp.standard_fp_status;
+float_status *s = &dummy_status;
 /* q = (int)(a * 512.0) */
 float64 q = float64_mul(float64_512, a, s);
 int64_t q_int = float64_to_int64_round_to_zero(q, s);
@@ -2812,7 +2816,11 @@ float32 HELPER(recpe_f32)(float32 a, CPUState *env)
  */
 static float64 recip_sqrt_estimate(float64 a, CPUState *env)
 {
-float_status *s = &env->vfp.standard_fp_status;
+/* These calculations mustn't set any fp exception flags,
+ * so we use a local copy of the fp_status.
+ */
+float_status dummy_status = env->vfp.standard_fp_status;
+float_status *s = &dummy_status;
 float64 q;
 int64_t q_int;
 
-- 
1.7.1

[Qemu-devel] [PATCH 2/7] target-arm: Signal InputDenormal for VRECPE, VRSQRTE, VRECPS, VRSQRTS

[Peter Maydell]

The helpers for VRECPE.F32, VSQRTE.F32, VRECPS and VRSQRTS handle denormals
as special cases, so we must set the InputDenormal exception flag ourselves.

Signed-off-by: Peter Maydell 
---
 target-arm/helper.c |   12 
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index 5ff6a9b..f072527 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -2720,6 +2720,9 @@ float32 HELPER(recps_f32)(float32 a, float32 b, CPUState 
*env)
 float_status *s = &env->vfp.standard_fp_status;
 if ((float32_is_infinity(a) && float32_is_zero_or_denormal(b)) ||
 (float32_is_infinity(b) && float32_is_zero_or_denormal(a))) {
+if (!(float32_is_zero(a) || float32_is_zero(b))) {
+float_raise(float_flag_input_denormal, s);
+}
 return float32_two;
 }
 return float32_sub(float32_two, float32_mul(a, b, s), s);
@@ -2731,6 +2734,9 @@ float32 HELPER(rsqrts_f32)(float32 a, float32 b, CPUState 
*env)
 float32 product;
 if ((float32_is_infinity(a) && float32_is_zero_or_denormal(b)) ||
 (float32_is_infinity(b) && float32_is_zero_or_denormal(a))) {
+if (!(float32_is_zero(a) || float32_is_zero(b))) {
+float_raise(float_flag_input_denormal, s);
+}
 return float32_one_point_five;
 }
 product = float32_mul(a, b, s);
@@ -2791,6 +2797,9 @@ float32 HELPER(recpe_f32)(float32 a, CPUState *env)
 } else if (float32_is_infinity(a)) {
 return float32_set_sign(float32_zero, float32_is_neg(a));
 } else if (float32_is_zero_or_denormal(a)) {
+if (!float32_is_zero(a)) {
+float_raise(float_flag_input_denormal, s);
+}
 float_raise(float_flag_divbyzero, s);
 return float32_set_sign(float32_infinity, float32_is_neg(a));
 } else if (a_exp >= 253) {
@@ -2882,6 +2891,9 @@ float32 HELPER(rsqrte_f32)(float32 a, CPUState *env)
 }
 return float32_default_nan;
 } else if (float32_is_zero_or_denormal(a)) {
+if (!float32_is_zero(a)) {
+float_raise(float_flag_input_denormal, s);
+}
 float_raise(float_flag_divbyzero, s);
 return float32_set_sign(float32_infinity, float32_is_neg(a));
 } else if (float32_is_neg(a)) {
-- 
1.7.1

[Qemu-devel] [PATCH 5/7] target-arm: Add separate Neon float-int conversion helpers

[Peter Maydell]

Add the Neon-specific float-int conversion helper functions which
use the standard FPSCR value rather than the VFP FPSCR.

Signed-off-by: Peter Maydell 
---
 target-arm/helper.h|   10 ++
 target-arm/op_helper.c |   12 
 target-arm/translate.c |   29 +
 3 files changed, 39 insertions(+), 12 deletions(-)

diff --git a/target-arm/helper.h b/target-arm/helper.h
index 2c54d5e..1b4005a 100644
--- a/target-arm/helper.h
+++ b/target-arm/helper.h
@@ -127,6 +127,16 @@ DEF_HELPER_2(vfp_sltod, f64, i64, i32)
 DEF_HELPER_2(vfp_uhtod, f64, i64, i32)
 DEF_HELPER_2(vfp_ultod, f64, i64, i32)
 
+DEF_HELPER_1(neon_sitos, f32, i32)
+DEF_HELPER_1(neon_uitos, f32, i32)
+DEF_HELPER_1(neon_tosizs, i32, f32)
+DEF_HELPER_1(neon_touizs, i32, f32)
+
+DEF_HELPER_2(neon_ultos, f32, i32, i32);
+DEF_HELPER_2(neon_sltos, f32, i32, i32);
+DEF_HELPER_2(neon_touls, i32, f32, i32);
+DEF_HELPER_2(neon_tosls, i32, f32, i32);
+
 DEF_HELPER_2(vfp_fcvt_f16_to_f32, f32, i32, env)
 DEF_HELPER_2(vfp_fcvt_f32_to_f16, i32, f32, env)
 DEF_HELPER_2(neon_fcvt_f16_to_f32, f32, i32, env)
diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index 1afea43..3998d9c 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -448,14 +448,23 @@ CONV_ITOF(vfp_##name##to##p, fsz, sign, 
&env->vfp.fp_status) \
 CONV_FTOI(vfp_##to##name##p, fsz, sign, &env->vfp.fp_status, ) \
 CONV_FTOI(vfp_##to##name##z##p, fsz, sign, &env->vfp.fp_status, _round_to_zero)
 
+#define NEON_CONVS(name, p, fsz, sign) \
+CONV_ITOF(neon_##name##to##p, fsz, sign, &env->vfp.standard_fp_status) \
+CONV_FTOI(neon_##to##name##z##p, fsz, sign, &env->vfp.standard_fp_status, \
+  _round_to_zero)
+
 VFP_CONVS(si, s, 32, )
 VFP_CONVS(si, d, 64, )
 VFP_CONVS(ui, s, 32, u)
 VFP_CONVS(ui, d, 64, u)
 
+NEON_CONVS(si, s, 32, )
+NEON_CONVS(ui, s, 32, u)
+
 #undef CONV_ITOF
 #undef CONV_FTOI
 #undef VFP_CONVS
+#undef NEON_CONVS
 
 /* VFP3 fixed point conversion.  */
 #define VFP_CONV_FIX(pfx, name, p, fsz, itype, sign, status) \
@@ -485,4 +494,7 @@ VFP_CONV_FIX(vfp_, sl, s, 32, int32, , &env->vfp.fp_status)
 VFP_CONV_FIX(vfp_, uh, s, 32, uint16, u, &env->vfp.fp_status)
 VFP_CONV_FIX(vfp_, ul, s, 32, uint32, u, &env->vfp.fp_status)
 
+VFP_CONV_FIX(neon_, sl, s, 32, int32, , &env->vfp.standard_fp_status)
+VFP_CONV_FIX(neon_, ul, s, 32, uint32, u, &env->vfp.standard_fp_status)
+
 #undef VFP_CONV_FIX
diff --git a/target-arm/translate.c b/target-arm/translate.c
index 195cf30..10592a5 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -5220,6 +5220,7 @@ static int disas_neon_data_insn(CPUState * env, 
DisasContext *s, uint32_t insn)
 }
 } else if (op >= 14) {
 /* VCVT fixed-point.  */
+TCGv tmp_shift;
 if (!(insn & (1 << 21)) || (q && ((rd | rm) & 1))) {
 return 1;
 }
@@ -5227,21 +5228,25 @@ static int disas_neon_data_insn(CPUState * env, 
DisasContext *s, uint32_t insn)
  * hence this 32-shift where the ARM ARM has 64-imm6.
  */
 shift = 32 - shift;
+tmp_shift = tcg_const_i32(shift);
 for (pass = 0; pass < (q ? 4 : 2); pass++) {
 tcg_gen_ld_f32(cpu_F0s, cpu_env, neon_reg_offset(rm, 
pass));
 if (!(op & 1)) {
-if (u)
-gen_vfp_ulto(0, shift);
-else
-gen_vfp_slto(0, shift);
+if (u) {
+gen_helper_neon_ultos(cpu_F0s, cpu_F0s, tmp_shift);
+} else {
+gen_helper_neon_sltos(cpu_F0s, cpu_F0s, tmp_shift);
+}
 } else {
-if (u)
-gen_vfp_toul(0, shift);
-else
-gen_vfp_tosl(0, shift);
+if (u) {
+gen_helper_neon_touls(cpu_F0s, cpu_F0s, tmp_shift);
+} else {
+gen_helper_neon_tosls(cpu_F0s, cpu_F0s, tmp_shift);
+}
 }
 tcg_gen_st_f32(cpu_F0s, cpu_env, neon_reg_offset(rd, 
pass));
 }
+tcg_temp_free_i32(tmp_shift);
 } else {
 return 1;
 }
@@ -6051,16 +6056,16 @@ static int disas_neon_data_insn(CPUState * env, 
DisasContext *s, uint32_t insn)
 gen_helper_rsqrte_f32(cpu_F0s, cpu_F0s, cpu_env);
 break;
 case NEON_2RM_VCVT_FS: /* VCVT.F32.S32 */
-gen_vfp_sito(0);
+gen_helper_neon_sitos(cpu_F0s, cpu_F0s);
 break;
 case NEON_2RM_VCVT_FU: /* VCVT.F32

[Qemu-devel] [PATCH 0/7] target-arm: Fix bugs in fp exception flag setting

[Peter Maydell]

This patch series fixes a number of minor bugs in the ARM target where
we were not correctly setting the cumulative exception flags in the
FPSCR. It includes adding a new flag to softfloat indicating when a
denormal result has been flushed to zero (as discussed previously on
the list.)

Tested with the usual random instruction sequence testing (covering
all the neon and vfp data processing instructions which can set FPSCR
exception flags). These patches fix all the FPSCR flags bugs I found,
with the exception of those in the VCVT float-int and float32-float16
conversion routines, which are a bit trickier to fix because they are
bugs in softfloat rather than merely in the arm helper functions.


Peter Maydell (7):
  target-arm: Don't set FP exceptions in recip, recip_sqrt estimate fns
  target-arm: Signal InputDenormal for VRECPE, VRSQRTE, VRECPS, VRSQRTS
  target-arm: Signal InvalidOp for Neon GE and GT compares of QNaN
  target-arm: Refactor int-float conversions
  target-arm: Add separate Neon float-int conversion helpers
  softfloat: Add new flag for when denormal result is flushed to zero
  target-arm: Signal Underflow when denormal flushed to zero on output

 fpu/softfloat.c  |   41 ++--
 fpu/softfloat.h  |3 +-
 target-arm/helper.c  |  151 +++---
 target-arm/helper.h  |   70 -
 target-arm/neon_helper.c |   40 ++---
 target-arm/op_helper.c   |   74 ++
 target-arm/translate.c   |   92 
 7 files changed, 243 insertions(+), 228 deletions(-)

[Qemu-devel] [PATCH 4/7] target-arm: Refactor int-float conversions

[Peter Maydell]

The Neon versions of int-float conversions need their own helper routines
because they must use the "standard FPSCR" rather than the default one.
Refactor the helper functions to make it easy to add the neon versions.
While we're touching the code, move the helpers to op_helper.c so that
we can use the global env variable rather than passing it as a parameter.

Signed-off-by: Peter Maydell 
---
 target-arm/helper.c|  125 
 target-arm/helper.h|   60 +++---
 target-arm/op_helper.c |   62 
 target-arm/translate.c |   63 +---
 4 files changed, 127 insertions(+), 183 deletions(-)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index f072527..de00468 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -2526,100 +2526,6 @@ DO_VFP_cmp(s, float32)
 DO_VFP_cmp(d, float64)
 #undef DO_VFP_cmp
 
-/* Integer to float conversion.  */
-float32 VFP_HELPER(uito, s)(uint32_t x, CPUState *env)
-{
-return uint32_to_float32(x, &env->vfp.fp_status);
-}
-
-float64 VFP_HELPER(uito, d)(uint32_t x, CPUState *env)
-{
-return uint32_to_float64(x, &env->vfp.fp_status);
-}
-
-float32 VFP_HELPER(sito, s)(uint32_t x, CPUState *env)
-{
-return int32_to_float32(x, &env->vfp.fp_status);
-}
-
-float64 VFP_HELPER(sito, d)(uint32_t x, CPUState *env)
-{
-return int32_to_float64(x, &env->vfp.fp_status);
-}
-
-/* Float to integer conversion.  */
-uint32_t VFP_HELPER(toui, s)(float32 x, CPUState *env)
-{
-if (float32_is_any_nan(x)) {
-float_raise(float_flag_invalid, &env->vfp.fp_status);
-return 0;
-}
-return float32_to_uint32(x, &env->vfp.fp_status);
-}
-
-uint32_t VFP_HELPER(toui, d)(float64 x, CPUState *env)
-{
-if (float64_is_any_nan(x)) {
-float_raise(float_flag_invalid, &env->vfp.fp_status);
-return 0;
-}
-return float64_to_uint32(x, &env->vfp.fp_status);
-}
-
-uint32_t VFP_HELPER(tosi, s)(float32 x, CPUState *env)
-{
-if (float32_is_any_nan(x)) {
-float_raise(float_flag_invalid, &env->vfp.fp_status);
-return 0;
-}
-return float32_to_int32(x, &env->vfp.fp_status);
-}
-
-uint32_t VFP_HELPER(tosi, d)(float64 x, CPUState *env)
-{
-if (float64_is_any_nan(x)) {
-float_raise(float_flag_invalid, &env->vfp.fp_status);
-return 0;
-}
-return float64_to_int32(x, &env->vfp.fp_status);
-}
-
-uint32_t VFP_HELPER(touiz, s)(float32 x, CPUState *env)
-{
-if (float32_is_any_nan(x)) {
-float_raise(float_flag_invalid, &env->vfp.fp_status);
-return 0;
-}
-return float32_to_uint32_round_to_zero(x, &env->vfp.fp_status);
-}
-
-uint32_t VFP_HELPER(touiz, d)(float64 x, CPUState *env)
-{
-if (float64_is_any_nan(x)) {
-float_raise(float_flag_invalid, &env->vfp.fp_status);
-return 0;
-}
-return float64_to_uint32_round_to_zero(x, &env->vfp.fp_status);
-}
-
-uint32_t VFP_HELPER(tosiz, s)(float32 x, CPUState *env)
-{
-if (float32_is_any_nan(x)) {
-float_raise(float_flag_invalid, &env->vfp.fp_status);
-return 0;
-}
-return float32_to_int32_round_to_zero(x, &env->vfp.fp_status);
-}
-
-uint32_t VFP_HELPER(tosiz, d)(float64 x, CPUState *env)
-{
-if (float64_is_any_nan(x)) {
-float_raise(float_flag_invalid, &env->vfp.fp_status);
-return 0;
-}
-return float64_to_int32_round_to_zero(x, &env->vfp.fp_status);
-}
-
 /* floating point conversion */
 float64 VFP_HELPER(fcvtd, s)(float32 x, CPUState *env)
 {
@@ -2639,37 +2545,6 @@ float32 VFP_HELPER(fcvts, d)(float64 x, CPUState *env)
 return float32_maybe_silence_nan(r);
 }
 
-/* VFP3 fixed point conversion.  */
-#define VFP_CONV_FIX(name, p, fsz, itype, sign) \
-float##fsz VFP_HELPER(name##to, p)(uint##fsz##_t  x, uint32_t shift, \
-   CPUState *env) \
-{ \
-float##fsz tmp; \
-tmp = sign##int32_to_##float##fsz ((itype##_t)x, &env->vfp.fp_status); \
-return float##fsz##_scalbn(tmp, -(int)shift, &env->vfp.fp_status); \
-} \
-uint##fsz##_t VFP_HELPER(to##name, p)(float##fsz x, uint32_t shift, \
-  CPUState *env) \
-{ \
-float##fsz tmp; \
-if (float##fsz##_is_any_nan(x)) { \
-float_raise(float_flag_invalid, &env->vfp.fp_status); \
-return 0; \
-} \
-tmp = float##fsz##_scalbn(x, shift, &env->vfp.fp_status); \
-return float##fsz##_to_##itype##_round_to_zero(tmp, &env->vfp.fp_status); \
-}
-
-VFP_CONV_FIX(sh, d, 64, int16, )
-VFP_CONV_FIX(sl, d, 64, int32, )
-VFP_CONV_FIX(uh, d, 64, uint16, u)
-VFP_CONV_FIX(ul, d, 64, uint32, u)
-VFP_CONV_FIX(sh, s, 32, int16, )
-VFP_CONV_FIX(sl, s, 32, int32, )
-VFP_CONV_FIX(uh, s, 32, uint16, u)
-VFP_CONV_FIX(ul, s, 32, uint32, u)
-#undef VFP_CONV_FIX
-
 /* Half precision conversions.  */
 static float32 do_fcvt_f16_to_f32(uint32_t a, CPUState *env, float_status *s)
 {
diff --git a/target-arm/helper.h b/target-arm/helper.

[Qemu-devel] [PATCH 6/7] softfloat: Add new flag for when denormal result is flushed to zero

[Peter Maydell]

Add a new float_flag_output_denormal which is set when the result
of a floating point operation would be denormal but is flushed to
zero because we are in flush_to_zero mode. This is necessary because
some architectures signal this condition as an underflow and others
signal it as an inexact result.

Signed-off-by: Peter Maydell 
---
 fpu/softfloat.c |   41 ++---
 fpu/softfloat.h |3 ++-
 2 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index baba1dc..e3cd8a7 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -341,7 +341,10 @@ static float32 roundAndPackFloat32( flag zSign, int16 
zExp, uint32_t zSig STATUS
 return packFloat32( zSign, 0xFF, - ( roundIncrement == 0 ));
 }
 if ( zExp < 0 ) {
-if ( STATUS(flush_to_zero) ) return packFloat32( zSign, 0, 0 );
+if (STATUS(flush_to_zero)) {
+float_raise(float_flag_output_denormal STATUS_VAR);
+return packFloat32(zSign, 0, 0);
+}
 isTiny =
( STATUS(float_detect_tininess) == 
float_tininess_before_rounding )
 || ( zExp < -1 )
@@ -520,7 +523,10 @@ static float64 roundAndPackFloat64( flag zSign, int16 
zExp, uint64_t zSig STATUS
 return packFloat64( zSign, 0x7FF, - ( roundIncrement == 0 ));
 }
 if ( zExp < 0 ) {
-if ( STATUS(flush_to_zero) ) return packFloat64( zSign, 0, 0 );
+if (STATUS(flush_to_zero)) {
+float_raise(float_flag_output_denormal STATUS_VAR);
+return packFloat64(zSign, 0, 0);
+}
 isTiny =
( STATUS(float_detect_tininess) == 
float_tininess_before_rounding )
 || ( zExp < -1 )
@@ -699,7 +705,10 @@ static floatx80
 goto overflow;
 }
 if ( zExp <= 0 ) {
-if ( STATUS(flush_to_zero) ) return packFloatx80( zSign, 0, 0 );
+if (STATUS(flush_to_zero)) {
+float_raise(float_flag_output_denormal STATUS_VAR);
+return packFloatx80(zSign, 0, 0);
+}
 isTiny =
( STATUS(float_detect_tininess) == 
float_tininess_before_rounding )
 || ( zExp < 0 )
@@ -1030,7 +1039,10 @@ static float128
 return packFloat128( zSign, 0x7FFF, 0, 0 );
 }
 if ( zExp < 0 ) {
-if ( STATUS(flush_to_zero) ) return packFloat128( zSign, 0, 0, 0 );
+if (STATUS(flush_to_zero)) {
+float_raise(float_flag_output_denormal STATUS_VAR);
+return packFloat128(zSign, 0, 0, 0);
+}
 isTiny =
( STATUS(float_detect_tininess) == 
float_tininess_before_rounding )
 || ( zExp < -1 )
@@ -1761,7 +1773,12 @@ static float32 addFloat32Sigs( float32 a, float32 b, 
flag zSign STATUS_PARAM)
 return a;
 }
 if ( aExp == 0 ) {
-if ( STATUS(flush_to_zero) ) return packFloat32( zSign, 0, 0 );
+if (STATUS(flush_to_zero)) {
+if (aSig | bSig) {
+float_raise(float_flag_output_denormal STATUS_VAR);
+}
+return packFloat32(zSign, 0, 0);
+}
 return packFloat32( zSign, 0, ( aSig + bSig )>>6 );
 }
 zSig = 0x4000 + aSig + bSig;
@@ -3120,7 +3137,12 @@ static float64 addFloat64Sigs( float64 a, float64 b, 
flag zSign STATUS_PARAM )
 return a;
 }
 if ( aExp == 0 ) {
-if ( STATUS(flush_to_zero) ) return packFloat64( zSign, 0, 0 );
+if (STATUS(flush_to_zero)) {
+if (aSig | bSig) {
+float_raise(float_flag_output_denormal STATUS_VAR);
+}
+return packFloat64(zSign, 0, 0);
+}
 return packFloat64( zSign, 0, ( aSig + bSig )>>9 );
 }
 zSig = LIT64( 0x4000 ) + aSig + bSig;
@@ -5282,7 +5304,12 @@ static float128 addFloat128Sigs( float128 a, float128 b, 
flag zSign STATUS_PARAM
 }
 add128( aSig0, aSig1, bSig0, bSig1, &zSig0, &zSig1 );
 if ( aExp == 0 ) {
-if ( STATUS(flush_to_zero) ) return packFloat128( zSign, 0, 0, 0 );
+if (STATUS(flush_to_zero)) {
+if (zSig0 | zSig1) {
+float_raise(float_flag_output_denormal STATUS_VAR);
+}
+return packFloat128(zSign, 0, 0, 0);
+}
 return packFloat128( zSign, 0, zSig0, zSig1 );
 }
 zSig2 = 0;
diff --git a/fpu/softfloat.h b/fpu/softfloat.h
index 5eff085..58c9b7b 100644
--- a/fpu/softfloat.h
+++ b/fpu/softfloat.h
@@ -193,7 +193,8 @@ enum {
 float_flag_overflow  =  8,
 float_flag_underflow = 16,
 float_flag_inexact   = 32,
-float_flag_input_denormal = 64
+float_flag_inpu

Re: [Qemu-devel] [PATCH 1/8] block: add bdrv_aio_stream

[Stefan Hajnoczi]

On Fri, Apr 29, 2011 at 12:56 PM, Kevin Wolf  wrote:
> Am 27.04.2011 15:27, schrieb Stefan Hajnoczi:
>> +/**
>> + * Attempt to stream an image starting from sector_num.
>> + *
>> + * @sector_num - the first sector to start streaming from
>> + * @cb - block completion callback
>> + * @opaque - data to pass completion callback
>> + *
>> + * Returns NULL if the image format not support streaming, the image is
>> + * read-only, or no image is open.
>> + *
>> + * The intention of this function is for a user to execute it once with a
>> + * sector_num of 0 and then upon receiving a completion callback, to 
>> remember
>> + * the number of sectors "streamed", and then to call this function again 
>> with
>> + * an offset adjusted by the number of sectors previously streamed.
>> + *
>> + * This allows a user to progressive stream in an image at a pace that makes
>> + * sense.  In general, this function tries to do the smallest amount of I/O
>> + * possible to do some useful work.
>> + *
>> + * This function only really makes sense in combination with a block format
>> + * that supports copy on read and has it enabled.  If copy on read is not
>> + * enabled, a block format driver may return NULL.
>> + */
>> +BlockDriverAIOCB *bdrv_aio_stream(BlockDriverState *bs, int64_t sector_num,
>> +                                  BlockDriverCompletionFunc *cb, void 
>> *opaque)
>
> I think bdrv_aio_stream is a bad name for this. It only becomes image
> streaming because the caller repeatedly calls this function. What the
> function really does is copying some data from the backing file into the
> overlay image.

That's true but bdrv_aio_copy_from_backing_file() is a bit long.  The
special thing about this operation is that it takes a starting
sector_num but no length.  The callback receives the nb_sectors.  So
this operation isn't an ordinary [start, length) copy either so
bdrv_aio_stream() isn't that bad?

> I'm not sure how the caller would know how many sectors have been
> copied. A BlockDriverCompletionFunc usually returns 0 on success, did
> you change it here to use positive numbers for something else? At least
> this must be documented somewhere, but I would suggest to add a
> nb_sectors argument instead so that the caller decides how many sectors
> to copy.

Yes, I agree that a separate nb_sectors argument would be clearer.

> If you say that it only makes sense with copy on read, should one think
> of it as a read that throws the read data away? I think considering it a
> copy function makes more sense and is independent of copy on read.

I actually think the copy-on-read statement is an implementation
detail.  I can imagine doing essentially the same behavior without
exposing copy on read to the user.  But in QED streaming is based on
copy-on-read.  Let's remove this comment.

Stefan

Re: [Qemu-devel] [PATCH 2/8] qmp: Add QMP support for stream commands

[Stefan Hajnoczi]

On Fri, Apr 29, 2011 at 1:09 PM, Kevin Wolf  wrote:
> Am 27.04.2011 15:27, schrieb Stefan Hajnoczi:
>> From: Anthony Liguori 
>>
>> For leaf images with copy on read semantics, the stream commands allow the 
>> user
>> to populate local blocks by manually streaming them from the backing image.
>> Once all blocks have been streamed, the dependency on the original backing
>> image can be removed.  Therefore, stream commands can be used to implement
>> post-copy live block migration and rapid deployment.
>>
>> The stream command can be used to stream a single sector, to start streaming
>> the entire device, and to cancel an active stream.  It is easiest to allow 
>> the
>> stream command to manage streaming for the entire device but a managent tool
>> could use single sector mode to throttle the I/O rate.  When a single sector 
>> is
>> streamed, the command returns an offset that can be used for a subsequent 
>> call.
>
> You mean literally single sectors? You're not interested in completing
> the job in finite time, are you? ;-)
>
> I would suggest adding a length argument for the all=false case, so that
> management tools can choose more reasonable sizes.

Discussion on libvir-list suggests the same thing.  Let's take a
nb_sectors where 0=all.

Stefan

Re: [Qemu-devel] [PATCH 3/8] qed: add support for Copy-on-Read

[Stefan Hajnoczi]

On Fri, Apr 29, 2011 at 1:14 PM, Kevin Wolf  wrote:
> Am 27.04.2011 15:27, schrieb Stefan Hajnoczi:
>> From: Anthony Liguori 
>>
>> When creating an image using qemu-img, just pass '-o copy_on_read' and then
>> whenever QED reads from a backing file, it will write the block to the QED
>> file after the read completes ensuring that you only fetch from the backing
>> device once.
>
> As discussed previously, this is not the right primary interface.
>
> Also, the patch still seems to be broken with respect to concurrent writes.

Both good points, I really wanted to give a full RFC patch series to
show all the code that has been written today.  But your points need
to be addressed for the next version.

Stefan

Re: [Qemu-devel] [PATCH 1/8] block: add bdrv_aio_stream

[Kevin Wolf]

Am 06.05.2011 15:21, schrieb Stefan Hajnoczi:
> On Fri, Apr 29, 2011 at 12:56 PM, Kevin Wolf  wrote:
>> Am 27.04.2011 15:27, schrieb Stefan Hajnoczi:
>>> +/**
>>> + * Attempt to stream an image starting from sector_num.
>>> + *
>>> + * @sector_num - the first sector to start streaming from
>>> + * @cb - block completion callback
>>> + * @opaque - data to pass completion callback
>>> + *
>>> + * Returns NULL if the image format not support streaming, the image is
>>> + * read-only, or no image is open.
>>> + *
>>> + * The intention of this function is for a user to execute it once with a
>>> + * sector_num of 0 and then upon receiving a completion callback, to 
>>> remember
>>> + * the number of sectors "streamed", and then to call this function again 
>>> with
>>> + * an offset adjusted by the number of sectors previously streamed.
>>> + *
>>> + * This allows a user to progressive stream in an image at a pace that 
>>> makes
>>> + * sense.  In general, this function tries to do the smallest amount of I/O
>>> + * possible to do some useful work.
>>> + *
>>> + * This function only really makes sense in combination with a block format
>>> + * that supports copy on read and has it enabled.  If copy on read is not
>>> + * enabled, a block format driver may return NULL.
>>> + */
>>> +BlockDriverAIOCB *bdrv_aio_stream(BlockDriverState *bs, int64_t sector_num,
>>> +  BlockDriverCompletionFunc *cb, void 
>>> *opaque)
>>
>> I think bdrv_aio_stream is a bad name for this. It only becomes image
>> streaming because the caller repeatedly calls this function. What the
>> function really does is copying some data from the backing file into the
>> overlay image.
> 
> That's true but bdrv_aio_copy_from_backing_file() is a bit long. 

bdrv_copy_backing() or something should be short enough and still
describes what it's really doing.

> The
> special thing about this operation is that it takes a starting
> sector_num but no length.  The callback receives the nb_sectors.  So
> this operation isn't an ordinary [start, length) copy either so
> bdrv_aio_stream() isn't that bad?

Well, you're going to introduce nb_sectors anyway, so it's not really
special any more.

> I actually think the copy-on-read statement is an implementation
> detail.  I can imagine doing essentially the same behavior without
> exposing copy on read to the user.  But in QED streaming is based on
> copy-on-read.  Let's remove this comment.

Ok. Removing the comment and calling it something with "copy" in the
name should make clear what the intention is.

Kevin

[Qemu-devel] qemu.org down

[Anthony Liguori]

The hosting company qemu.org uses is experiencing an outage this 
morning.  I'll keep the list up to date as I learn more.


Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 4/7] target-arm: Refactor int-float conversions

[Paul Brook]

> The Neon versions of int-float conversions need their own helper routines
> because they must use the "standard FPSCR" rather than the default one.
> Refactor the helper functions to make it easy to add the neon versions.
> While we're touching the code, move the helpers to op_helper.c so that
> we can use the global env variable rather than passing it as a parameter.

IMO this is going in the wrong direction.  We should in aiming for less 
implicit accesses to cpu state, not more.

Maybe better would be to explicitly pass a pointer the fp status. That way you 
don't even need separate VFP and NEON variants of these routines.

Paul

Re: [Qemu-devel] [PATCH] target-arm: Minimal implementation of performance counters

[Peter Maydell]

On 26 April 2011 11:23, Aurelien Jarno  wrote:
> On Mon, Apr 25, 2011 at 11:35:54PM +0100, Peter Maydell wrote:
>> On 25 April 2011 23:31, Aurelien Jarno  wrote:
>> > On Mon, Apr 25, 2011 at 10:59:52PM +0100, Peter Maydell wrote:
>> >> On 25 April 2011 22:09, Aurelien Jarno  wrote:
>> >> > Instead of having this complex test for all cp15 access, but only for
>> >> > catching a few access to performance registers, wouldn't it make more
>> >> > sense to have this test and an exception triggering directly in
>> >> > helper.c?
>> >>
>> >> That was what my first design did, but in discussions on IRC
>> >> with Paul Brook he basically said that you can't generate an
>> >> exception in the helper routine, you have to either generate
>> >> runtime code to do the test or throw away the TBs. Unfortunately
>> >> I forget the exact rationale, so I've cc'd Paul to remind me :-)
>> >
>> > This is something strange, plenty of targets are raising exceptions from
>> > helpers without any problem.
>>
>> You'd at minimum need to move the cp15 helper functions to a different
>> file, they're currently in helper.c which doesn't get compiled
>> with access to the global 'env' register.

> I agree, but it's something that has to be done sooner or later anyway.

I just spoke with Paul on IRC about this. In summary:
 * for a helper to cause an exception then it has (a) to make sure CPU
state (pc, condflags) is sync'd before the call to the helper and (b)
the helper has to be in a file with access to global env, because it
needs to call cpu_loop_exit()
 * (a) is a bit fragile because it's easy to forget and if TCG gets
cleverer things might break in a hard-to-track down way
 * (b) is bad because it increases the set of helper functions accessing
global env
 * and therefore helpers which throw exceptions are a bad idea

For rationale for/arguing about (b) see the comment on this patch:
 http://patchwork.ozlabs.org/patch/94384/

-- PMM

Re: [Qemu-devel] [PATCH 7/7] PPC: Qdev'ify e500 pci

[Paul Brook]

> PCIBus *ppce500_pci_init(qemu_irq pci_irqs[4], target_phys_addr_t
> registers) {
> -PPCE500PCIState *controller;
> +DeviceState *dev;
> +PCIBus *b;
> +PCIHostState *h;
> +PPCE500PCIState *s;
>  PCIDevice *d;
> -int index;
>  static int ppce500_pci_id;
> +SysBusDevice *sb;
> +
> +dev = qdev_create(NULL, "e500-pcihost");
> +sb = sysbus_from_qdev(dev);
> +h = FROM_SYSBUS(PCIHostState, sb);
> +s = DO_UPCAST(PPCE500PCIState, pci_state, h);
> +
> +b = pci_register_bus(&s->pci_state.busdev.qdev, NULL,

No.  This function should not exist.  All this should be done in 
e500_pcihost_initfn.  Please do the qdev conversion properly.

See versatilepb.c/versatile_pci.c for an example of how this is supposed to be 
done.

> +.qdev.no_user = 1,

There's no good reason for this. It indicates you did the qdev conversion 
incorrectly.

Paul

Re: [Qemu-devel] [PATCH v3 5/5] hpet 'driftfix': add code in hpet_timer() to compensate delayed callbacks and coalesced interrupts

[Marcelo Tosatti]

On Thu, May 05, 2011 at 04:07:19AM -0400, Ulrich Obergfell wrote:
> 
> Hi Marcelo,
>  
> > Other than that, shouldnt reset accounting variables to init state on
> > write to GLOBAL_ENABLE_CFG / writes to main counter?
> 
> I'd suggest to initialize/reset the driftfix-related fields in the
> 'HPETTimer' structure (including the backlog of unaccounted ticks)
> in the following situations.
> 
> 
> - When the guest o/s sets the 'CFG_ENABLE' bit (overall enable) in
>   the General Configuration Register.
> 
>   This is the place in hpet_ram_writel():
> 
> case HPET_CFG:
> ...
> if (activating_bit(old_val, new_val, HPET_CFG_ENABLE)) {
> ...
> for (i = 0; i < s->num_timers; i++) {
> if ((&s->timer[i])->cmp != ~0ULL) {
> // initialize / reset fields here
> hpet_set_timer(&s->timer[i]);
> 
> 
> - When the guest o/s sets the 'TN_ENABLE' bit (timer N interrupt
>   enable) in the Timer N Configuration and Capabilities Register.
> 
>   This is the place in hpet_ram_writel():
> 
> case HPET_TN_CFG:
> ...
> if (activating_bit(old_val, new_val, HPET_TN_ENABLE)) {
> // initialize / reset fields here
> hpet_set_timer(timer);
> 
> 
> This should cover cases such as ...
> 
> - first time initialization of HPET & timers during guest o/s boot.
>   (includes kexec and reboot)
> 
> - if a guest o/s stops and restarts the timer.
>   (for example, to change the comparator register value or
>to switch a timer between periodic mode and non-periodic mode)
> 
> 
> Regards,
> 
> Uli

Uli, looks good.

Re: [Qemu-devel] [PATCH 4/7] target-arm: Refactor int-float conversions

[Peter Maydell]

On 6 May 2011 15:09, Paul Brook  wrote:
>> The Neon versions of int-float conversions need their own helper routines
>> because they must use the "standard FPSCR" rather than the default one.
>> Refactor the helper functions to make it easy to add the neon versions.
>> While we're touching the code, move the helpers to op_helper.c so that
>> we can use the global env variable rather than passing it as a parameter.
>
> IMO this is going in the wrong direction.  We should in aiming for less
> implicit accesses to cpu state, not more.

I don't have a very strong feeling about this personally, I've just been
going in the direction suggested by past discussions eg
http://lists.gnu.org/archive/html/qemu-devel/2011-04/msg00183.html

> Maybe better would be to explicitly pass a pointer the fp status. That way you
> don't even need separate VFP and NEON variants of these routines.

If you were otherwise going to pass in a CPUState pointer then just passing
the pointer to the fp_status is probably better, yes.

-- PMM

Re: [Qemu-devel] [PATCH 0/3]: QMP: Introduce inject-nmi command

[Luiz Capitulino]

On Fri, 06 May 2011 11:08:08 +0200
Markus Armbruster  wrote:

> Blue Swirl  writes:
> 
> > On Mon, May 2, 2011 at 6:57 PM, Luiz Capitulino  
> > wrote:
> >> On Sat, 30 Apr 2011 09:33:15 +0300
> >> Blue Swirl  wrote:
> >>
> >>> On Sat, Apr 30, 2011 at 1:40 AM, Luiz Capitulino  
> >>> wrote:
> >>> > This series introduces the inject-nmi command for QMP, which sends an
> >>> > NMI to _all_ guest's CPUs.
> >>> >
> >>> > Also note that this series changes the human monitor nmi command to use
> >>> > the QMP implementation, which means that it now has a DIFFERENT 
> >>> > behavior.
> >>> > Please, check patch 3/3 for details.
> >>>
> >>> As discussed earlier, please change the QMP version for future
> >>> expandability so that instead of single command 'inject-nmi', 'inject'
> >>> takes parameter 'nmi'. HMP command 'nmi' can remain for now, but
> >>> 'inject' should be added.
> >>
> >> I'm not sure I agree with this, because we risky overloading 'inject' the
> >> same way we did with the 'change' command.
> >>
> >> What's 'inject' supposed to do in the future?
> >
> > Inject other IRQs, for example inject nmi could become an alias to
> > something like
> > inject /apic@fee0:l1int
> > which would be a shorthand for
> > raise /apic@fee0:l1int
> > lower /apic@fee0:l1int
> >
> > I think we only need a registration framework for IRQs and other signals.
> 
> Yes, we could use nicer infrastructure for modeling IRQs.  No, we
> shouldn't reject Lai's work because it doesn't get us there.  Perfect is
> the enemy of good.
> 
> Pick one:
> 
> 1. We take inject-nmi now.  Should we get a more general inject command
> like the one you envisage later, we can deprecate inject-nmi, and remove
> it after a suitable grace time.  Big deal.  We get the special problem
> solved now, without really compromising future solutions for the general
> problem.

We don't even need to drop it, we just call the new one 'inject' or something
like it and we're set (internally we could re-write 'inject-nmi' to use
'inject' in the future).

> 2. We reject inject-nmi now.  The itch Lai tries to scratch remains
> unscratched until we get a more general inject command.
> 
> 2a. Rejection "motivates" Lai to solve the general problem[*].  Or maybe
> it motivates somebody else.  We get the general problem solved sooner.
> And maybe I get a pony for my birthday, too.
> 
> 2b. The general problem remains unsolved along with the special problem.
> We get nothing.
> 
> 
> [*] He's been trying to give us NMI injection via QMP for five months,
> so what's a few months more to him.
> 

Re: [Qemu-devel] [PATCH 1/2] Add documentation for qemu_progres_print()

[Markus Armbruster]

Jes Sorensen  writes:

> On 05/06/11 12:40, Brad Hards wrote:
>> On Fri, 6 May 2011 07:39:10 PM jes.soren...@redhat.com wrote:
>>> +/*
>>> + * Add delta to current state, and print the output if the current
>>> + * state has progressed more than min_skip since the last value was
>>> + * printed. 'max' specifies the relative percentage, ie. a function
>>> + * can count for 30% of the total work, and still count from 0-100, by
>>> + * setting max to 30. If max is set to zero, the percent argument
>>> + * becomes an absolute value for current state.
>>> + */
>>>  void qemu_progress_print(float percent, int max)
>> I hate to critique anyone adding docs, but this makes no sense at all to me 
>> without reading the code. Is "percent" the amount we are adding (i.e. the 
>> delta) or the result (i.e. absolute progress)? Or does it vary according to 
>> the value of max?
>
> What you add is a delta, which is relative to the max. We can change the
> argument name of the function to be delta instead if that makes it
> easier to follow.

Here's my try:

/*
 * Report progress.
 * @percent is how much progress we made.
 * If @max is zero, @percent is how much of the job is done.
 * Else, @percent is a progress delta since the last call, as a fraction
 * of @max.  I.e. delta is @percent * @max / 100.  This is for
 * convenience, it lets you account for @max% of the total work in some
 * function, and still count @percent from 0 to 100.
 */

Document min_skip with qemu_progress_init():

/*
 * Initialize progress reporting.
 * If @enabled is false, actual reporting is suppressed.  The user can
 * still trigger a report by sending SIGUSR1.
 * Reports are also suppressed unless we've had at least @min_skip
 * percent progress since the last report.
 */

To be honest, the @max feature is a pain to document.  I'd ditch it.

For non-zero max,

qemu_progress_report(x, max)

becomes

qemu_progress_report(x * max/100)

Not much of an inconvenience, in my opinion.  None at all when max is
100, which is the case for all non-zero max arguments so far.

The only use of the absolute feature (zero max) so far is setting it to
"all done", like this:

qemu_progress_print(100, 0);

Can be done just as well with a delta >= 100, e.g.

qemu_progress_print(100);

If a need for setting other absolute progress arises, I'd consider
adding second function.

[Qemu-devel] TPM patchset

[Serge E. Hallyn]

Hi Stefan,

I was hoping to take a look at the TPM patchset.  Rumor has it you
have some updates.  Would you mind re-sending the set?

thanks,
-serge

Re: [Qemu-devel] [PATCH 4/7] target-arm: Refactor int-float conversions

[Blue Swirl]

On Fri, May 6, 2011 at 5:09 PM, Paul Brook  wrote:
>> The Neon versions of int-float conversions need their own helper routines
>> because they must use the "standard FPSCR" rather than the default one.
>> Refactor the helper functions to make it easy to add the neon versions.
>> While we're touching the code, move the helpers to op_helper.c so that
>> we can use the global env variable rather than passing it as a parameter.
>
> IMO this is going in the wrong direction.  We should in aiming for less
> implicit accesses to cpu state, not more.

Performance wise global env variable is faster and the register is
always available. Do you mean that we should aim to get rid of special
status of global env, so that if no op uses it, it could be discarded
to free a register?

> Maybe better would be to explicitly pass a pointer the fp status. That way you
> don't even need separate VFP and NEON variants of these routines.

It would be nice to have generic float functions callable directly as
TCG helper.

Re: [Qemu-devel] qemu.org down

[Anthony Liguori]

On 05/06/2011 08:40 AM, Anthony Liguori wrote:

The hosting company qemu.org uses is experiencing an outage this
morning. I'll keep the list up to date as I learn more.


The data center is in Fremont, CA which experienced a severe power 
outage this morning.  It's taken down a number of VPS providers.


Regards,

Anthony Liguori


Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 0/3]: QMP: Introduce inject-nmi command

[Blue Swirl]

On Fri, May 6, 2011 at 12:08 PM, Markus Armbruster  wrote:
> Blue Swirl  writes:
>
>> On Mon, May 2, 2011 at 6:57 PM, Luiz Capitulino  
>> wrote:
>>> On Sat, 30 Apr 2011 09:33:15 +0300
>>> Blue Swirl  wrote:
>>>
 On Sat, Apr 30, 2011 at 1:40 AM, Luiz Capitulino  
 wrote:
 > This series introduces the inject-nmi command for QMP, which sends an
 > NMI to _all_ guest's CPUs.
 >
 > Also note that this series changes the human monitor nmi command to use
 > the QMP implementation, which means that it now has a DIFFERENT behavior.
 > Please, check patch 3/3 for details.

 As discussed earlier, please change the QMP version for future
 expandability so that instead of single command 'inject-nmi', 'inject'
 takes parameter 'nmi'. HMP command 'nmi' can remain for now, but
 'inject' should be added.
>>>
>>> I'm not sure I agree with this, because we risky overloading 'inject' the
>>> same way we did with the 'change' command.
>>>
>>> What's 'inject' supposed to do in the future?
>>
>> Inject other IRQs, for example inject nmi could become an alias to
>> something like
>> inject /apic@fee0:l1int
>> which would be a shorthand for
>> raise /apic@fee0:l1int
>> lower /apic@fee0:l1int
>>
>> I think we only need a registration framework for IRQs and other signals.
>
> Yes, we could use nicer infrastructure for modeling IRQs.  No, we
> shouldn't reject Lai's work because it doesn't get us there.  Perfect is
> the enemy of good.
>
> Pick one:
>
> 1. We take inject-nmi now.  Should we get a more general inject command
> like the one you envisage later, we can deprecate inject-nmi, and remove
> it after a suitable grace time.  Big deal.  We get the special problem
> solved now, without really compromising future solutions for the general
> problem.
>
> 2. We reject inject-nmi now.  The itch Lai tries to scratch remains
> unscratched until we get a more general inject command.
>
> 2a. Rejection "motivates" Lai to solve the general problem[*].  Or maybe
> it motivates somebody else.  We get the general problem solved sooner.
> And maybe I get a pony for my birthday, too.
>
> 2b. The general problem remains unsolved along with the special problem.
> We get nothing.

2c. Don't add full generic IRQ registration and aliases just now but
handle 'inject' with only 'nmi'. That way we introduce no legacy
baggage to the syntax.

Re: [Qemu-devel] [PATCH 1/8] block: add bdrv_aio_stream

[Stefan Hajnoczi]

On Fri, May 6, 2011 at 2:36 PM, Kevin Wolf  wrote:
> Am 06.05.2011 15:21, schrieb Stefan Hajnoczi:
>> On Fri, Apr 29, 2011 at 12:56 PM, Kevin Wolf  wrote:
>>> Am 27.04.2011 15:27, schrieb Stefan Hajnoczi:
 +/**
 + * Attempt to stream an image starting from sector_num.
 + *
 + * @sector_num - the first sector to start streaming from
 + * @cb - block completion callback
 + * @opaque - data to pass completion callback
 + *
 + * Returns NULL if the image format not support streaming, the image is
 + * read-only, or no image is open.
 + *
 + * The intention of this function is for a user to execute it once with a
 + * sector_num of 0 and then upon receiving a completion callback, to 
 remember
 + * the number of sectors "streamed", and then to call this function again 
 with
 + * an offset adjusted by the number of sectors previously streamed.
 + *
 + * This allows a user to progressive stream in an image at a pace that 
 makes
 + * sense.  In general, this function tries to do the smallest amount of 
 I/O
 + * possible to do some useful work.
 + *
 + * This function only really makes sense in combination with a block 
 format
 + * that supports copy on read and has it enabled.  If copy on read is not
 + * enabled, a block format driver may return NULL.
 + */
 +BlockDriverAIOCB *bdrv_aio_stream(BlockDriverState *bs, int64_t 
 sector_num,
 +                                  BlockDriverCompletionFunc *cb, void 
 *opaque)
>>>
>>> I think bdrv_aio_stream is a bad name for this. It only becomes image
>>> streaming because the caller repeatedly calls this function. What the
>>> function really does is copying some data from the backing file into the
>>> overlay image.
>>
>> That's true but bdrv_aio_copy_from_backing_file() is a bit long.
>
> bdrv_copy_backing() or something should be short enough and still
> describes what it's really doing.
>
>> The
>> special thing about this operation is that it takes a starting
>> sector_num but no length.  The callback receives the nb_sectors.  So
>> this operation isn't an ordinary [start, length) copy either so
>> bdrv_aio_stream() isn't that bad?
>
> Well, you're going to introduce nb_sectors anyway, so it's not really
> special any more.

I guess you're right.  First I wasn't planning on passing nb_sectors
to this function since its the blockdev.c streaming loop that drives
the streaming process - we may not need nb_sectors here.  But I guess
this is like a read(2) function and the block driver can return short
reads if that is convenient due to cluster sizes or other image format
internals.  By passing in nb_sectors we avoid streaming too much at
the end.

Stefan

Re: [Qemu-devel] [PATCH 7/7] PPC: Qdev'ify e500 pci

[Paul Brook]

> > PCIBus *ppce500_pci_init(qemu_irq pci_irqs[4], target_phys_addr_t
> > registers) {
> > -PPCE500PCIState *controller;
> > +DeviceState *dev;
> > +PCIBus *b;
> > +PCIHostState *h;
> > +PPCE500PCIState *s;
> > 
> >  PCIDevice *d;
> > 
> > -int index;
> > 
> >  static int ppce500_pci_id;
> > 
> > +SysBusDevice *sb;
> > +
> > +dev = qdev_create(NULL, "e500-pcihost");
> > +sb = sysbus_from_qdev(dev);
> > +h = FROM_SYSBUS(PCIHostState, sb);
> > +s = DO_UPCAST(PPCE500PCIState, pci_state, h);
> > +
> > +b = pci_register_bus(&s->pci_state.busdev.qdev, NULL,
> 
> No.  This function should not exist.  All this should be done in
> e500_pcihost_initfn.  Please do the qdev conversion properly.

Or more precicely it should not depend on the internals of ppce500_pci.c.
In principle the only public entry point in that file should be 
device_init(...).  If used by multiple boards a simple helper function may be 
appropriate (e.g. smc91c111_init).  Note that this helper function has no ties 
to the rest of that file, and could be trivially moved into a different file 
or replaced with a macro/inline funciton.

ppce500_pci_init definitely should not be creating the PCI bus.

Paul

Re: [Qemu-devel] [PATCH 4/7] target-arm: Refactor int-float conversions

[Paul Brook]

> On Fri, May 6, 2011 at 5:09 PM, Paul Brook  wrote:
> >> The Neon versions of int-float conversions need their own helper
> >> routines because they must use the "standard FPSCR" rather than the
> >> default one. Refactor the helper functions to make it easy to add the
> >> neon versions. While we're touching the code, move the helpers to
> >> op_helper.c so that we can use the global env variable rather than
> >> passing it as a parameter.
> > 
> > IMO this is going in the wrong direction.  We should in aiming for less
> > implicit accesses to cpu state, not more.
> 
> Performance wise global env variable is faster and the register is
> always available. 

Not entirely true.  Reserving the global env variable has significant cost, 
especially on hosts with limited register sets (i.e. x86).  It's also a rather 
fragile hack.  There's a fairly long history of nasy hacks and things that 
just don't work in this context.  For example you can't reliably include 
stdio.h from these files, and they often break if you turn optimization off, 
which makes debugging much harder than it should be.

> Do you mean that we should aim to get rid of special
> status of global env, so that if no op uses it, it could be discarded
> to free a register?

That's already true for most of qemu.  IMO more interesting is being able to 
tell TCG that helpers don't mess with cpu state in opaque ways.  In theory 
it's already possible to do that manually. However that tends to be somewhat 
fragile, relying on careful maintenance and code code auditing, with mistakes 
triggering subtle hard-to-debug failures.  Much better to avoid the implicit 
global interface and make all helper arguments explicit.

> > Maybe better would be to explicitly pass a pointer the fp status. That
> > way you don't even need separate VFP and NEON variants of these
> > routines.
> 
> It would be nice to have generic float functions callable directly as
> TCG helper.

Possibly.  I'd have to look quite a bit closer to determine whether exposing 
the generic float functions is useful in practice, or if you still end up 
needing wrappers in most cases for most targets.  Adding "native" floating 
point support to the TCG interface is also a possibility.  In practice this 
might end up as wrappers around helper functions, but might provide a nicer 
programming interface.

Paul

[Qemu-devel] [PATCH V4 04/10] Add tpm_tis driver to build process

[Stefan Berger]

The TPM interface (tpm_tis) needs to be explicitly enabled via 
./configure --enable-tpm. This patch also restricts the building of the
TPM support to i386 and x86_64 targets since both backends I know
of, the Xen backend and the libtpms-based backend, will likely only
be available for these targets, at least initially. The list can be
easily extend. I am trying to prevent that one will end up with
support for a frontend but no available backend.

v3:
 - fixed and moved hunks in Makefile.target into right place

Signed-off-by: Stefan Berger 

Index:qemu/Makefile.target
===
---
 Makefile.target |1 +
 configure   |   20 
 2 files changed, 21 insertions(+)

Index: qemu-git/Makefile.target
===
--- qemu-git.orig/Makefile.target
+++ qemu-git/Makefile.target
@@ -226,6 +226,7 @@ obj-i386-y += device-hotplug.o pci-hotpl
 obj-i386-y += debugcon.o multiboot.o
 obj-i386-y += pc_piix.o kvmclock.o
 obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o
+obj-i386-$(CONFIG_TPM) += tpm_tis.o
 
 # shared objects
 obj-ppc-y = ppc.o
Index: qemu-git/configure
===
--- qemu-git.orig/configure
+++ qemu-git/configure
@@ -178,6 +178,7 @@ rbd=""
 smartcard=""
 smartcard_nss=""
 opengl=""
+tpm="no"
 
 # parse CC options first
 for opt do
@@ -713,6 +714,8 @@ for opt do
   ;;
   --kerneldir=*) kerneldir="$optarg"
   ;;
+  --enable-tpm) tpm="yes"
+  ;;
   --with-pkgversion=*) pkgversion=" ($optarg)"
   ;;
   --disable-docs) docs="no"
@@ -945,6 +948,7 @@ echo "  --disable-smartcard  disable
 echo "  --enable-smartcard   enable smartcard support"
 echo "  --disable-smartcard-nss  disable smartcard nss support"
 echo "  --enable-smartcard-nss   enable smartcard nss support"
+echo "  --enable-tpm enables an emulated TPM"
 echo ""
 echo "NOTE: The object files are built at the place where configure is 
launched"
 exit 1
@@ -2623,6 +2627,7 @@ echo "rbd support   $rbd"
 echo "xfsctl support$xfs"
 echo "nss used  $smartcard_nss"
 echo "OpenGL support$opengl"
+echo "TPM support   $tpm"
 
 if test $sdl_too_old = "yes"; then
 echo "-> Your SDL version is too old - please upgrade to have SDL support"
@@ -3433,6 +3438,21 @@ if test "$gprof" = "yes" ; then
   fi
 fi
 
+if test "$tpm" = "yes"; then
+  has_tpm=0
+  if test "$target_softmmu" = "yes" ; then
+case "$TARGET_BASE_ARCH" in
+i386)
+  has_tpm=1
+;;
+esac
+  fi
+
+  if test "$has_tpm" = "1"; then
+  echo "CONFIG_TPM=y" >> $config_host_mak
+  fi
+fi
+
 linker_script="-Wl,-T../config-host.ld -Wl,-T,\$(SRC_PATH)/\$(ARCH).ld"
 if test "$target_linux_user" = "yes" -o "$target_bsd_user" = "yes" ; then
   case "$ARCH" in

[Qemu-devel] [PATCH V4 10/10] Encrypt state blobs using AES CBC encryption

[Stefan Berger]

This patch adds encryption of the individual state blobs that are written
into the block storage. The 'directory' at the beginnig of the block
storage is not encrypted.

Keys can be passed either as a string of hexadecimal digits forming a 256,
192 or 128 bit AES key. Those keys can optionally start with '0x'. If the
parse does not recognize it as such, the string itself is taken as the AES
key.

The key is passed via command line argument. It's wiped from the command
line after parsing. If key=0x1234... was passed before it will then be
changed to key=--... so that 'ps' does not show the key anymore. Obviously
it cannot be completely prevented that the key is visible during a very
short period of time until qemu is done parsing the command line parameters.

A flag is introduced in the directory structure indicating whether the blobs
are encrypted.

An additional 'layer' for reading and writing the blobs to the underlying
block storage is added. This layer encrypts the blobs for writing if a key is
available. Similarly it decrypts the blobs after reading.

Checks are added the test whether a key has been provided although all
data are stored in clear-text or whether a key is missing. In either one of
the cases the backend returns an error and Qemu terminates.

Signed-off-by: Stefan Berger 

---
 arch_init.c  |   10 ++
 hw/tpm_builtin.c |  214 +--
 qemu-config.c|5 +
 qemu-options.hx  |2 
 4 files changed, 226 insertions(+), 5 deletions(-)

Index: qemu-git/hw/tpm_builtin.c
===
--- qemu-git.orig/hw/tpm_builtin.c
+++ qemu-git/hw/tpm_builtin.c
@@ -26,6 +26,7 @@
 #include "hw/pc.h"
 #include "migration.h"
 #include "sysemu.h"
+#include "aes.h"
 
 #include 
 #include 
@@ -105,7 +106,8 @@ typedef struct BSDir {
 uint16_t  rev;
 uint32_t  checksum;
 uint32_t  num_entries;
-uint32_t  reserved[10];
+uint32_t  flags;
+uint32_t  reserved[9];
 BSEntry   entries[BS_DIR_MAX_NUM_ENTRIES];
 } __attribute__((packed)) BSDir;
 
@@ -114,6 +116,8 @@ typedef struct BSDir {
 
 #define BS_DIR_REV_CURRENT  BS_DIR_REV1
 
+#define BS_DIR_FLAG_ENC_BLOBS   (1 << 0)
+
 /* local variables */
 
 static QemuThread thread;
@@ -145,6 +149,8 @@ static const unsigned char tpm_std_fatal
 
 static char dev_description[80];
 
+static bool has_key;
+static AES_KEY tpm_enc_key, tpm_dec_key;
 
 static void tpm_builtin_adjust_data_layout(BlockDriverState *bs, BSDir *dir);
 static int tpm_builtin_load_sized_data_from_bs(BlockDriverState *bs,
@@ -201,6 +207,7 @@ static void tpm_builtin_dir_be_to_cpu(BS
 be16_to_cpus(&dir->rev);
 be32_to_cpus(&dir->checksum);
 be32_to_cpus(&dir->num_entries);
+be32_to_cpus(&dir->flags);
 
 for (c = 0; c < dir->num_entries && c < BS_DIR_MAX_NUM_ENTRIES; c++) {
 be32_to_cpus(&dir->entries[c].type);
@@ -227,6 +234,7 @@ static void tpm_builtin_dir_cpu_to_be(BS
 dir->rev = cpu_to_be16(dir->rev);
 dir->checksum= cpu_to_be32(dir->checksum);
 dir->num_entries = cpu_to_be32(dir->num_entries);
+dir->flags   = cpu_to_be32(dir->flags);
 }
 
 
@@ -292,6 +300,36 @@ static bool tpm_builtin_has_valid_conten
 }
 
 
+static uint32_t tpm_builtin_get_dir_flags(void)
+{
+if (has_key) {
+return BS_DIR_FLAG_ENC_BLOBS;
+}
+
+return 0;
+}
+
+
+static bool tpm_builtin_has_missing_key(const BSDir *dir)
+{
+if ((dir->flags & BS_DIR_FLAG_ENC_BLOBS) && !has_key) {
+return true;
+}
+
+return false;
+}
+
+
+static bool tpm_builtin_has_unnecessary_key(const BSDir *dir)
+{
+if (!(dir->flags & BS_DIR_FLAG_ENC_BLOBS) && has_key) {
+return true;
+}
+
+return false;
+}
+
+
 static int tpm_builtin_create_blank_dir(BlockDriverState *bs)
 {
 uint8_t buf[BDRV_SECTOR_SIZE];
@@ -302,6 +340,7 @@ static int tpm_builtin_create_blank_dir(
 dir = (BSDir *)buf;
 dir->rev = BS_DIR_REV_CURRENT;
 dir->num_entries = 0;
+dir->flags = tpm_builtin_get_dir_flags();
 
 dir->checksum = tpm_builtin_calc_dir_checksum(dir);
 
@@ -402,6 +441,28 @@ static int startup_bs(BlockDriverState *
 
 tpm_builtin_dir_be_to_cpu(dir);
 
+if (tpm_builtin_is_valid_bsdir(dir)) {
+if (tpm_builtin_has_missing_key(dir)) {
+fprintf(stderr,
+"tpm: the data are encrypted but I am missing the key.\n");
+rc = -EIO;
+goto err_exit;
+}
+if (tpm_builtin_has_unnecessary_key(dir)) {
+fprintf(stderr,
+"tpm: I have a key but the data are not encrypted.\n");
+rc = -EIO;
+goto err_exit;
+}
+if ((dir->flags & BS_DIR_FLAG_ENC_BLOBS) &&
+!tpm_builtin_has_valid_content(dir)) {
+fprintf(stderr, "tpm: cannot read the data - "
+"is this the wrong key?\n");
+rc = -EIO;
+goto err_exit;
+}
+ 

[Qemu-devel] [PATCH V4 09/10] Add block storage support for libtpms based TPM backend

[Stefan Berger]

This patch supports the storage of TPM persisten state.

The TPM creates state of varying size, depending for example how many
keys are loaded into it a a certain time. The worst-case sizes of
the different blobs the TPM can write have been pre-calculated and this
value is used to determine the minimum size of the Qcow2 image. It needs to
be 63kb. 'qemu-... -tpm ?' shows this number when this backend driver is
available.


The layout of the TPM's persistent data in the block storage is as follows:

The first sector (512 bytes) holds a primitive directory for the different
types of blobs that the TPM can write. This directory holds a revision
number, a checksum over its content, the number of entries, and the entries
themselves. The entries are described through their absolute offsets, their
maximum sizes, the number of currently valid bytes (the blobs infalte
and deflate) and what type of blob it is (see below for the types).

typedef struct BSDir {
uint16_t  rev;
uint32_t  checksum; 
uint32_t  num_entries;
uint32_t  reserved[10];
BSEntry   entries[BS_DIR_MAX_NUM_ENTRIES];
} __attribute__((packed)) BSDir;


Their worst case sizes have been calculated and according to these sizes
the blobs are written at certain offsets into the blockstorage. Their offsets
are all aligned to sectors (512 byte boundaries).

The TPM provides three different blobs that are written into the storage:

- volatile state
- permanent state
- save state

The 'save state' is written when the VM suspends (ACPI S3) and read when it
resumes. This is done in concert with the BIOS where the BIOS needs to send
a command to the TPM upon resume (TPM_Startup(ST_STATE)), while the OS
issues the command TPM_SaveState().

The 'perment state' is written when the TPM receives a command that alters
its permenent state, i.e., when the a key is loaded into the TPM that
is expected to be there upon reboot of the machine / VM.

Volatile state is written when the frontend triggers it to do so, i.e.,
when the VM's state is written out during taking of a snapshot, migration
or suspension to disk (as in 'virsh save'). This state serves to resume
at the point where the TPM previously stopped but there is no need for it
after a machine reboot for example.

Tricky parts here are related to encrypted storage where certain operations
need to be deferred since the key for the storage only becomes available
much later than the time that the backend is instantiated.

The backend also tries to check for the validity of the block storage for
example. If the Qcow2 is not encrypted and the checksum is found to be
bad, the block storage directory will be initialized. 
In case the Qcow2 is encrypted, initialization will only be done if
the directory is found to be all 0s. In case the directory cannot be
checksummed correctly, but is not all 0s, it is assumed that the user
provided a wrong key. In this case I am not exiting qemu, but black-out
the TPM interface (returns 0xff in all memory location) due to a presumed
fatal error and let the VM run (without TPM functionality).

v4:
  - functions prefixed with tpm_builtin
  - added 10 uint32_t to BSDir as being reserved for future use
  - never move data in the block storage while migration is going on
  - use brdv_lock/bdrv_unlock to serialize access to the TPM's state
file which is primarily necessary during migration and the startup
of qemu on the target host where the content of the drive is being
read and validated

v3:
  - added reserved int's for future extensions to the entries in the
directory structure
  - added crc32 to every entry in the directory structure and calculating
it when writing and checking it when reading
  - fixed an endianess issue related to crc calculation
  - surrounding debugging output function in adjust_data_layout
with #if defined DEBUG_TPM
  - probing for installed libtpms development package by test-compiling

Signed-off-by: Stefan Berger 

---
 configure|   25 +
 hw/tpm_builtin.c |  812 ++-
 2 files changed, 834 insertions(+), 3 deletions(-)

Index: qemu-git/hw/tpm_builtin.c
===
--- qemu-git.orig/hw/tpm_builtin.c
+++ qemu-git/hw/tpm_builtin.c
@@ -1,4 +1,4 @@
-/*
+;/*
  *  builtin TPM driver based on libtpms
  *
  *  Copyright (c) 2010, 2011 IBM Corporation
@@ -44,6 +44,33 @@
 #define PERMSTATE_TYPE 'P'
 #define VOLASTATE_TYPE 'V'
 
+#define ALIGN(VAL, SIZE) \
+  ( ( (VAL) + (SIZE) - 1 ) & ~( (SIZE) - 1 ) )
+
+
+#define DIRECTORY_SIZEBDRV_SECTOR_SIZE
+
+#define PERMSTATE_DISK_OFFSET ALIGN(DIRECTORY_SIZE, BDRV_SECTOR_SIZE)
+#define PERMSTATE_DISK_SPACE \
+  ALIGN(tpmlib_get_prop(TPMPROP_TPM_MAX_NV_SPACE),\
+BDRV_SECTOR_SIZE)
+#define SAVESTATE_DISK_OFFSET (PERMSTATE_DISK_OFFSET + PERMSTATE_DISK_SPACE)
+#define SAVESTATE_DISK_SPACE \
+  ALIGN(tpmlib_get_prop(TPMPROP_TPM_MAX_SA

[Qemu-devel] [PATCH V4 05/10] Add a debug register

[Stefan Berger]

This patch uses the possibility to add a vendor-specific register and
adds a debug register useful for dumping the TIS's internal state. This
register is only active in a debug build (#define DEBUG_TIS).

v3:
 - all output goes to stderr

Signed-off-by: Stefan Berger 

---
 hw/tpm_tis.c |   67 +++
 1 file changed, 67 insertions(+)

Index: qemu-git/hw/tpm_tis.c
===
--- qemu-git.orig/hw/tpm_tis.c
+++ qemu-git/hw/tpm_tis.c
@@ -42,6 +42,8 @@
 #define TIS_REG_DID_VID   0xf00
 #define TIS_REG_RID   0xf04
 
+/* vendor-specific registers */
+#define TIS_REG_DEBUG 0xf90
 
 #define STS_VALID(1 << 7)
 #define STS_COMMAND_READY(1 << 6)
@@ -356,6 +358,66 @@ static uint32_t tis_data_read(TPMState *
 }
 
 
+#ifdef DEBUG_TIS
+static void tis_dump_state(void *opaque, target_phys_addr_t addr)
+{
+static const unsigned regs[] = {
+TIS_REG_ACCESS,
+TIS_REG_INT_ENABLE,
+TIS_REG_INT_VECTOR,
+TIS_REG_INT_STATUS,
+TIS_REG_INTF_CAPABILITY,
+TIS_REG_STS,
+TIS_REG_DID_VID,
+TIS_REG_RID,
+0xfff};
+int idx;
+uint8_t locty = tis_locality_from_addr(addr);
+target_phys_addr_t base = addr & ~0xfff;
+TPMState *s = opaque;
+
+fprintf(stderr,
+"tpm_tis: active locality  : %d\n"
+"tpm_tis: state of locality %d : %d\n"
+"tpm_tis: register dump:\n",
+s->active_locty,
+locty, s->loc[locty].state);
+
+for (idx = 0; regs[idx] != 0xfff; idx++) {
+fprintf(stderr, "tpm_tis: 0x%04x : 0x%08x\n", regs[idx],
+tis_mem_readl(opaque, base + regs[idx]));
+}
+
+fprintf(stderr,
+"tpm_tis: read offset   : %d\n"
+"tpm_tis: result buffer : ",
+s->loc[locty].r_offset);
+for (idx = 0;
+ idx < tis_get_size_from_buffer(&s->loc[locty].r_buffer);
+ idx++) {
+fprintf(stderr, "%c%02x%s",
+s->loc[locty].r_offset == idx ? '>' : ' ',
+s->loc[locty].r_buffer.buffer[idx],
+((idx & 0xf) == 0xf) ? "\ntpm_tis: " : "");
+}
+fprintf(stderr,
+"\n"
+"tpm_tis: write offset  : %d\n"
+"tpm_tis: request buffer: ",
+s->loc[locty].w_offset);
+for (idx = 0;
+ idx < tis_get_size_from_buffer(&s->loc[locty].w_buffer);
+ idx++) {
+fprintf(stderr, "%c%02x%s",
+s->loc[locty].w_offset == idx ? '>' : ' ',
+s->loc[locty].w_buffer.buffer[idx],
+((idx & 0xf) == 0xf) ? "\ntpm_tis: " : "");
+}
+fprintf(stderr,"\n");
+}
+#endif
+
+
 /*
  * Read a register of the TIS interface
  * See specs pages 33-63 for description of the registers
@@ -431,6 +493,11 @@ static uint32_t tis_mem_readl(void *opaq
 case TIS_REG_RID:
 val = TPM_RID;
 break;
+#ifdef DEBUG_TIS
+case TIS_REG_DEBUG:
+tis_dump_state(opaque, addr);
+break;
+#endif
 }
 
 qemu_mutex_unlock(&s->state_lock);

[Qemu-devel] [PATCH V4 00/10] Qemu Trusted Platform Module (TPM) integration

[Stefan Berger]

The following series of patches adds a TPM (Trusted Platform Module)
TIS (TPM Interface Spec) interface to Qemu and with that provides
means to access a backend implementing the actual TPM functionality.
This frontend enables for example Linux's TPM TIS (tpm_tis) driver.

I am also posting the implementation of a backend implementation that is based
on a library (libtpms) providing TPM functionality. This library is currently
undergoing further testing but is now available via Fedora Rawhide:

http://download.fedora.redhat.com/pub/fedora/linux/development/rawhide/x86_64/os/Packages/libtpms-0.5.1-5.x86_64.rpm
http://download.fedora.redhat.com/pub/fedora/linux/development/rawhide/x86_64/os/Packages/libtpms-devel-0.5.1-5.x86_64.rpm

All testing was done with the libtpms-based backend. It provides support for
VM suspend/resume, migration and snapshotting. It uses QCoW2 as the file
format for storing its persistent state onto, which is necessary for support
of snapshotting. Using Linux as the OS along with some recently posted patches
for the Linux TPM TIS driver, suspend/resume works fine (using 'virsh
save/restore') along with hibernation and OS suspend (ACPI S3).

Proper support for the TPM requires support in the BIOS since the BIOS
needs to initialize the TPM upon machine start or issue commands to the TPM
when it resumes from suspend (ACPI S3). It also builds and connects the
necessary ACPI tables (SSDT for TPM device, TCPA table for logging) to the
ones that are built by a BIOS. To support this I have fairly extensive
set of extensions for SeaBIOS that have already been posted to the SeaBIOS
mailing list and been ACK'ed by Kevin (thank you! :-)).

v4:
 - applies to checkout of d2d979c6
 - more coding style fixes
 - adding patch for supporting blob encryption (in addition to the existing
   QCoW2-level encryption)
   - this allows for graceful termination of a migration if the target
 is detected to have a wrong key
   - tested with big and little endian hosts
 - main thread releases mutex while checking for work to do on behalf of
   backend
 - introducing file locking (fcntl) on the block layer for serializing access
   to shared (QCoW2) files (used during migration)

v3:
 - Building a null driver at patch 5/8 that responds to all requests
   with an error response; subsequently this driver is transformed to the
   libtpms-based driver for real TPM functionality
 - Reworked the threading; dropped the patch for qemu_thread_join; the
   main thread synchronizing with the TPM thread termination may need
   to write data to the block storage while waiting for the thread to 
   terminate; did not previously show a problem but is safer
 - A lot of testing based on recent git checkout 4b4a72e5 (4/10):
   - migration of i686 VM from x86_64 host to i686 host to ppc64 host while
 running tests inside the VM
   - tests with S3 suspend/resume
   - tests with snapshots
   - multiple-hour tests with VM suspend/resume (using virsh save/restore)
 while running a TPM test suite inside the VM
   All tests passed; [not all of them were done on the ppc64 host]

v2:
 - splitting some of the patches into smaller ones for easier review
 - fixes in individual patches

Regards,
Stefan

[Qemu-devel] [PATCH V4 03/10] Add persistent state handling to TPM TIS frontend driver

[Stefan Berger]

This patch adds support for handling of persistent state to the TPM TIS
frontend.

The currently used buffer is determined (can only be in currently active
locality and either be a read or a write buffer) and only that buffer's content
is stored. The reverse is done when the state is restored from disk
where the buffer's content are copied into the currently used buffer.

To keep compatibility with existing Xen the VMStateDescription was adapted
to be compatible with existing state. For that I am adding Andreas
Niederl as an author to the file.

v4:
 - main thread releases the 'state' lock while periodically calling the
   backends function that may request it to write data into block storage.

v3:
 - all functions prefixed with tis_
 - while the main thread is waiting for an outstanding TPM command to finish,
   it periodically does some work (writes data to the block storage)

Signed-off-by: Stefan Berger 

---
 hw/tpm_tis.c |  167 +++
 1 file changed, 167 insertions(+)

Index: qemu-git/hw/tpm_tis.c
===
--- qemu-git.orig/hw/tpm_tis.c
+++ qemu-git/hw/tpm_tis.c
@@ -6,6 +6,8 @@
  * Author: Stefan Berger 
  * David Safford 
  *
+ * Xen 4 support: Andrease Niederl 
+ *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License as
  * published by the Free Software Foundation, version 2 of the
@@ -869,3 +871,168 @@ static int tis_init(ISADevice *dev)
 return -1;
 }
 
+/* persistent state handling */
+
+static void tis_pre_save(void *opaque)
+{
+TPMState *s = opaque;
+uint8_t locty = s->active_locty;
+
+qemu_mutex_lock(&s->state_lock);
+
+/* wait for outstanding requests to complete */
+if (IS_VALID_LOCTY(locty) && s->loc[locty].state == STATE_EXECUTION) {
+if (!active_be->job_for_main_thread) {
+qemu_cond_wait(&s->from_tpm_cond, &s->state_lock);
+} else {
+while (s->loc[locty].state == STATE_EXECUTION) {
+qemu_mutex_unlock(&s->state_lock);
+
+active_be->job_for_main_thread(NULL);
+usleep(1);
+
+qemu_mutex_lock(&s->state_lock);
+}
+}
+}
+
+#ifdef DEBUG_TIS_SR
+fprintf(stderr,"tpm_tis: suspend: locty 0 : r_offset = %d, w_offset = 
%d\n",
+s->loc[0].r_offset,
+s->loc[0].w_offset);
+if (s->loc[0].r_offset) {
+tis_dump_state(opaque, 0);
+}
+#endif
+
+qemu_mutex_unlock(&s->state_lock);
+
+/* copy current active read or write buffer into the buffer
+   written to disk */
+if (IS_VALID_LOCTY(locty)) {
+switch (s->loc[locty].state) {
+case STATE_RECEPTION:
+memcpy(s->buf,
+   s->loc[locty].w_buffer.buffer,
+   MIN(sizeof(s->buf),
+   s->loc[locty].w_buffer.size));
+s->offset = s->loc[locty].w_offset;
+break;
+case STATE_COMPLETION:
+memcpy(s->buf,
+   s->loc[locty].r_buffer.buffer,
+   MIN(sizeof(s->buf),
+   s->loc[locty].r_buffer.size));
+s->offset = s->loc[locty].r_offset;
+break;
+default:
+/* leak nothing */
+memset(s->buf, 0x0, sizeof(s->buf));
+break;
+}
+}
+
+tis_get_active_backend()->save_volatile_data();
+}
+
+
+static int tis_post_load(void *opaque,
+ int version_id __attribute__((unused)))
+{
+TPMState *s = opaque;
+
+uint8_t locty = s->active_locty;
+
+if (IS_VALID_LOCTY(locty)) {
+switch (s->loc[locty].state) {
+case STATE_RECEPTION:
+memcpy(s->loc[locty].w_buffer.buffer,
+   s->buf,
+   MIN(sizeof(s->buf),
+   s->loc[locty].w_buffer.size));
+s->loc[locty].w_offset = s->offset;
+break;
+case STATE_COMPLETION:
+memcpy(s->loc[locty].r_buffer.buffer,
+   s->buf,
+   MIN(sizeof(s->buf),
+   s->loc[locty].r_buffer.size));
+s->loc[locty].r_offset = s->offset;
+break;
+default:
+break;
+}
+}
+
+#ifdef DEBUG_TIS_SR
+fprintf(stderr,"tpm_tis: resume : locty 0 : r_offset = %d, w_offset = 
%d\n",
+s->loc[0].r_offset,
+s->loc[0].w_offset);
+#endif
+
+return tis_get_active_backend()->load_volatile_data(s);
+}
+
+
+static const VMStateDescription vmstate_locty = {
+.name = "loc",
+.version_id = 1,
+.minimum_version_id = 0,
+.minimum_version_id_old = 0,
+.fields  = (VMStateField[]) {
+VMSTATE_UINT32(state   , TPMLocality),
+VMSTATE_UINT32(inte, TPMLocality),
+VMSTATE_UINT32(ints, TPMLocality),
+VMSTATE_UINT8 (access  , TPMLocality),

[Qemu-devel] [PATCH V4 08/10] Introduce file lock for the block layer

[Stefan Berger]

This patch introduces file locking via fcntl() for the block layer so that
concurrent access to files shared by 2 Qemu instances, for example via NFS,
can be serialized. This feature is useful primarily during initial phases of
VM migration where the target machine's TIS driver validates the block
storage (and in a later patch checks for missing AES keys).

Support for win32 is based on win32 API and has been lightly tested with a
standalone test program locking shared storage from two different machines.

To enable locking a file multiple times, a counter is used. Actual locking
happens the very first time and unlocking happens when the counter is zero.

Signed-off-by: Stefan Berger 

---

---
 block.c   |   40 ++
 block.h   |8 ++
 block/raw-posix.c |   62 ++
 block/raw-win32.c |   51 
 block_int.h   |4 +++
 5 files changed, 165 insertions(+)

Index: qemu-git/block.c
===
--- qemu-git.orig/block.c
+++ qemu-git/block.c
@@ -475,6 +475,8 @@ static int bdrv_open_common(BlockDriverS
 goto free_and_fail;
 }
 
+drv->num_locks = 0;
+
 bs->keep_read_only = bs->read_only = !(open_flags & BDRV_O_RDWR);
 
 ret = refresh_total_sectors(bs, bs->total_sectors);
@@ -1181,6 +1183,44 @@ void bdrv_get_geometry(BlockDriverState 
 *nb_sectors_ptr = length;
 }
 
+/* file locking */
+static int bdrv_lock_common(BlockDriverState *bs, BDRVLockType lock_type)
+{
+BlockDriver *drv = bs->drv;
+
+if (!drv)
+return -ENOMEDIUM;
+
+if (bs->file) {
+drv = bs->file->drv;
+if (drv->bdrv_lock) {
+return drv->bdrv_lock(bs->file, lock_type);
+}
+}
+
+if (drv->bdrv_lock) {
+return drv->bdrv_lock(bs, lock_type);
+}
+
+return -ENOTSUP;
+}
+
+
+int bdrv_lock(BlockDriverState *bs)
+{
+if (bdrv_is_read_only(bs)) {
+return bdrv_lock_common(bs, BDRV_F_RDLCK);
+}
+
+return bdrv_lock_common(bs, BDRV_F_WRLCK);
+}
+
+void bdrv_unlock(BlockDriverState *bs)
+{
+bdrv_lock_common(bs, BDRV_F_UNLCK);
+}
+
+
 struct partition {
 uint8_t boot_ind;   /* 0x80 - active */
 uint8_t head;   /* starting head */
Index: qemu-git/block.h
===
--- qemu-git.orig/block.h
+++ qemu-git/block.h
@@ -42,6 +42,12 @@ typedef struct QEMUSnapshotInfo {
 #define BDRV_SECTOR_MASK   ~(BDRV_SECTOR_SIZE - 1)
 
 typedef enum {
+BDRV_F_UNLCK,
+BDRV_F_RDLCK,
+BDRV_F_WRLCK,
+} BDRVLockType;
+
+typedef enum {
 BLOCK_ERR_REPORT, BLOCK_ERR_IGNORE, BLOCK_ERR_STOP_ENOSPC,
 BLOCK_ERR_STOP_ANY
 } BlockErrorAction;
@@ -95,6 +101,8 @@ int bdrv_commit(BlockDriverState *bs);
 void bdrv_commit_all(void);
 int bdrv_change_backing_file(BlockDriverState *bs,
 const char *backing_file, const char *backing_fmt);
+int bdrv_lock(BlockDriverState *bs);
+void bdrv_unlock(BlockDriverState *bs);
 void bdrv_register(BlockDriver *bdrv);
 
 
Index: qemu-git/block/raw-posix.c
===
--- qemu-git.orig/block/raw-posix.c
+++ qemu-git/block/raw-posix.c
@@ -718,6 +718,66 @@ static int64_t raw_getlength(BlockDriver
 }
 #endif
 
+static int raw_lock(BlockDriverState *bs, BDRVLockType lock_type)
+{
+BlockDriver *drv = bs->drv;
+BDRVRawState *s = bs->opaque;
+struct flock flock = {
+.l_whence = SEEK_SET,
+.l_start = 0,
+.l_len = 0,
+};
+int n;
+
+switch (lock_type) {
+case BDRV_F_RDLCK:
+case BDRV_F_WRLCK:
+if (drv->num_locks) {
+drv->num_locks++;
+return 0;
+}
+flock.l_type = (lock_type == BDRV_F_RDLCK) ? F_RDLCK : F_WRLCK;
+break;
+
+case BDRV_F_UNLCK:
+if (--drv->num_locks > 0) {
+return 0;
+}
+
+assert(drv->num_locks == 0);
+
+flock.l_type = F_UNLCK;
+break;
+
+default:
+return -EINVAL;
+}
+
+while (1) {
+n = fcntl(s->fd, F_SETLKW, &flock);
+if (n < 0) {
+if (errno == EINTR) {
+continue;
+}
+if (errno == EAGAIN) {
+usleep(1);
+continue;
+}
+}
+break;
+}
+
+if (n == 0 &&
+((lock_type == BDRV_F_RDLCK) || (lock_type == BDRV_F_WRLCK))) {
+drv->num_locks = 1;
+}
+
+if (n)
+return -errno;
+
+return 0;
+}
+
 static int raw_create(const char *filename, QEMUOptionParameter *options)
 {
 int fd;
@@ -814,6 +874,8 @@ static BlockDriver bdrv_file = {
 .bdrv_truncate = raw_truncate,
 .bdrv_getlength = raw_getlength,
 
+.bdrv_lock = raw_lock,
+
 .create_options = raw_create_options,
 };
 
Index: qemu-git/block_int.h
=

[Qemu-devel] [PATCH V4 02/10] Add TPM (frontend) hardware interface (TPM TIS) to Qemu

[Stefan Berger]

This patch adds the main code of the TPM frontend driver, the TPM TIS
interface, to Qemu. The code is largely based on my previous implementation
for Xen but has been significantly extended to meet the standard's
requirements, such as the support for changing of localities and all the
functionality of the available flags.

Communication with the backend (i.e., for Xen or the libtpms-based one)
is cleanly separated through an interface which the backend driver needs
to implement.

The TPM TIS driver's backend was previously chosen in the code added
to arch_init. The frontend holds a pointer to the chosen backend (interface).

Communication with the backend is largely based on signals and conditions.
Whenever the frontend has collected a complete packet, it will signal
the backend, which then starts processing the command. Once the result
has been returned, the backend invokes a callback function
(tis_tpm_receive_cb()).

The one tricky part is support for VM suspend while the TPM is processing
a command. In this case the frontend driver is waiting for the backend
to return the result of the last command before shutting down. It waits
on a condition for a signal from the backend, which is delivered in 
tis_tpm_receive_cb().

Testing the proper functioning of the different flags and localities 
cannot be done from user space when running in Linux for example, since
access to the address space of the TPM TIS interface is not possible. Also
the Linux driver itself does not exercise all functionality. So, for
testing there is a fairly extensive test suite as part of the SeaBIOS patches
since from within the BIOS one can have full access to all the TPM's registers.

v3:
  - prefixing functions with tis_
  - added a function to the backend interface 'early_startup_tpm' that
allows to detect the presence of the block storage and gracefully fails
Qemu if it's not available. This works with migration using shared
storage but doesn't support migration with block storage migration.
For encyrypted QCoW2 and in case of a snapshot resue the late_startup_tpm
interface function is called

Signed-off-by: Stefan Berger 

---
 hw/tpm_tis.c |  871 +++
 1 file changed, 871 insertions(+)

Index: qemu-git/hw/tpm_tis.c
===
--- /dev/null
+++ qemu-git/hw/tpm_tis.c
@@ -0,0 +1,871 @@
+/*
+ * tpm_tis.c - QEMU emulator for a 1.2 TPM with TIS interface
+ *
+ * Copyright (C) 2006,2010 IBM Corporation
+ *
+ * Author: Stefan Berger 
+ * David Safford 
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation, version 2 of the
+ * License.
+ *
+ *
+ * Implementation of the TIS interface according to specs at
+ * 
https://www.trustedcomputinggroup.org/groups/pc_client/TCG_PCClientTPMSpecification_1-20_1-00_FINAL.pdf
+ *
+ */
+
+#include "block.h"
+#include "hw/hw.h"
+#include "hw/pc.h"
+#include "hw/tpm_tis.h"
+
+#include 
+
+//#define DEBUG_TIS
+
+/* whether the STS interrupt is supported */
+//#define RAISE_STS_IRQ
+
+/* tis registers */
+#define TIS_REG_ACCESS0x00
+#define TIS_REG_INT_ENABLE0x08
+#define TIS_REG_INT_VECTOR0x0c
+#define TIS_REG_INT_STATUS0x10
+#define TIS_REG_INTF_CAPABILITY   0x14
+#define TIS_REG_STS   0x18
+#define TIS_REG_DATA_FIFO 0x24
+#define TIS_REG_DID_VID   0xf00
+#define TIS_REG_RID   0xf04
+
+
+#define STS_VALID(1 << 7)
+#define STS_COMMAND_READY(1 << 6)
+#define STS_TPM_GO   (1 << 5)
+#define STS_DATA_AVAILABLE   (1 << 4)
+#define STS_EXPECT   (1 << 3)
+#define STS_RESPONSE_RETRY   (1 << 1)
+
+#define ACCESS_TPM_REG_VALID_STS (1 << 7)
+#define ACCESS_ACTIVE_LOCALITY   (1 << 5)
+#define ACCESS_BEEN_SEIZED   (1 << 4)
+#define ACCESS_SEIZE (1 << 3)
+#define ACCESS_PENDING_REQUEST   (1 << 2)
+#define ACCESS_REQUEST_USE   (1 << 1)
+#define ACCESS_TPM_ESTABLISHMENT (1 << 0)
+
+#define INT_ENABLED  (1 << 31)
+#define INT_DATA_AVAILABLE   (1 << 0)
+#define INT_STS_VALID(1 << 1)
+#define INT_LOCALITY_CHANGED (1 << 2)
+#define INT_COMMAND_READY(1 << 7)
+
+#ifndef RAISE_STS_IRQ
+
+# define INTERRUPTS_SUPPORTED (INT_LOCALITY_CHANGED | \
+   INT_DATA_AVAILABLE   | \
+   INT_COMMAND_READY)
+
+#else
+
+# define INTERRUPTS_SUPPORTED (INT_LOCALITY_CHANGED | \
+   INT_DATA_AVAILABLE   | \
+   INT_STS_VALID | \
+   INT_COMMAND_READY)
+
+#endif
+
+#define CAPABILITIES_SUPPORTED 

[Qemu-devel] [PATCH V4 01/10] Support for TPM command line options

[Stefan Berger]

This patch adds support for TPM command line options.
The command line supported here (considering the libtpms based
backend) are

./qemu-... -tpm type=,path=,

and

./qemu-... -tpm ?

where the latter works similar to -soundhw ? and shows a list of
available TPM backends (i.e., libtpms-based, Xen).

Only the 'type' is interpreted in arch_init.c. Using this parameter,
the backend is chosen, i.e., 'builtin' for the libtpms-based
builtin TPM. The interpretation of the other parameters along with
determining whether enough parameters were provided is pushed into
the backend driver, which needs to implement the interface function
'handle_options' and return true if the VM can be started or 'false'
if not enough or bad parameters were provided.

v4:
 - coding style fixes

v3:
 - added hw/tpm_tis.h to this patch so Qemu compiles at this stage

Signed-off-by: Stefan Berger 

---
 arch_init.c |   80 +++
 arch_init.h |2 
 hw/pc.c |4 +
 hw/pc.h |7 ++
 hw/tpm_tis.h|  143 
 qemu-config.c   |   20 +++
 qemu-options.hx |   11 
 vl.c|   12 
 8 files changed, 279 insertions(+)

Index: qemu-git/hw/pc.h
===
--- qemu-git.orig/hw/pc.h
+++ qemu-git/hw/pc.h
@@ -6,6 +6,7 @@
 #include "isa.h"
 #include "fdc.h"
 #include "net.h"
+#include "tpm_tis.h"
 
 /* PC-style peripherals (also used by other machines).  */
 
@@ -128,6 +129,12 @@ void pc_register_ferr_irq(qemu_irq irq);
 void pc_cmos_set_s3_resume(void *opaque, int irq, int level);
 void pc_acpi_smi_interrupt(void *opaque, int irq, int level);
 
+/* tpm_tis.c */
+extern bool has_tpm;
+const BackendTPMDriver *tis_set_backend_driver(const char *tpm_type);
+void tis_display_backend_drivers(FILE *);
+
+
 void pc_cpus_init(const char *cpu_model);
 void pc_memory_init(ram_addr_t ram_size,
 const char *kernel_filename,
Index: qemu-git/qemu-options.hx
===
--- qemu-git.orig/qemu-options.hx
+++ qemu-git/qemu-options.hx
@@ -1041,6 +1041,17 @@ Specify SMBIOS type 0 fields
 Specify SMBIOS type 1 fields
 ETEXI
 
+#ifndef _WIN32
+# ifdef CONFIG_TPM
+DEF("tpm", HAS_ARG, QEMU_OPTION_tpm, \
+""
+"-tpm type=,path=\n" \
+"enable a TPM with state from file in given path\n"
+"use -tpm ? to get a list of supported TPM types\n",
+QEMU_ARCH_I386)
+# endif
+#endif
+
 DEFHEADING()
 STEXI
 @end table
Index: qemu-git/vl.c
===
--- qemu-git.orig/vl.c
+++ qemu-git/vl.c
@@ -244,6 +244,8 @@ int nb_numa_nodes;
 uint64_t node_mem[MAX_NODES];
 uint64_t node_cpumask[MAX_NODES];
 
+bool has_tpm = false;
+
 static QEMUTimer *nographic_timer;
 
 uint8_t qemu_uuid[16];
@@ -2333,6 +2335,16 @@ int main(int argc, char **argv, char **e
 ram_size = value;
 break;
 }
+#ifdef CONFIG_TPM
+case QEMU_OPTION_tpm:
+if (!(tpm_available())) {
+printf("Option %s not supported for this target\n",
+   popt->name);
+exit(1);
+}
+select_tpm(optarg);
+break;
+#endif
 case QEMU_OPTION_mempath:
 mem_path = optarg;
 break;
Index: qemu-git/qemu-config.c
===
--- qemu-git.orig/qemu-config.c
+++ qemu-git/qemu-config.c
@@ -450,6 +450,25 @@ QemuOptsList qemu_option_rom_opts = {
 },
 };
 
+static QemuOptsList qemu_tpm_opts = {
+.name = "tpm",
+.head = QTAILQ_HEAD_INITIALIZER(qemu_tpm_opts.head),
+.desc = {
+{
+.name = "type",
+.type = QEMU_OPT_STRING,
+.help = "Type of TPM backend",
+},
+{
+.name = "path",
+.type = QEMU_OPT_STRING,
+.help = "Persitent storage for TPM state",
+},
+{ /* end of list */ }
+},
+};
+
+
 static QemuOptsList *vm_config_groups[32] = {
 &qemu_drive_opts,
 &qemu_chardev_opts,
@@ -464,6 +483,7 @@ static QemuOptsList *vm_config_groups[32
 &qemu_trace_opts,
 #endif
 &qemu_option_rom_opts,
+&qemu_tpm_opts,
 NULL,
 };
 
Index: qemu-git/arch_init.c
===
--- qemu-git.orig/arch_init.c
+++ qemu-git/arch_init.c
@@ -41,6 +41,8 @@
 #include "net.h"
 #include "gdbstub.h"
 #include "hw/smbios.h"
+#include "blockdev.h"
+#include "hw/tpm_tis.h"
 
 #ifdef TARGET_SPARC
 int graphic_width = 1024;
@@ -726,3 +728,81 @@ int xen_available(void)
 return 0;
 #endif
 }
+
+int tpm_available(void) {
+#ifdef CONFIG_TPM
+return 1;
+#else
+return 0;
+#endif
+}
+
+#ifdef CONFIG_TPM
+
+#if defined (TARGET_I386) || defined (TARGET_X86_64)
+

[Qemu-devel] [PATCH V4 07/10] Implementation of the libtpms-based backend

[Stefan Berger]

This patch provides the glue for the TPM TIS interface (frontend) to
the libtpms that provides the actual TPM functionality.

Some details:

This part of the patch provides support for the spawning of a thread
that will interact with the libtpms-based TPM. It expects a signal
from the frontend to wake and pick up the TPM command that is supposed
to be processed and delivers the response packet using a callback
function provided by the frontend.

The backend connectects itself to the frontend by filling out an interface
structure with pointers to the function implementing support for various
operations.

In this part a structure with callback functions with is registered with
libtpms. Those callback functions mostly deal with persistent storage.

The libtpms-based backend implements functionality to write into a 
Qemu block storage device rather than to plain files. With that we
can support VM snapshotting and we also get the possibility to use
encrypted QCoW2 for free. Thanks to Anthony for pointing this out.
The storage part of the driver has been split off into its own patch.

v3:
  - temporarily deactivate the building of the tpm_builtin.c until
subsequent patch completely converts it to the libtpms based driver

v2:
  - fixes to adhere to the qemu coding style


Signed-off-by: Stefan Berger 

---
 configure|1 
 hw/tpm_builtin.c |  415 ---
 hw/tpm_tis.h |   17 ++
 3 files changed, 412 insertions(+), 21 deletions(-)

Index: qemu-git/hw/tpm_tis.h
===
--- qemu-git.orig/hw/tpm_tis.h
+++ qemu-git/hw/tpm_tis.h
@@ -142,4 +142,21 @@ static inline void dumpBuffer(FILE *stre
 fprintf(stream, "\n");
 }
 
+static inline void clear_sized_buffer(TPMSizedBuffer *tpmsb)
+{
+if (tpmsb->buffer) {
+   tpmsb->size = 0;
+   qemu_free(tpmsb->buffer);
+   tpmsb->buffer = NULL;
+}
+}
+
+static inline void set_sized_buffer(TPMSizedBuffer *tpmsb,
+uint8_t *buffer, uint32_t size)
+{
+clear_sized_buffer(tpmsb);
+tpmsb->size = size;
+tpmsb->buffer = buffer;
+}
+
 #endif /* _HW_TPM_TIS_H */
Index: qemu-git/hw/tpm_builtin.c
===
--- qemu-git.orig/hw/tpm_builtin.c
+++ qemu-git/hw/tpm_builtin.c
@@ -1,5 +1,5 @@
 /*
- *  builtin 'null' TPM driver
+ *  builtin TPM driver based on libtpms
  *
  *  Copyright (c) 2010, 2011 IBM Corporation
  *  Copyright (c) 2010, 2011 Stefan Berger
@@ -18,16 +18,32 @@
  * License along with this library; if not, see 
  */
 
+#include "blockdev.h"
+#include "block_int.h"
 #include "qemu-common.h"
 #include "hw/hw.h"
 #include "hw/tpm_tis.h"
 #include "hw/pc.h"
+#include "migration.h"
+#include "sysemu.h"
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
 
 
 //#define DEBUG_TPM
 //#define DEBUG_TPM_SR /* suspend - resume */
 
 
+#define SAVESTATE_TYPE 'S'
+#define PERMSTATE_TYPE 'P'
+#define VOLASTATE_TYPE 'V'
+
 /* data structures */
 
 typedef struct ThreadParams {
@@ -43,12 +59,18 @@ static QemuThread thread;
 
 static QemuMutex state_mutex; /* protects *_state below */
 static QemuMutex tpm_initialized_mutex; /* protect tpm_initialized */
+static QemuCond bs_write_result_cond;
+static TPMSizedBuffer permanent_state = { .size = 0, .buffer = NULL, };
+static TPMSizedBuffer volatile_state  = { .size = 0, .buffer = NULL, };
+static TPMSizedBuffer save_state  = { .size = 0, .buffer = NULL, };
+static int pipefd[2] =  {-1, -1};
 
 static bool thread_terminate = false;
 static bool tpm_initialized = false;
 static bool had_fatal_error = false;
 static bool had_startup_error = false;
 static bool thread_running = false;
+static bool need_read_volatile = false;
 
 static ThreadParams tpm_thread_params;
 
@@ -62,9 +84,21 @@ static const unsigned char tpm_std_fatal
 static char dev_description[80];
 
 
+static int tpmlib_get_prop(enum TPMLIB_TPMProperty prop)
+{
+int result;
+
+TPM_RESULT res = TPMLIB_GetTPMProperty(prop, &result);
+
+assert(res == TPM_SUCCESS);
+
+return result;
+}
+
+
 static void *tpm_builtin_main_loop(void *d)
 {
-int res = 1;
+TPM_RESULT res;
 ThreadParams *thr_parms = d;
 uint32_t in_len, out_len;
 uint8_t *in, *out;
@@ -74,9 +108,10 @@ static void *tpm_builtin_main_loop(void 
 fprintf(stderr, "tpm: THREAD IS STARTING\n");
 #endif
 
-if (res != 0) {
+res = TPMLIB_MainInit();
+if (res != TPM_SUCCESS) {
 #if defined DEBUG_TPM || defined DEBUG_TPM_SR
-fprintf(stderr, "tpm: Error: TPM initialization failed (rc=%d)\n",
+fprintf(stderr," tpm: Error: Call to TPMLIB_MainInit() failed 
(rc=%d)\n",
 res);
 #endif
had_fatal_error = true;
@@ -94,6 +129,8 @@ static void *tpm_builtin_main_loop(void 
 in_len = 0;
 do {
 #ifdef DEBUG_TPM
+/* TPMLIB output goes to stderr *

[Qemu-devel] [PATCH V4 06/10] Add a TPM backend skeleton implementation

[Stefan Berger]

This patch provides a TPM backend skeleton implementation. It doesn't do
anything useful (except for returning error response for every TPM command)
but it compiles.

v3:
  - in tpm_builtin.c all functions prefixed with tpm_builtin_
  - build the builtin TPM driver available at this point; it returns
a failure response message for every command
  - do not try to join the TPM thread but poll for its termination;
the libtpms-based driver will require Qemu's main thread to write
data to the block storage device while trying to join

V2:
  - only terminating thread in tpm_atexit if it's running

Signed-off-by: Stefan Berger 

---
 Makefile.target  |5 
 configure|1 
 hw/tpm_builtin.c |  425 +++
 hw/tpm_tis.c |3 
 hw/tpm_tis.h |2 
 5 files changed, 436 insertions(+)

Index: qemu-git/hw/tpm_builtin.c
===
--- /dev/null
+++ qemu-git/hw/tpm_builtin.c
@@ -0,0 +1,425 @@
+/*
+ *  builtin 'null' TPM driver
+ *
+ *  Copyright (c) 2010, 2011 IBM Corporation
+ *  Copyright (c) 2010, 2011 Stefan Berger
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see 
+ */
+
+#include "qemu-common.h"
+#include "hw/hw.h"
+#include "hw/tpm_tis.h"
+#include "hw/pc.h"
+
+
+//#define DEBUG_TPM
+//#define DEBUG_TPM_SR /* suspend - resume */
+
+
+/* data structures */
+
+typedef struct ThreadParams {
+TPMState *tpm_state;
+
+TPMRecvDataCB *recv_data_callback;
+} ThreadParams;
+
+
+/* local variables */
+
+static QemuThread thread;
+
+static QemuMutex state_mutex; /* protects *_state below */
+static QemuMutex tpm_initialized_mutex; /* protect tpm_initialized */
+
+static bool thread_terminate = false;
+static bool tpm_initialized = false;
+static bool had_fatal_error = false;
+static bool had_startup_error = false;
+static bool thread_running = false;
+
+static ThreadParams tpm_thread_params;
+
+/* locality of the command being executed by libtpms */
+static uint8_t g_locty;
+
+static const unsigned char tpm_std_fatal_error_response[10] = {
+0x00, 0xc4, 0x00, 0x00, 0x00, 0x0A, 0x00, 0x00, 0x00, 0x09 /* TPM_FAIL */
+};
+
+static char dev_description[80];
+
+
+static void *tpm_builtin_main_loop(void *d)
+{
+int res = 1;
+ThreadParams *thr_parms = d;
+uint32_t in_len, out_len;
+uint8_t *in, *out;
+uint32_t resp_size; /* total length of response */
+
+#ifdef DEBUG_TPM
+fprintf(stderr, "tpm: THREAD IS STARTING\n");
+#endif
+
+if (res != 0) {
+#if defined DEBUG_TPM || defined DEBUG_TPM_SR
+fprintf(stderr, "tpm: Error: TPM initialization failed (rc=%d)\n",
+res);
+#endif
+   had_fatal_error = true;
+} else {
+qemu_mutex_lock(&tpm_initialized_mutex);
+
+tpm_initialized = true;
+
+qemu_mutex_unlock(&tpm_initialized_mutex);
+}
+
+/* start command processing */
+while (!thread_terminate) {
+/* receive and handle commands */
+in_len = 0;
+do {
+#ifdef DEBUG_TPM
+fprintf(stderr, "tpm: waiting for commands...\n");
+#endif
+
+if (thread_terminate) {
+break;
+}
+
+qemu_mutex_lock(&thr_parms->tpm_state->state_lock);
+
+/* in case we were to slow and missed the signal, the
+   to_tpm_execute boolean tells us about a pending command */
+if (!thr_parms->tpm_state->to_tpm_execute) {
+qemu_cond_wait(&thr_parms->tpm_state->to_tpm_cond,
+   &thr_parms->tpm_state->state_lock);
+}
+
+thr_parms->tpm_state->to_tpm_execute = false;
+
+qemu_mutex_unlock(&thr_parms->tpm_state->state_lock);
+
+if (thread_terminate) {
+break;
+}
+
+g_locty = thr_parms->tpm_state->command_locty;
+
+in = thr_parms->tpm_state->loc[g_locty].w_buffer.buffer;
+in_len = thr_parms->tpm_state->loc[g_locty].w_offset;
+
+if (!had_fatal_error) {
+
+out_len = thr_parms->tpm_state->loc[g_locty].r_buffer.size;
+
+#ifdef DEBUG_TPM
+fprintf(stderr,
+"tpm: received %d bytes from VM in locality %d\n",
+in_len,
+g_locty);
+dumpBuf

Re: [Qemu-devel] [PATCH 5/6] PPC: Implement e500 (FSL) MMU

[Scott Wood]

On Fri, 6 May 2011 12:01:11 +0200
Alexander Graf  wrote:

> >> +for (i = env->nb_tlbs[0]; i < env->nb_tlb; i++) {
> >> +tlb = &env->tlb[i].tlbe;
> >> +ret = mmubooke_check_tlb(env, tlb, &raddr, &ctx->prot, address, 
> >> rw,
> >> + access_type, i);
> >> +if (!ret) {
> >> +goto found_tlb;
> >> }
> >> }
> >> -if (ret >= 0)
> >> +
> >> +/* check possible TLB0 entries */
> >> +for (i = 0; i < env->nb_ways; i++) {
> >> +tlb = &env->tlb[ea | (i << 7)].tlbe;
> > 
> > Do we have to hardcode 7 (and 0x7f) here?
> 
> I'm not sure - the spec isn't exactly obvious on this part. How do I find out 
> the mask from TLBnCFG?

I don't think the ISA specifies the hash at all.  The implementation manual
should specify it.  I think 7 bits is correct on all e500-family chips so
far, but it's really a function of the TLB geometry.  It may be desireable
in some situations for qemu to create a larger TLB than hardware actually
has, to reduce the TLB misses that the guest has to handle.

> > Better victim selection would check for empty ways, and perhaps keep
> > last_way on a per-set basis.
> 
> Does the hardware do this?

Hmm, I thought it did, but checking the docs it seems to be similar to
what you implemented.  I guess I was thinking of the I/D-cache victim
selection.

> >> +static const target_phys_addr_t e500_tlb_sizes[] = {
> >> +0,
> >> +4 * 1024,
> >> +16 * 1024,
> >> +64 * 1024,
> >> +256 * 1024,
> >> +1024 * 1024,
> >> +4 * 1024 * 1024,
> >> +16 * 1024 * 1024,
> >> +64 * 1024 * 1024,
> >> +256 * 1024 * 1024,
> >> +1024 * 1024 * 1024,
> >> +(target_phys_addr_t)(4ULL * 1024ULL * 1024ULL * 1024ULL),
> >> +};
> > 
> > FWIW, power-of-2 page sizes are architected, and may appear in future
> > e500-family chips.  The TSIZE field is extended by one bit on the right
> > (previously reserved and should-be-zero).
> 
> Yeah, I've seen that mentioned as MAV 2.0 or so change. This is the exact 
> list of supported page sizes for an e500v2 though. Should we just support all 
> of the possible one and ignore the chip's deficiencies?

I'd say just support all of them -- it's undefined behavior if a MAV 1.0
guest sets that low bit, so this is as valid an implementation as any. :-)

> >> +static inline target_phys_addr_t e500_tlb_to_page_size(int size)
> > 
> > unsigned int
> 
> Why?

So you don't have to check size < 0.

> >> +} else {
> >> +rpn = ((uint64_t)env->spr[SPR_BOOKE_MAS7] << 32) |
> >> +  (env->spr[SPR_BOOKE_MAS3] & 0xf000);
> >> +}
> > 
> > Not sure why this is in an else-branch versus msr_gs.
> 
> Because that's what the spec says:
> 
> if category E.HV.LRAT supported & (MSRGS=1) & (MAS1V=1) then
>   rpn <- translate_logical_to_real(MAS7RPNU || MAS3RPNL, MAS8TLPID)
> else if MAS7 implemented then
>   rpn <- MAS7RPNU || MAS3RPNL
> else rpn <- 320 || MAS3RPNL

Sorry, I was thinking of MAS8[TGS], not MSR[GS].

> Yes, I need to think of a good way to generalize NV still. It isn't defined 
> that there's only a single NV available in the architecture either...

Architecturally, the NV algorithm is implementation-defined.

> >> @@ -8433,7 +8505,7 @@ GEN_HANDLER2(icbt_40x, "icbt", 0x1F, 0x06, 0x08, 
> >> 0x03E1, PPC_40x_ICBT),
> >> GEN_HANDLER(iccci, 0x1F, 0x06, 0x1E, 0x0001, PPC_4xx_COMMON),
> >> GEN_HANDLER(icread, 0x1F, 0x06, 0x1F, 0x03E1, PPC_4xx_COMMON),
> >> GEN_HANDLER2(rfci_40x, "rfci", 0x13, 0x13, 0x01, 0x03FF8001, PPC_40x_EXCP),
> >> -GEN_HANDLER(rfci, 0x13, 0x13, 0x01, 0x03FF8001, PPC_BOOKE),
> >> +GEN_HANDLER_E(rfci, 0x13, 0x13, 0x01, 0x03FF8001, PPC_BOOKE, 
> >> PPC2_BOOKE_FSL),
> > 
> > What is PPC2_BOOKE_FSL supposed to indicate?
> > 
> > rfci is basic booke.  It's in the 440.
> 
> It's also in e500, right?

Yes.

> The flags there are a mask. Basically it means "this opcode is available
> on PPC_BOOKE and PPC2_BOOKE_FSL capable CPUs".

OK, it looked like it was being limited to only FSL.  I missed that
PPC_BOOKE and PPC2_BOOKE_FSL are the same "kind" of flags despite being in
different words.  Does PPC_BOOKE not refer to things that are common to all
booke?  Why do the individual implementations of booke need to be listed?

-Scott

Re: [Qemu-devel] qemu.org down

[Anthony Liguori]

On 05/06/2011 08:40 AM, Anthony Liguori wrote:

The hosting company qemu.org uses is experiencing an outage this
morning. I'll keep the list up to date as I learn more.


And we're back online.

Sorry for any inconvenience.

Regards,

Anthony Liguori


Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 5/6] PPC: Implement e500 (FSL) MMU

[Alexander Graf]

On 06.05.2011, at 19:40, Scott Wood wrote:

> On Fri, 6 May 2011 12:01:11 +0200
> Alexander Graf  wrote:
> 
 +for (i = env->nb_tlbs[0]; i < env->nb_tlb; i++) {
 +tlb = &env->tlb[i].tlbe;
 +ret = mmubooke_check_tlb(env, tlb, &raddr, &ctx->prot, address, 
 rw,
 + access_type, i);
 +if (!ret) {
 +goto found_tlb;
}
}
 -if (ret >= 0)
 +
 +/* check possible TLB0 entries */
 +for (i = 0; i < env->nb_ways; i++) {
 +tlb = &env->tlb[ea | (i << 7)].tlbe;
>>> 
>>> Do we have to hardcode 7 (and 0x7f) here?
>> 
>> I'm not sure - the spec isn't exactly obvious on this part. How do I find 
>> out the mask from TLBnCFG?
> 
> I don't think the ISA specifies the hash at all.  The implementation manual
> should specify it.  I think 7 bits is correct on all e500-family chips so
> far, but it's really a function of the TLB geometry.  It may be desireable
> in some situations for qemu to create a larger TLB than hardware actually
> has, to reduce the TLB misses that the guest has to handle.

Yup, changed that one :).

> 
>>> Better victim selection would check for empty ways, and perhaps keep
>>> last_way on a per-set basis.
>> 
>> Does the hardware do this?
> 
> Hmm, I thought it did, but checking the docs it seems to be similar to
> what you implemented.  I guess I was thinking of the I/D-cache victim
> selection.
> 
 +static const target_phys_addr_t e500_tlb_sizes[] = {
 +0,
 +4 * 1024,
 +16 * 1024,
 +64 * 1024,
 +256 * 1024,
 +1024 * 1024,
 +4 * 1024 * 1024,
 +16 * 1024 * 1024,
 +64 * 1024 * 1024,
 +256 * 1024 * 1024,
 +1024 * 1024 * 1024,
 +(target_phys_addr_t)(4ULL * 1024ULL * 1024ULL * 1024ULL),
 +};
>>> 
>>> FWIW, power-of-2 page sizes are architected, and may appear in future
>>> e500-family chips.  The TSIZE field is extended by one bit on the right
>>> (previously reserved and should-be-zero).
>> 
>> Yeah, I've seen that mentioned as MAV 2.0 or so change. This is the exact 
>> list of supported page sizes for an e500v2 though. Should we just support 
>> all of the possible one and ignore the chip's deficiencies?
> 
> I'd say just support all of them -- it's undefined behavior if a MAV 1.0
> guest sets that low bit, so this is as valid an implementation as any. :-)

I have a nice algorithm for MAV 1.0 now - when 2.0 comes along we can check 
again and see what fits.

> 
 +static inline target_phys_addr_t e500_tlb_to_page_size(int size)
>>> 
>>> unsigned int
>> 
>> Why?
> 
> So you don't have to check size < 0.

Ah, I see. That one's moot by now since we don't look up things in an array 
anymore.

> 
 +} else {
 +rpn = ((uint64_t)env->spr[SPR_BOOKE_MAS7] << 32) |
 +  (env->spr[SPR_BOOKE_MAS3] & 0xf000);
 +}
>>> 
>>> Not sure why this is in an else-branch versus msr_gs.
>> 
>> Because that's what the spec says:
>> 
>> if category E.HV.LRAT supported & (MSRGS=1) & (MAS1V=1) then
>>  rpn <- translate_logical_to_real(MAS7RPNU || MAS3RPNL, MAS8TLPID)
>> else if MAS7 implemented then
>>  rpn <- MAS7RPNU || MAS3RPNL
>> else rpn <- 320 || MAS3RPNL
> 
> Sorry, I was thinking of MAS8[TGS], not MSR[GS].
> 
>> Yes, I need to think of a good way to generalize NV still. It isn't defined 
>> that there's only a single NV available in the architecture either...
> 
> Architecturally, the NV algorithm is implementation-defined.

Meh :).

> 
 @@ -8433,7 +8505,7 @@ GEN_HANDLER2(icbt_40x, "icbt", 0x1F, 0x06, 0x08, 
 0x03E1, PPC_40x_ICBT),
 GEN_HANDLER(iccci, 0x1F, 0x06, 0x1E, 0x0001, PPC_4xx_COMMON),
 GEN_HANDLER(icread, 0x1F, 0x06, 0x1F, 0x03E1, PPC_4xx_COMMON),
 GEN_HANDLER2(rfci_40x, "rfci", 0x13, 0x13, 0x01, 0x03FF8001, PPC_40x_EXCP),
 -GEN_HANDLER(rfci, 0x13, 0x13, 0x01, 0x03FF8001, PPC_BOOKE),
 +GEN_HANDLER_E(rfci, 0x13, 0x13, 0x01, 0x03FF8001, PPC_BOOKE, 
 PPC2_BOOKE_FSL),
>>> 
>>> What is PPC2_BOOKE_FSL supposed to indicate?
>>> 
>>> rfci is basic booke.  It's in the 440.
>> 
>> It's also in e500, right?
> 
> Yes.
> 
>> The flags there are a mask. Basically it means "this opcode is available
>> on PPC_BOOKE and PPC2_BOOKE_FSL capable CPUs".
> 
> OK, it looked like it was being limited to only FSL.  I missed that
> PPC_BOOKE and PPC2_BOOKE_FSL are the same "kind" of flags despite being in
> different words.  Does PPC_BOOKE not refer to things that are common to all
> booke?  Why do the individual implementations of booke need to be listed?

Well, the PPC_BOOKE flag was also used for the 440 tlb modification opcodes, 
which are different for 2.06. So I just introduced a new one :). We could of 
course also introduce yet another flag for 440s and define specific 
instructions individually, but I don't really see it buying us too much for now.


Alex

[Qemu-devel] Qemu + RBD = ceph::buffer::end_of_buffer

[Dyweni - Qemu-Devel]

Hi List!

I am tripping across this error as soon as the qemu rbd disk is
probed by the windows 2000 installer:

VNC server running on `127.0.0.1:5900'
terminate called after throwing an instance of 'ceph::buffer::end_of_buffer'
  what():  buffer::end_of_buffer
Aborted (core dumped)



Has anyone else tripped across this?



I am running the following:
  Linux Kernel 2.6.37-gentoo-r4
  Ceph 0.27
  QEMU-KVM (commit 28262112181f27f302b5186f0df6428df6b513e7)
Pulled from: git://git.kernel.org/pub/scm/virt/kvm/qemu-kvm.git
And patched with Josh Durgin's "rbd improvements" patchset
 (http://lists.gnu.org/archive/html/qemu-devel/2011-04/msg01211.html)



The GDB Backtrace is:

#0  0x7f7733c2a495 in raise (sig=) at
../nptl/sysdeps/unix/sysv/linux/raise.c:64
#1  0x7f7733c2b81f in abort () at abort.c:92
#2  0x7f7733175a25 in __gnu_cxx::__verbose_terminate_handler () at
/usr/src/debug/sys-devel/gcc-4.4.5/gcc-4.4.5/libstdc++-v3/libsupc++/vterminate.cc:93
#3  0x7f7733172c64 in __cxxabiv1::__terminate (handler=0x7f7733175817
<__gnu_cxx::__verbose_terminate_handler()>)
at
/usr/src/debug/sys-devel/gcc-4.4.5/gcc-4.4.5/libstdc++-v3/libsupc++/eh_terminate.cc:38
#4  0x7f7733172c8c in std::terminate () at
/usr/src/debug/sys-devel/gcc-4.4.5/gcc-4.4.5/libstdc++-v3/libsupc++/eh_terminate.cc:48
#5  0x7f7733172ea4 in __cxxabiv1::__cxa_throw (obj=0x18d62a0,
tinfo=0x7f7735d57ce0, dest=0x7f7735b4548a
)
at
/usr/src/debug/sys-devel/gcc-4.4.5/gcc-4.4.5/libstdc++-v3/libsupc++/eh_throw.cc:83
#6  0x7f7735b45b31 in ceph::buffer::list::iterator::copy
(this=0x7f7731cb2930, len=4, dest=0x7f7731cb28dc "") at
include/buffer.h:381
#7  0x7f7735b483de in decode_raw<__le32> (t=@0x7f7731cb28dc, p=...) at
include/encoding.h:35
#8  0x7f7735b46a40 in decode (v=@0x7f7731cb290c, p=...) at
include/encoding.h:80
#9  0x7f7735b46b94 in decode (s=..., p=...) at include/encoding.h:189
#10 0x7f77356ea3b5 in
librados::RadosClient::C_aio_sparse_read_Ack::finish (this=0x7f77280618d0,
r=0) at librados.cc:463
#11 0x7f773572dff4 in Objecter::handle_osd_op_reply (this=0x181ff10,
m=0x18f1440) at osdc/Objecter.cc:801
#12 0x7f77356d347c in librados::RadosClient::_dispatch
(this=0x181cdc0, m=0x18f1440) at librados.cc:751
#13 0x7f77356d327c in librados::RadosClient::ms_dispatch
(this=0x181cdc0, m=0x18f1440) at librados.cc:717
#14 0x7f773571d93d in Messenger::ms_deliver_dispatch (this=0x181f300,
m=0x18f1440) at msg/Messenger.h:98
#15 0x7f773570b135 in SimpleMessenger::dispatch_entry (this=0x181f300)
at msg/SimpleMessenger.cc:352
#16 0x7f77356e49ba in SimpleMessenger::DispatchThread::entry
(this=0x181f790) at msg/SimpleMessenger.h:533
#17 0x7f773571c75d in Thread::_entry_func (arg=0x181f790) at
common/Thread.h:41
#18 0x7f7736372ac4 in start_thread (arg=) at
pthread_create.c:297
#19 0x7f7733cc938d in clone () at
../sysdeps/unix/sysv/linux/x86_64/clone.S:115



Thanks,
Dyweni

Re: [Qemu-devel] [PATCH 5/6] PPC: Implement e500 (FSL) MMU

[Scott Wood]

On Fri, 6 May 2011 20:33:24 +0200
Alexander Graf  wrote:

> On 06.05.2011, at 19:40, Scott Wood wrote:
> 
> > OK, it looked like it was being limited to only FSL.  I missed that
> > PPC_BOOKE and PPC2_BOOKE_FSL are the same "kind" of flags despite being in
> > different words.  Does PPC_BOOKE not refer to things that are common to all
> > booke?  Why do the individual implementations of booke need to be listed?
> 
> Well, the PPC_BOOKE flag was also used for the 440 tlb modification opcodes, 
> which are different for 2.06. So I just introduced a new one :). We could of 
> course also introduce yet another flag for 440s and define specific 
> instructions individually, but I don't really see it buying us too much for 
> now.

It would help reduce confusion if PPC_BOOKE really meant booke, and not
440. :-)

-Scott

[Qemu-devel] [PATCH] virtio-9p: Fix a memory leak

[Pedro Scarapicchia Junior]

At v9fs_walk_complete(), the memory allocated at v9fs_walk() is not being
released leading system to crash due out of memory.

This patch releases structure V9fsWalkState after v9fs_walk is complete.

Signed-off-by: Pedro Scarapicchia Junior 
---
 hw/9pfs/virtio-9p.c |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
index b5fc52b..b1da2b7 100644
--- a/hw/9pfs/virtio-9p.c
+++ b/hw/9pfs/virtio-9p.c
@@ -1490,6 +1490,8 @@ static void v9fs_walk_complete(V9fsState *s, 
V9fsWalkState *vs, int err)
 qemu_free(vs->wnames);
 qemu_free(vs->qids);
 }
+
+qemu_free(vs);
 }
 
 static void v9fs_walk_marshal(V9fsWalkState *vs)
-- 
1.7.1

Re: [Qemu-devel] Qemu + RBD = ceph::buffer::end_of_buffer

[Dyweni - Qemu-Devel]

Hi List!

I upgraded Ceph to the latest development version
Commit: 0edbc75a5fe8c3028faf85546f3264d28653ea3f
Pulled from: git://ceph.newdream.net/ceph.git

I recompiled the latest GIT version of QEMU-KVM (with Josh Durgin's
patches) against the latest git version of Ceph.

However, this error is still occurring:

terminate called after throwing an instance of 'ceph::buffer::end_of_buffer'
  what():  buffer::end_of_buffer
Aborted (core dumped)



Here's another backtrace from GDB:

#0  0x7f16ff829495 in raise (sig=) at
../nptl/sysdeps/unix/sysv/linux/raise.c:64
#1  0x7f16ff82a81f in abort () at abort.c:92
#2  0x7f16fed74a25 in __gnu_cxx::__verbose_terminate_handler () at
/usr/src/debug/sys-devel/gcc-4.4.5/gcc-4.4.5/libstdc++-v3/libsupc++/vterminate.cc:93
#3  0x7f16fed71c64 in __cxxabiv1::__terminate (handler=0x7f16fed74817
<__gnu_cxx::__verbose_terminate_handler()>)
at
/usr/src/debug/sys-devel/gcc-4.4.5/gcc-4.4.5/libstdc++-v3/libsupc++/eh_terminate.cc:38
#4  0x7f16fed71c8c in std::terminate () at
/usr/src/debug/sys-devel/gcc-4.4.5/gcc-4.4.5/libstdc++-v3/libsupc++/eh_terminate.cc:48
#5  0x7f16fed71ea4 in __cxxabiv1::__cxa_throw (obj=0x1346470,
tinfo=0x7f1701952ce0, dest=0x7f17017403d4
)
at
/usr/src/debug/sys-devel/gcc-4.4.5/gcc-4.4.5/libstdc++-v3/libsupc++/eh_throw.cc:83
#6  0x7f1701740a7b in ceph::buffer::list::iterator::copy
(this=0x7f16fd8b1930, len=4, dest=0x7f16fd8b18dc "") at
include/buffer.h:379
#7  0x7f1701743328 in decode_raw<__le32> (t=@0x7f16fd8b18dc, p=...) at
include/encoding.h:35
#8  0x7f170174198a in decode (v=@0x7f16fd8b190c, p=...) at
include/encoding.h:80
#9  0x7f1701741ade in decode (s=..., p=...) at include/encoding.h:189
#10 0x7f17012e8369 in
librados::RadosClient::C_aio_sparse_read_Ack::finish (this=0x7f16f40d6200,
r=0) at librados.cc:463
#11 0x7f170132bb5a in Objecter::handle_osd_op_reply (this=0x13423e0,
m=0x1346520) at osdc/Objecter.cc:794
#12 0x7f17012d1444 in librados::RadosClient::_dispatch
(this=0x133f810, m=0x1346520) at librados.cc:751
#13 0x7f17012d1244 in librados::RadosClient::ms_dispatch
(this=0x133f810, m=0x1346520) at librados.cc:717
#14 0x7f170131b57b in Messenger::ms_deliver_dispatch (this=0x1341910,
m=0x1346520) at msg/Messenger.h:98
#15 0x7f17013090d3 in SimpleMessenger::dispatch_entry (this=0x1341910)
at msg/SimpleMessenger.cc:352
#16 0x7f17012e296e in SimpleMessenger::DispatchThread::entry
(this=0x1341da0) at msg/SimpleMessenger.h:533
#17 0x7f170131a39b in Thread::_entry_func (arg=0x1341da0) at
common/Thread.h:41
#18 0x7f1701f6dac4 in start_thread (arg=) at
pthread_create.c:297
#19 0x7f16ff8c838d in clone () at
../sysdeps/unix/sysv/linux/x86_64/clone.S:115


Thanks,
Dyweni




> Hi List!
>
> I am tripping across this error as soon as the qemu rbd disk is
> probed by the windows 2000 installer:
>
> VNC server running on `127.0.0.1:5900'
> terminate called after throwing an instance of
> 'ceph::buffer::end_of_buffer'
>   what():  buffer::end_of_buffer
> Aborted (core dumped)
>
>
>
> Has anyone else tripped across this?
>
>
>
> I am running the following:
>   Linux Kernel 2.6.37-gentoo-r4
>   Ceph 0.27
>   QEMU-KVM (commit 28262112181f27f302b5186f0df6428df6b513e7)
> Pulled from: git://git.kernel.org/pub/scm/virt/kvm/qemu-kvm.git
> And patched with Josh Durgin's "rbd improvements" patchset
>  (http://lists.gnu.org/archive/html/qemu-devel/2011-04/msg01211.html)
>
>
>
> The GDB Backtrace is:
>
> #0  0x7f7733c2a495 in raise (sig=) at
> ../nptl/sysdeps/unix/sysv/linux/raise.c:64
> #1  0x7f7733c2b81f in abort () at abort.c:92
> #2  0x7f7733175a25 in __gnu_cxx::__verbose_terminate_handler () at
> /usr/src/debug/sys-devel/gcc-4.4.5/gcc-4.4.5/libstdc++-v3/libsupc++/vterminate.cc:93
> #3  0x7f7733172c64 in __cxxabiv1::__terminate (handler=0x7f7733175817
> <__gnu_cxx::__verbose_terminate_handler()>)
> at
> /usr/src/debug/sys-devel/gcc-4.4.5/gcc-4.4.5/libstdc++-v3/libsupc++/eh_terminate.cc:38
> #4  0x7f7733172c8c in std::terminate () at
> /usr/src/debug/sys-devel/gcc-4.4.5/gcc-4.4.5/libstdc++-v3/libsupc++/eh_terminate.cc:48
> #5  0x7f7733172ea4 in __cxxabiv1::__cxa_throw (obj=0x18d62a0,
> tinfo=0x7f7735d57ce0, dest=0x7f7735b4548a
> )
> at
> /usr/src/debug/sys-devel/gcc-4.4.5/gcc-4.4.5/libstdc++-v3/libsupc++/eh_throw.cc:83
> #6  0x7f7735b45b31 in ceph::buffer::list::iterator::copy
> (this=0x7f7731cb2930, len=4, dest=0x7f7731cb28dc "") at
> include/buffer.h:381
> #7  0x7f7735b483de in decode_raw<__le32> (t=@0x7f7731cb28dc, p=...) at
> include/encoding.h:35
> #8  0x7f7735b46a40 in decode (v=@0x7f7731cb290c, p=...) at
> include/encoding.h:80
> #9  0x7f7735b46b94 in decode (s=..., p=...) at include/encoding.h:189
> #10 0x7f77356ea3b5 in
> librados::RadosClient::C_aio_sparse_read_Ack::finish (this=0x7f77280618d0,
> r=0) at librados.cc:463
> #11 0x7f773572dff4 in Objecter::handle_osd_op_reply (this=0x181ff10,
> m=0x18f

Re: [Qemu-devel] Qemu + RBD = ceph::buffer::end_of_buffer

[Josh Durgin]

CCing the ceph list.

On 05/06/2011 12:23 PM, Dyweni - Qemu-Devel wrote:

Hi List!

I upgraded Ceph to the latest development version
Commit: 0edbc75a5fe8c3028faf85546f3264d28653ea3f
Pulled from: git://ceph.newdream.net/ceph.git

I recompiled the latest GIT version of QEMU-KVM (with Josh Durgin's
patches) against the latest git version of Ceph.

However, this error is still occurring:

terminate called after throwing an instance of 'ceph::buffer::end_of_buffer'
   what():  buffer::end_of_buffer
Aborted (core dumped)



Here's another backtrace from GDB:

#0  0x7f16ff829495 in raise (sig=) at
../nptl/sysdeps/unix/sysv/linux/raise.c:64
#1  0x7f16ff82a81f in abort () at abort.c:92
#2  0x7f16fed74a25 in __gnu_cxx::__verbose_terminate_handler () at
/usr/src/debug/sys-devel/gcc-4.4.5/gcc-4.4.5/libstdc++-v3/libsupc++/vterminate.cc:93
#3  0x7f16fed71c64 in __cxxabiv1::__terminate (handler=0x7f16fed74817
<__gnu_cxx::__verbose_terminate_handler()>)
 at
/usr/src/debug/sys-devel/gcc-4.4.5/gcc-4.4.5/libstdc++-v3/libsupc++/eh_terminate.cc:38
#4  0x7f16fed71c8c in std::terminate () at
/usr/src/debug/sys-devel/gcc-4.4.5/gcc-4.4.5/libstdc++-v3/libsupc++/eh_terminate.cc:48
#5  0x7f16fed71ea4 in __cxxabiv1::__cxa_throw (obj=0x1346470,
tinfo=0x7f1701952ce0, dest=0x7f17017403d4
)
 at
/usr/src/debug/sys-devel/gcc-4.4.5/gcc-4.4.5/libstdc++-v3/libsupc++/eh_throw.cc:83
#6  0x7f1701740a7b in ceph::buffer::list::iterator::copy
(this=0x7f16fd8b1930, len=4, dest=0x7f16fd8b18dc "") at
include/buffer.h:379
#7  0x7f1701743328 in decode_raw<__le32>  (t=@0x7f16fd8b18dc, p=...) at
include/encoding.h:35
#8  0x7f170174198a in decode (v=@0x7f16fd8b190c, p=...) at
include/encoding.h:80
#9  0x7f1701741ade in decode (s=..., p=...) at include/encoding.h:189
#10 0x7f17012e8369 in
librados::RadosClient::C_aio_sparse_read_Ack::finish (this=0x7f16f40d6200,
r=0) at librados.cc:463
#11 0x7f170132bb5a in Objecter::handle_osd_op_reply (this=0x13423e0,
m=0x1346520) at osdc/Objecter.cc:794
#12 0x7f17012d1444 in librados::RadosClient::_dispatch
(this=0x133f810, m=0x1346520) at librados.cc:751
#13 0x7f17012d1244 in librados::RadosClient::ms_dispatch
(this=0x133f810, m=0x1346520) at librados.cc:717
#14 0x7f170131b57b in Messenger::ms_deliver_dispatch (this=0x1341910,
m=0x1346520) at msg/Messenger.h:98
#15 0x7f17013090d3 in SimpleMessenger::dispatch_entry (this=0x1341910)
at msg/SimpleMessenger.cc:352
#16 0x7f17012e296e in SimpleMessenger::DispatchThread::entry
(this=0x1341da0) at msg/SimpleMessenger.h:533
#17 0x7f170131a39b in Thread::_entry_func (arg=0x1341da0) at
common/Thread.h:41
#18 0x7f1701f6dac4 in start_thread (arg=) at
pthread_create.c:297
#19 0x7f16ff8c838d in clone () at
../sysdeps/unix/sysv/linux/x86_64/clone.S:115


I haven't seen that error before, but it's probably a bug in the OSD 
where it doesn't set an error code. If you've still got the core file, 
could you go to frame 10 and send us the values of r, bl._len, and iter.off?


Thanks,
Josh

Re: [Qemu-devel] Qemu + RBD = ceph::buffer::end_of_buffer

[Dyweni - Qemu-Devel]

Hi Josh/Lists!

463 ::decode(*data_bl, iter);
(gdb) print r
$1 = 0
(gdb) print data_bl
$2 = (ceph::bufferlist *) 0x7f16f40d6060
(gdb) print data_bl->_len
$3 = 0
(gdb) print iter->off
$4 = 20


Thanks,
Dyweni



> CCing the ceph list.
>
> On 05/06/2011 12:23 PM, Dyweni - Qemu-Devel wrote:
>> Hi List!
>>
>> I upgraded Ceph to the latest development version
>>  Commit: 0edbc75a5fe8c3028faf85546f3264d28653ea3f
>>  Pulled from: git://ceph.newdream.net/ceph.git
>>
>> I recompiled the latest GIT version of QEMU-KVM (with Josh Durgin's
>> patches) against the latest git version of Ceph.
>>
>> However, this error is still occurring:
>>
>> terminate called after throwing an instance of
>> 'ceph::buffer::end_of_buffer'
>>what():  buffer::end_of_buffer
>> Aborted (core dumped)
>>
>>
>>
>> Here's another backtrace from GDB:
>>
>> #0  0x7f16ff829495 in raise (sig=) at
>> ../nptl/sysdeps/unix/sysv/linux/raise.c:64
>> #1  0x7f16ff82a81f in abort () at abort.c:92
>> #2  0x7f16fed74a25 in __gnu_cxx::__verbose_terminate_handler () at
>> /usr/src/debug/sys-devel/gcc-4.4.5/gcc-4.4.5/libstdc++-v3/libsupc++/vterminate.cc:93
>> #3  0x7f16fed71c64 in __cxxabiv1::__terminate
>> (handler=0x7f16fed74817
>> <__gnu_cxx::__verbose_terminate_handler()>)
>>  at
>> /usr/src/debug/sys-devel/gcc-4.4.5/gcc-4.4.5/libstdc++-v3/libsupc++/eh_terminate.cc:38
>> #4  0x7f16fed71c8c in std::terminate () at
>> /usr/src/debug/sys-devel/gcc-4.4.5/gcc-4.4.5/libstdc++-v3/libsupc++/eh_terminate.cc:48
>> #5  0x7f16fed71ea4 in __cxxabiv1::__cxa_throw (obj=0x1346470,
>> tinfo=0x7f1701952ce0, dest=0x7f17017403d4
>> )
>>  at
>> /usr/src/debug/sys-devel/gcc-4.4.5/gcc-4.4.5/libstdc++-v3/libsupc++/eh_throw.cc:83
>> #6  0x7f1701740a7b in ceph::buffer::list::iterator::copy
>> (this=0x7f16fd8b1930, len=4, dest=0x7f16fd8b18dc "") at
>> include/buffer.h:379
>> #7  0x7f1701743328 in decode_raw<__le32>  (t=@0x7f16fd8b18dc, p=...)
>> at
>> include/encoding.h:35
>> #8  0x7f170174198a in decode (v=@0x7f16fd8b190c, p=...) at
>> include/encoding.h:80
>> #9  0x7f1701741ade in decode (s=..., p=...) at
>> include/encoding.h:189
>> #10 0x7f17012e8369 in
>> librados::RadosClient::C_aio_sparse_read_Ack::finish
>> (this=0x7f16f40d6200,
>> r=0) at librados.cc:463
>> #11 0x7f170132bb5a in Objecter::handle_osd_op_reply (this=0x13423e0,
>> m=0x1346520) at osdc/Objecter.cc:794
>> #12 0x7f17012d1444 in librados::RadosClient::_dispatch
>> (this=0x133f810, m=0x1346520) at librados.cc:751
>> #13 0x7f17012d1244 in librados::RadosClient::ms_dispatch
>> (this=0x133f810, m=0x1346520) at librados.cc:717
>> #14 0x7f170131b57b in Messenger::ms_deliver_dispatch
>> (this=0x1341910,
>> m=0x1346520) at msg/Messenger.h:98
>> #15 0x7f17013090d3 in SimpleMessenger::dispatch_entry
>> (this=0x1341910)
>> at msg/SimpleMessenger.cc:352
>> #16 0x7f17012e296e in SimpleMessenger::DispatchThread::entry
>> (this=0x1341da0) at msg/SimpleMessenger.h:533
>> #17 0x7f170131a39b in Thread::_entry_func (arg=0x1341da0) at
>> common/Thread.h:41
>> #18 0x7f1701f6dac4 in start_thread (arg=) at
>> pthread_create.c:297
>> #19 0x7f16ff8c838d in clone () at
>> ../sysdeps/unix/sysv/linux/x86_64/clone.S:115
>
> I haven't seen that error before, but it's probably a bug in the OSD
> where it doesn't set an error code. If you've still got the core file,
> could you go to frame 10 and send us the values of r, bl._len, and
> iter.off?
>
> Thanks,
> Josh
>

Re: [Qemu-devel] Qemu + RBD = ceph::buffer::end_of_buffer

[Sage Weil]

On Fri, 6 May 2011, Dyweni - Qemu-Devel wrote:
> Hi Josh/Lists!
> 
> 463 ::decode(*data_bl, iter);
> (gdb) print r
> $1 = 0
> (gdb) print data_bl
> $2 = (ceph::bufferlist *) 0x7f16f40d6060
> (gdb) print data_bl->_len
> $3 = 0

What about c->bl._len?

sage


> (gdb) print iter->off
> $4 = 20
> 
> 
> Thanks,
> Dyweni
> 
> 
> 
> > CCing the ceph list.
> >
> > On 05/06/2011 12:23 PM, Dyweni - Qemu-Devel wrote:
> >> Hi List!
> >>
> >> I upgraded Ceph to the latest development version
> >>Commit: 0edbc75a5fe8c3028faf85546f3264d28653ea3f
> >>Pulled from: git://ceph.newdream.net/ceph.git
> >>
> >> I recompiled the latest GIT version of QEMU-KVM (with Josh Durgin's
> >> patches) against the latest git version of Ceph.
> >>
> >> However, this error is still occurring:
> >>
> >> terminate called after throwing an instance of
> >> 'ceph::buffer::end_of_buffer'
> >>what():  buffer::end_of_buffer
> >> Aborted (core dumped)
> >>
> >>
> >>
> >> Here's another backtrace from GDB:
> >>
> >> #0  0x7f16ff829495 in raise (sig=) at
> >> ../nptl/sysdeps/unix/sysv/linux/raise.c:64
> >> #1  0x7f16ff82a81f in abort () at abort.c:92
> >> #2  0x7f16fed74a25 in __gnu_cxx::__verbose_terminate_handler () at
> >> /usr/src/debug/sys-devel/gcc-4.4.5/gcc-4.4.5/libstdc++-v3/libsupc++/vterminate.cc:93
> >> #3  0x7f16fed71c64 in __cxxabiv1::__terminate
> >> (handler=0x7f16fed74817
> >> <__gnu_cxx::__verbose_terminate_handler()>)
> >>  at
> >> /usr/src/debug/sys-devel/gcc-4.4.5/gcc-4.4.5/libstdc++-v3/libsupc++/eh_terminate.cc:38
> >> #4  0x7f16fed71c8c in std::terminate () at
> >> /usr/src/debug/sys-devel/gcc-4.4.5/gcc-4.4.5/libstdc++-v3/libsupc++/eh_terminate.cc:48
> >> #5  0x7f16fed71ea4 in __cxxabiv1::__cxa_throw (obj=0x1346470,
> >> tinfo=0x7f1701952ce0, dest=0x7f17017403d4
> >> )
> >>  at
> >> /usr/src/debug/sys-devel/gcc-4.4.5/gcc-4.4.5/libstdc++-v3/libsupc++/eh_throw.cc:83
> >> #6  0x7f1701740a7b in ceph::buffer::list::iterator::copy
> >> (this=0x7f16fd8b1930, len=4, dest=0x7f16fd8b18dc "") at
> >> include/buffer.h:379
> >> #7  0x7f1701743328 in decode_raw<__le32>  (t=@0x7f16fd8b18dc, p=...)
> >> at
> >> include/encoding.h:35
> >> #8  0x7f170174198a in decode (v=@0x7f16fd8b190c, p=...) at
> >> include/encoding.h:80
> >> #9  0x7f1701741ade in decode (s=..., p=...) at
> >> include/encoding.h:189
> >> #10 0x7f17012e8369 in
> >> librados::RadosClient::C_aio_sparse_read_Ack::finish
> >> (this=0x7f16f40d6200,
> >> r=0) at librados.cc:463
> >> #11 0x7f170132bb5a in Objecter::handle_osd_op_reply (this=0x13423e0,
> >> m=0x1346520) at osdc/Objecter.cc:794
> >> #12 0x7f17012d1444 in librados::RadosClient::_dispatch
> >> (this=0x133f810, m=0x1346520) at librados.cc:751
> >> #13 0x7f17012d1244 in librados::RadosClient::ms_dispatch
> >> (this=0x133f810, m=0x1346520) at librados.cc:717
> >> #14 0x7f170131b57b in Messenger::ms_deliver_dispatch
> >> (this=0x1341910,
> >> m=0x1346520) at msg/Messenger.h:98
> >> #15 0x7f17013090d3 in SimpleMessenger::dispatch_entry
> >> (this=0x1341910)
> >> at msg/SimpleMessenger.cc:352
> >> #16 0x7f17012e296e in SimpleMessenger::DispatchThread::entry
> >> (this=0x1341da0) at msg/SimpleMessenger.h:533
> >> #17 0x7f170131a39b in Thread::_entry_func (arg=0x1341da0) at
> >> common/Thread.h:41
> >> #18 0x7f1701f6dac4 in start_thread (arg=) at
> >> pthread_create.c:297
> >> #19 0x7f16ff8c838d in clone () at
> >> ../sysdeps/unix/sysv/linux/x86_64/clone.S:115
> >
> > I haven't seen that error before, but it's probably a bug in the OSD
> > where it doesn't set an error code. If you've still got the core file,
> > could you go to frame 10 and send us the values of r, bl._len, and
> > iter.off?
> >
> > Thanks,
> > Josh
> >
> 
> 
> 
> 

Re: [Qemu-devel] Qemu + RBD = ceph::buffer::end_of_buffer

[Dyweni - Qemu-Devel]

Hi Sage/Lists!


(gdb) print c->bl._len
$1 = 20


And in case this is helpful:

(gdb) print *c
$2 = {lock = {name = 0x7f1701430f8d "AioCompletionImpl lock", id = -1,
recursive = false, lockdep = true, backtrace = false, _m = {__data =
{__lock = 1, __count = 0,
__owner = 25800, __nusers = 1, __kind = 2, __spins = 0, __list =
{__prev = 0x0, __next = 0x0}},
  __size =
"\001\000\000\000\000\000\000\000\310d\000\000\001\000\000\000\002",
'\000' , __align = 1}, nlock = 1}, cond = {
_vptr.Cond = 0x7f1701952bd0, _c = {__data = {__lock = 0, __futex = 0,
__total_seq = 0, __wakeup_seq = 0, __woken_seq = 0, __mutex = 0x0,
__nwaiters = 0,
__broadcast_seq = 0}, __size = '\000' , __align
= 0}}, ref = 1, rval = 0, released = true, ack = true, safe =
false, objver = {version = 0,
epoch = 0, __pad = 0}, callback_complete = 0x7f170173de33
,
  callback_safe = 0x7f170173d8bd , callback_arg = 0x7f16f40d6010, bl = {
_buffers = { >> = {
_M_impl = { >> =
{<__gnu_cxx::new_allocator >> =
{}, }, _M_node = {_M_next =
0x1350530, _M_prev = 0x1350530}}}, }, _len = 20,
append_buffer = {_raw = 0x0, _off = 0, _len = 0}, last_p = {
  bl = 0x7f16f40d6170, ls = 0x7f16f40d6170, off = 0, p = {_M_node =
0x7f16f40d6170}, p_off = 0}}, pbl = 0x0, buf = 0x0, maxlen = 0}



Thanks,
Dyweni




> On Fri, 6 May 2011, Dyweni - Qemu-Devel wrote:
>> Hi Josh/Lists!
>>
>> 463 ::decode(*data_bl, iter);
>> (gdb) print r
>> $1 = 0
>> (gdb) print data_bl
>> $2 = (ceph::bufferlist *) 0x7f16f40d6060
>> (gdb) print data_bl->_len
>> $3 = 0
>
> What about c->bl._len?
>
> sage
>
>
>> (gdb) print iter->off
>> $4 = 20
>>
>>
>> Thanks,
>> Dyweni
>>
>>
>>
>> > CCing the ceph list.
>> >
>> > On 05/06/2011 12:23 PM, Dyweni - Qemu-Devel wrote:
>> >> Hi List!
>> >>
>> >> I upgraded Ceph to the latest development version
>> >>   Commit: 0edbc75a5fe8c3028faf85546f3264d28653ea3f
>> >>   Pulled from: git://ceph.newdream.net/ceph.git
>> >>
>> >> I recompiled the latest GIT version of QEMU-KVM (with Josh Durgin's
>> >> patches) against the latest git version of Ceph.
>> >>
>> >> However, this error is still occurring:
>> >>
>> >> terminate called after throwing an instance of
>> >> 'ceph::buffer::end_of_buffer'
>> >>what():  buffer::end_of_buffer
>> >> Aborted (core dumped)
>> >>
>> >>
>> >>
>> >> Here's another backtrace from GDB:
>> >>
>> >> #0  0x7f16ff829495 in raise (sig=) at
>> >> ../nptl/sysdeps/unix/sysv/linux/raise.c:64
>> >> #1  0x7f16ff82a81f in abort () at abort.c:92
>> >> #2  0x7f16fed74a25 in __gnu_cxx::__verbose_terminate_handler ()
>> at
>> >> /usr/src/debug/sys-devel/gcc-4.4.5/gcc-4.4.5/libstdc++-v3/libsupc++/vterminate.cc:93
>> >> #3  0x7f16fed71c64 in __cxxabiv1::__terminate
>> >> (handler=0x7f16fed74817
>> >> <__gnu_cxx::__verbose_terminate_handler()>)
>> >>  at
>> >> /usr/src/debug/sys-devel/gcc-4.4.5/gcc-4.4.5/libstdc++-v3/libsupc++/eh_terminate.cc:38
>> >> #4  0x7f16fed71c8c in std::terminate () at
>> >> /usr/src/debug/sys-devel/gcc-4.4.5/gcc-4.4.5/libstdc++-v3/libsupc++/eh_terminate.cc:48
>> >> #5  0x7f16fed71ea4 in __cxxabiv1::__cxa_throw (obj=0x1346470,
>> >> tinfo=0x7f1701952ce0, dest=0x7f17017403d4
>> >> )
>> >>  at
>> >> /usr/src/debug/sys-devel/gcc-4.4.5/gcc-4.4.5/libstdc++-v3/libsupc++/eh_throw.cc:83
>> >> #6  0x7f1701740a7b in ceph::buffer::list::iterator::copy
>> >> (this=0x7f16fd8b1930, len=4, dest=0x7f16fd8b18dc "") at
>> >> include/buffer.h:379
>> >> #7  0x7f1701743328 in decode_raw<__le32>  (t=@0x7f16fd8b18dc,
>> p=...)
>> >> at
>> >> include/encoding.h:35
>> >> #8  0x7f170174198a in decode (v=@0x7f16fd8b190c, p=...) at
>> >> include/encoding.h:80
>> >> #9  0x7f1701741ade in decode (s=..., p=...) at
>> >> include/encoding.h:189
>> >> #10 0x7f17012e8369 in
>> >> librados::RadosClient::C_aio_sparse_read_Ack::finish
>> >> (this=0x7f16f40d6200,
>> >> r=0) at librados.cc:463
>> >> #11 0x7f170132bb5a in Objecter::handle_osd_op_reply
>> (this=0x13423e0,
>> >> m=0x1346520) at osdc/Objecter.cc:794
>> >> #12 0x7f17012d1444 in librados::RadosClient::_dispatch
>> >> (this=0x133f810, m=0x1346520) at librados.cc:751
>> >> #13 0x7f17012d1244 in librados::RadosClient::ms_dispatch
>> >> (this=0x133f810, m=0x1346520) at librados.cc:717
>> >> #14 0x7f170131b57b in Messenger::ms_deliver_dispatch
>> >> (this=0x1341910,
>> >> m=0x1346520) at msg/Messenger.h:98
>> >> #15 0x7f17013090d3 in SimpleMessenger::dispatch_entry
>> >> (this=0x1341910)
>> >> at msg/SimpleMessenger.cc:352
>> >> #16 0x7f17012e296e in SimpleMessenger::DispatchThread::entry
>> >> (this=0x1341da0) at msg/SimpleMessenger.h:533
>> >> #17 0x7f170131a39b in Thread::_entry_func (arg=0x1341da0) at
>> >> common/Thread.h:41
>> >> #18 0x7f1701f6dac4 in start_thread (arg=) at
>> >> pthread_create.c:297
>> >> #19 0x7f16ff8c838d in clone () at
>> >> ../sysdeps/unix/sysv/linux/x86_64/clone.S:115
>> >
>> > I haven't seen that error before, but it's probably a bug in th

Re: [Qemu-devel] Qemu + RBD = ceph::buffer::end_of_buffer

[Sage Weil]

f 9  (or 8?)
p n
p s

(BTW this might be faster over irc, #ceph on irc.oftc.net)

Thanks!
sage


On Fri, 6 May 2011, Dyweni - Qemu-Devel wrote:

> Hi Sage/Lists!
> 
> 
> (gdb) print c->bl._len
> $1 = 20
> 
> 
> And in case this is helpful:
> 
> (gdb) print *c
> $2 = {lock = {name = 0x7f1701430f8d "AioCompletionImpl lock", id = -1,
> recursive = false, lockdep = true, backtrace = false, _m = {__data =
> {__lock = 1, __count = 0,
> __owner = 25800, __nusers = 1, __kind = 2, __spins = 0, __list =
> {__prev = 0x0, __next = 0x0}},
>   __size =
> "\001\000\000\000\000\000\000\000\310d\000\000\001\000\000\000\002",
> '\000' , __align = 1}, nlock = 1}, cond = {
> _vptr.Cond = 0x7f1701952bd0, _c = {__data = {__lock = 0, __futex = 0,
> __total_seq = 0, __wakeup_seq = 0, __woken_seq = 0, __mutex = 0x0,
> __nwaiters = 0,
> __broadcast_seq = 0}, __size = '\000' , __align
> = 0}}, ref = 1, rval = 0, released = true, ack = true, safe =
> false, objver = {version = 0,
> epoch = 0, __pad = 0}, callback_complete = 0x7f170173de33
> ,
>   callback_safe = 0x7f170173d8bd  void*)>, callback_arg = 0x7f16f40d6010, bl = {
> _buffers = { std::allocator >> = {
> _M_impl = { >> =
> {<__gnu_cxx::new_allocator >> =
> {}, }, _M_node = {_M_next =
> 0x1350530, _M_prev = 0x1350530}}}, }, _len = 20,
> append_buffer = {_raw = 0x0, _off = 0, _len = 0}, last_p = {
>   bl = 0x7f16f40d6170, ls = 0x7f16f40d6170, off = 0, p = {_M_node =
> 0x7f16f40d6170}, p_off = 0}}, pbl = 0x0, buf = 0x0, maxlen = 0}
> 
> 
> 
> Thanks,
> Dyweni
> 
> 
> 
> 
> > On Fri, 6 May 2011, Dyweni - Qemu-Devel wrote:
> >> Hi Josh/Lists!
> >>
> >> 463 ::decode(*data_bl, iter);
> >> (gdb) print r
> >> $1 = 0
> >> (gdb) print data_bl
> >> $2 = (ceph::bufferlist *) 0x7f16f40d6060
> >> (gdb) print data_bl->_len
> >> $3 = 0
> >
> > What about c->bl._len?
> >
> > sage
> >
> >
> >> (gdb) print iter->off
> >> $4 = 20
> >>
> >>
> >> Thanks,
> >> Dyweni
> >>
> >>
> >>
> >> > CCing the ceph list.
> >> >
> >> > On 05/06/2011 12:23 PM, Dyweni - Qemu-Devel wrote:
> >> >> Hi List!
> >> >>
> >> >> I upgraded Ceph to the latest development version
> >> >> Commit: 0edbc75a5fe8c3028faf85546f3264d28653ea3f
> >> >> Pulled from: git://ceph.newdream.net/ceph.git
> >> >>
> >> >> I recompiled the latest GIT version of QEMU-KVM (with Josh Durgin's
> >> >> patches) against the latest git version of Ceph.
> >> >>
> >> >> However, this error is still occurring:
> >> >>
> >> >> terminate called after throwing an instance of
> >> >> 'ceph::buffer::end_of_buffer'
> >> >>what():  buffer::end_of_buffer
> >> >> Aborted (core dumped)
> >> >>
> >> >>
> >> >>
> >> >> Here's another backtrace from GDB:
> >> >>
> >> >> #0  0x7f16ff829495 in raise (sig=) at
> >> >> ../nptl/sysdeps/unix/sysv/linux/raise.c:64
> >> >> #1  0x7f16ff82a81f in abort () at abort.c:92
> >> >> #2  0x7f16fed74a25 in __gnu_cxx::__verbose_terminate_handler ()
> >> at
> >> >> /usr/src/debug/sys-devel/gcc-4.4.5/gcc-4.4.5/libstdc++-v3/libsupc++/vterminate.cc:93
> >> >> #3  0x7f16fed71c64 in __cxxabiv1::__terminate
> >> >> (handler=0x7f16fed74817
> >> >> <__gnu_cxx::__verbose_terminate_handler()>)
> >> >>  at
> >> >> /usr/src/debug/sys-devel/gcc-4.4.5/gcc-4.4.5/libstdc++-v3/libsupc++/eh_terminate.cc:38
> >> >> #4  0x7f16fed71c8c in std::terminate () at
> >> >> /usr/src/debug/sys-devel/gcc-4.4.5/gcc-4.4.5/libstdc++-v3/libsupc++/eh_terminate.cc:48
> >> >> #5  0x7f16fed71ea4 in __cxxabiv1::__cxa_throw (obj=0x1346470,
> >> >> tinfo=0x7f1701952ce0, dest=0x7f17017403d4
> >> >> )
> >> >>  at
> >> >> /usr/src/debug/sys-devel/gcc-4.4.5/gcc-4.4.5/libstdc++-v3/libsupc++/eh_throw.cc:83
> >> >> #6  0x7f1701740a7b in ceph::buffer::list::iterator::copy
> >> >> (this=0x7f16fd8b1930, len=4, dest=0x7f16fd8b18dc "") at
> >> >> include/buffer.h:379
> >> >> #7  0x7f1701743328 in decode_raw<__le32>  (t=@0x7f16fd8b18dc,
> >> p=...)
> >> >> at
> >> >> include/encoding.h:35
> >> >> #8  0x7f170174198a in decode (v=@0x7f16fd8b190c, p=...) at
> >> >> include/encoding.h:80
> >> >> #9  0x7f1701741ade in decode (s=..., p=...) at
> >> >> include/encoding.h:189
> >> >> #10 0x7f17012e8369 in
> >> >> librados::RadosClient::C_aio_sparse_read_Ack::finish
> >> >> (this=0x7f16f40d6200,
> >> >> r=0) at librados.cc:463
> >> >> #11 0x7f170132bb5a in Objecter::handle_osd_op_reply
> >> (this=0x13423e0,
> >> >> m=0x1346520) at osdc/Objecter.cc:794
> >> >> #12 0x7f17012d1444 in librados::RadosClient::_dispatch
> >> >> (this=0x133f810, m=0x1346520) at librados.cc:751
> >> >> #13 0x7f17012d1244 in librados::RadosClient::ms_dispatch
> >> >> (this=0x133f810, m=0x1346520) at librados.cc:717
> >> >> #14 0x7f170131b57b in Messenger::ms_deliver_dispatch
> >> >> (this=0x1341910,
> >> >> m=0x1346520) at msg/Messenger.h:98
> >> >> #15 0x7f17013090d3 in SimpleMessenger::dispatch_entry
> >> >> (this=0x1341910)
> >> >> at msg/SimpleMessenger.cc:352
> >> >> #16 0x

Re: [Qemu-devel] Qemu + RBD = ceph::buffer::end_of_buffer

[Dyweni - Qemu-Devel]

Hi Sage/Lists!


(gdb) f 8
#8  0x7f170174198a in decode (v=@0x7f16fd8b190c, p=...) at
include/encoding.h:80
80  WRITE_INTTYPE_ENCODER(uint32_t, le32)
(gdb) p n
No symbol "n" in current context.
(gdb) p s
No symbol "s" in current context.


(gdb) f 9
#9  0x7f1701741ade in decode (s=..., p=...) at include/encoding.h:189
189   decode(len, p);
(gdb) p n
No symbol "n" in current context.
(gdb) p s
$3 = (ceph::bufferlist &) @0x7f16f40d6060: {_buffers =
{ >>
= {
  _M_impl = { >> =
{<__gnu_cxx::new_allocator >> =
{}, }, _M_node = {_M_next =
0x7f16f40d6060, _M_prev = 0x7f16f40d6060}}}, }, _len
= 0, append_buffer = {_raw = 0x0, _off = 0, _len = 0}, last_p = {
bl = 0x7f16f40d6060, ls = 0x7f16f40d6060, off = 0, p = {_M_node =
0x7f16f40d6060}, p_off = 0}}


Sorry, I don't have access to IRC from where I am at.

Thanks,
Dyweni




> f 9  (or 8?)
> p n
> p s
>
> (BTW this might be faster over irc, #ceph on irc.oftc.net)
>
> Thanks!
> sage
>
>
> On Fri, 6 May 2011, Dyweni - Qemu-Devel wrote:
>
>> Hi Sage/Lists!
>>
>>
>> (gdb) print c->bl._len
>> $1 = 20
>>
>>
>> And in case this is helpful:
>>
>> (gdb) print *c
>> $2 = {lock = {name = 0x7f1701430f8d "AioCompletionImpl lock", id = -1,
>> recursive = false, lockdep = true, backtrace = false, _m = {__data =
>> {__lock = 1, __count = 0,
>> __owner = 25800, __nusers = 1, __kind = 2, __spins = 0, __list =
>> {__prev = 0x0, __next = 0x0}},
>>   __size =
>> "\001\000\000\000\000\000\000\000\310d\000\000\001\000\000\000\002",
>> '\000' , __align = 1}, nlock = 1}, cond = {
>> _vptr.Cond = 0x7f1701952bd0, _c = {__data = {__lock = 0, __futex =
>> 0,
>> __total_seq = 0, __wakeup_seq = 0, __woken_seq = 0, __mutex = 0x0,
>> __nwaiters = 0,
>> __broadcast_seq = 0}, __size = '\000' ,
>> __align
>> = 0}}, ref = 1, rval = 0, released = true, ack = true, safe =
>> false, objver = {version = 0,
>> epoch = 0, __pad = 0}, callback_complete = 0x7f170173de33
>> ,
>>   callback_safe = 0x7f170173d8bd > void*)>, callback_arg = 0x7f16f40d6010, bl = {
>> _buffers = {> std::allocator >> = {
>> _M_impl = { >>
>> =
>> {<__gnu_cxx::new_allocator >> =
>> {}, }, _M_node = {_M_next =
>> 0x1350530, _M_prev = 0x1350530}}}, }, _len = 20,
>> append_buffer = {_raw = 0x0, _off = 0, _len = 0}, last_p = {
>>   bl = 0x7f16f40d6170, ls = 0x7f16f40d6170, off = 0, p = {_M_node =
>> 0x7f16f40d6170}, p_off = 0}}, pbl = 0x0, buf = 0x0, maxlen = 0}
>>
>>
>>
>> Thanks,
>> Dyweni
>>
>>
>>
>>
>> > On Fri, 6 May 2011, Dyweni - Qemu-Devel wrote:
>> >> Hi Josh/Lists!
>> >>
>> >> 463 ::decode(*data_bl, iter);
>> >> (gdb) print r
>> >> $1 = 0
>> >> (gdb) print data_bl
>> >> $2 = (ceph::bufferlist *) 0x7f16f40d6060
>> >> (gdb) print data_bl->_len
>> >> $3 = 0
>> >
>> > What about c->bl._len?
>> >
>> > sage
>> >
>> >
>> >> (gdb) print iter->off
>> >> $4 = 20
>> >>
>> >>
>> >> Thanks,
>> >> Dyweni
>> >>
>> >>
>> >>
>> >> > CCing the ceph list.
>> >> >
>> >> > On 05/06/2011 12:23 PM, Dyweni - Qemu-Devel wrote:
>> >> >> Hi List!
>> >> >>
>> >> >> I upgraded Ceph to the latest development version
>> >> >>Commit: 0edbc75a5fe8c3028faf85546f3264d28653ea3f
>> >> >>Pulled from: git://ceph.newdream.net/ceph.git
>> >> >>
>> >> >> I recompiled the latest GIT version of QEMU-KVM (with Josh
>> Durgin's
>> >> >> patches) against the latest git version of Ceph.
>> >> >>
>> >> >> However, this error is still occurring:
>> >> >>
>> >> >> terminate called after throwing an instance of
>> >> >> 'ceph::buffer::end_of_buffer'
>> >> >>what():  buffer::end_of_buffer
>> >> >> Aborted (core dumped)
>> >> >>
>> >> >>
>> >> >>
>> >> >> Here's another backtrace from GDB:
>> >> >>
>> >> >> #0  0x7f16ff829495 in raise (sig=) at
>> >> >> ../nptl/sysdeps/unix/sysv/linux/raise.c:64
>> >> >> #1  0x7f16ff82a81f in abort () at abort.c:92
>> >> >> #2  0x7f16fed74a25 in __gnu_cxx::__verbose_terminate_handler
>> ()
>> >> at
>> >> >> /usr/src/debug/sys-devel/gcc-4.4.5/gcc-4.4.5/libstdc++-v3/libsupc++/vterminate.cc:93
>> >> >> #3  0x7f16fed71c64 in __cxxabiv1::__terminate
>> >> >> (handler=0x7f16fed74817
>> >> >> <__gnu_cxx::__verbose_terminate_handler()>)
>> >> >>  at
>> >> >> /usr/src/debug/sys-devel/gcc-4.4.5/gcc-4.4.5/libstdc++-v3/libsupc++/eh_terminate.cc:38
>> >> >> #4  0x7f16fed71c8c in std::terminate () at
>> >> >> /usr/src/debug/sys-devel/gcc-4.4.5/gcc-4.4.5/libstdc++-v3/libsupc++/eh_terminate.cc:48
>> >> >> #5  0x7f16fed71ea4 in __cxxabiv1::__cxa_throw (obj=0x1346470,
>> >> >> tinfo=0x7f1701952ce0, dest=0x7f17017403d4
>> >> >> )
>> >> >>  at
>> >> >> /usr/src/debug/sys-devel/gcc-4.4.5/gcc-4.4.5/libstdc++-v3/libsupc++/eh_throw.cc:83
>> >> >> #6  0x7f1701740a7b in ceph::buffer::list::iterator::copy
>> >> >> (this=0x7f16fd8b1930, len=4, dest=0x7f16fd8b18dc "") at
>> >> >> include/buffer.h:379
>> >> >> #7  0x7f1701743328 in decode_raw<__le32>  (t=@0x7f16fd8b18dc,
>> >> p=...)
>> >> >> at
>> >> >> include/encoding.h:35
>> >> >> #8  

Re: [Qemu-devel] Qemu + RBD = ceph::buffer::end_of_buffer

[Sage Weil]

On Fri, 6 May 2011, Dyweni - Qemu-Devel wrote:
> Hi Sage/Lists!
> 
> 
> (gdb) f 8
> #8  0x7f170174198a in decode (v=@0x7f16fd8b190c, p=...) at
> include/encoding.h:80
> 80  WRITE_INTTYPE_ENCODER(uint32_t, le32)
> (gdb) p n
> No symbol "n" in current context.
> (gdb) p s
> No symbol "s" in current context.
> 
> 
> (gdb) f 9
> #9  0x7f1701741ade in decode (s=..., p=...) at include/encoding.h:189
> 189   decode(len, p);
> (gdb) p n
> No symbol "n" in current context.
> (gdb) p s
> $3 = (ceph::bufferlist &) @0x7f16f40d6060: {_buffers =
> { >>
> = {
>   _M_impl = { >> =
> {<__gnu_cxx::new_allocator >> =
> {}, }, _M_node = {_M_next =
> 0x7f16f40d6060, _M_prev = 0x7f16f40d6060}}}, }, _len
> = 0, append_buffer = {_raw = 0x0, _off = 0, _len = 0}, last_p = {
> bl = 0x7f16f40d6060, ls = 0x7f16f40d6060, off = 0, p = {_M_node =
> 0x7f16f40d6060}, p_off = 0}}
> 
> 
> Sorry, I don't have access to IRC from where I am at.

No worries.

Are you OSDs, by chance, running on 32bit machines?  This looks like a 
word size encoding thing.

sage


> 
> Thanks,
> Dyweni
> 
> 
> 
> 
> > f 9  (or 8?)
> > p n
> > p s
> >
> > (BTW this might be faster over irc, #ceph on irc.oftc.net)
> >
> > Thanks!
> > sage
> >
> >
> > On Fri, 6 May 2011, Dyweni - Qemu-Devel wrote:
> >
> >> Hi Sage/Lists!
> >>
> >>
> >> (gdb) print c->bl._len
> >> $1 = 20
> >>
> >>
> >> And in case this is helpful:
> >>
> >> (gdb) print *c
> >> $2 = {lock = {name = 0x7f1701430f8d "AioCompletionImpl lock", id = -1,
> >> recursive = false, lockdep = true, backtrace = false, _m = {__data =
> >> {__lock = 1, __count = 0,
> >> __owner = 25800, __nusers = 1, __kind = 2, __spins = 0, __list =
> >> {__prev = 0x0, __next = 0x0}},
> >>   __size =
> >> "\001\000\000\000\000\000\000\000\310d\000\000\001\000\000\000\002",
> >> '\000' , __align = 1}, nlock = 1}, cond = {
> >> _vptr.Cond = 0x7f1701952bd0, _c = {__data = {__lock = 0, __futex =
> >> 0,
> >> __total_seq = 0, __wakeup_seq = 0, __woken_seq = 0, __mutex = 0x0,
> >> __nwaiters = 0,
> >> __broadcast_seq = 0}, __size = '\000' ,
> >> __align
> >> = 0}}, ref = 1, rval = 0, released = true, ack = true, safe =
> >> false, objver = {version = 0,
> >> epoch = 0, __pad = 0}, callback_complete = 0x7f170173de33
> >> ,
> >>   callback_safe = 0x7f170173d8bd  >> void*)>, callback_arg = 0x7f16f40d6010, bl = {
> >> _buffers = { >> std::allocator >> = {
> >> _M_impl = { >>
> >> =
> >> {<__gnu_cxx::new_allocator >> =
> >> {}, }, _M_node = {_M_next =
> >> 0x1350530, _M_prev = 0x1350530}}}, }, _len = 20,
> >> append_buffer = {_raw = 0x0, _off = 0, _len = 0}, last_p = {
> >>   bl = 0x7f16f40d6170, ls = 0x7f16f40d6170, off = 0, p = {_M_node =
> >> 0x7f16f40d6170}, p_off = 0}}, pbl = 0x0, buf = 0x0, maxlen = 0}
> >>
> >>
> >>
> >> Thanks,
> >> Dyweni
> >>
> >>
> >>
> >>
> >> > On Fri, 6 May 2011, Dyweni - Qemu-Devel wrote:
> >> >> Hi Josh/Lists!
> >> >>
> >> >> 463 ::decode(*data_bl, iter);
> >> >> (gdb) print r
> >> >> $1 = 0
> >> >> (gdb) print data_bl
> >> >> $2 = (ceph::bufferlist *) 0x7f16f40d6060
> >> >> (gdb) print data_bl->_len
> >> >> $3 = 0
> >> >
> >> > What about c->bl._len?
> >> >
> >> > sage
> >> >
> >> >
> >> >> (gdb) print iter->off
> >> >> $4 = 20
> >> >>
> >> >>
> >> >> Thanks,
> >> >> Dyweni
> >> >>
> >> >>
> >> >>
> >> >> > CCing the ceph list.
> >> >> >
> >> >> > On 05/06/2011 12:23 PM, Dyweni - Qemu-Devel wrote:
> >> >> >> Hi List!
> >> >> >>
> >> >> >> I upgraded Ceph to the latest development version
> >> >> >>  Commit: 0edbc75a5fe8c3028faf85546f3264d28653ea3f
> >> >> >>  Pulled from: git://ceph.newdream.net/ceph.git
> >> >> >>
> >> >> >> I recompiled the latest GIT version of QEMU-KVM (with Josh
> >> Durgin's
> >> >> >> patches) against the latest git version of Ceph.
> >> >> >>
> >> >> >> However, this error is still occurring:
> >> >> >>
> >> >> >> terminate called after throwing an instance of
> >> >> >> 'ceph::buffer::end_of_buffer'
> >> >> >>what():  buffer::end_of_buffer
> >> >> >> Aborted (core dumped)
> >> >> >>
> >> >> >>
> >> >> >>
> >> >> >> Here's another backtrace from GDB:
> >> >> >>
> >> >> >> #0  0x7f16ff829495 in raise (sig=) at
> >> >> >> ../nptl/sysdeps/unix/sysv/linux/raise.c:64
> >> >> >> #1  0x7f16ff82a81f in abort () at abort.c:92
> >> >> >> #2  0x7f16fed74a25 in __gnu_cxx::__verbose_terminate_handler
> >> ()
> >> >> at
> >> >> >> /usr/src/debug/sys-devel/gcc-4.4.5/gcc-4.4.5/libstdc++-v3/libsupc++/vterminate.cc:93
> >> >> >> #3  0x7f16fed71c64 in __cxxabiv1::__terminate
> >> >> >> (handler=0x7f16fed74817
> >> >> >> <__gnu_cxx::__verbose_terminate_handler()>)
> >> >> >>  at
> >> >> >> /usr/src/debug/sys-devel/gcc-4.4.5/gcc-4.4.5/libstdc++-v3/libsupc++/eh_terminate.cc:38
> >> >> >> #4  0x7f16fed71c8c in std::terminate () at
> >> >> >> /usr/src/debug/sys-devel/gcc-4.4.5/gcc-4.4.5/libstdc++-v3/libsupc++/eh_terminate.cc:48
> >> >> >> #5  0x7f16fed71ea4 in __cxxabiv1::__cxa_throw (obj=

Re: [Qemu-devel] [PATCH V4 01/10] Support for TPM command line options

[Serge E. Hallyn]

Quoting Stefan Berger (stef...@linux.vnet.ibm.com):
> This patch adds support for TPM command line options.
> The command line supported here (considering the libtpms based
> backend) are
> 
> ./qemu-... -tpm type=,path=,
> 
> and
> 
> ./qemu-... -tpm ?
>
> where the latter works similar to -soundhw ? and shows a list of
> available TPM backends (i.e., libtpms-based, Xen).
> 
> Only the 'type' is interpreted in arch_init.c. Using this parameter,
> the backend is chosen, i.e., 'builtin' for the libtpms-based
> builtin TPM. The interpretation of the other parameters along with
> determining whether enough parameters were provided is pushed into
> the backend driver, which needs to implement the interface function
> 'handle_options' and return true if the VM can be started or 'false'
> if not enough or bad parameters were provided.
> 
> v4:
>  - coding style fixes
> 
> v3:
>  - added hw/tpm_tis.h to this patch so Qemu compiles at this stage
> 
> Signed-off-by: Stefan Berger 

Thanks, Stefan.  Two nits:

> +static QemuOptsList qemu_tpm_opts = {
> +.name = "tpm",
> +.head = QTAILQ_HEAD_INITIALIZER(qemu_tpm_opts.head),
> +.desc = {
> +{
> +.name = "type",
> +.type = QEMU_OPT_STRING,
> +.help = "Type of TPM backend",
> +},
> +{
> +.name = "path",
> +.type = QEMU_OPT_STRING,
> +.help = "Persitent storage for TPM state",

Persistent.

...

> +# else /* CONFIG_TPM */
> +
> +void select_tpm(const char *optarg)
> +{
> +(void)optarg;
> +}

I realize this should never get called if !CONFIG_TPM, but that still
doesn't seem like cause to go directly calling a potentially NULL
string.

Otherwise, fwiw

Acked-by: Serge Hallyn 

thanks,
-serge

Re: [Qemu-devel] Qemu + RBD = ceph::buffer::end_of_buffer

[Dyweni - Qemu-Devel]

Hi Sage/Lists!


Yes!  The entire Ceph cluster (1 Mon, 1 MSD, 3 OSD) are 32bit linux.

The machine running Qemu is 64bit linux.


Thanks,
Dyweni



> On Fri, 6 May 2011, Dyweni - Qemu-Devel wrote:
>> Hi Sage/Lists!
>>
>>
>> (gdb) f 8
>> #8  0x7f170174198a in decode (v=@0x7f16fd8b190c, p=...) at
>> include/encoding.h:80
>> 80  WRITE_INTTYPE_ENCODER(uint32_t, le32)
>> (gdb) p n
>> No symbol "n" in current context.
>> (gdb) p s
>> No symbol "s" in current context.
>>
>>
>> (gdb) f 9
>> #9  0x7f1701741ade in decode (s=..., p=...) at
>> include/encoding.h:189
>> 189   decode(len, p);
>> (gdb) p n
>> No symbol "n" in current context.
>> (gdb) p s
>> $3 = (ceph::bufferlist &) @0x7f16f40d6060: {_buffers =
>> {
>> >>
>> = {
>>   _M_impl = { >> =
>> {<__gnu_cxx::new_allocator >> =
>> {}, }, _M_node = {_M_next =
>> 0x7f16f40d6060, _M_prev = 0x7f16f40d6060}}}, }, _len
>> = 0, append_buffer = {_raw = 0x0, _off = 0, _len = 0}, last_p = {
>> bl = 0x7f16f40d6060, ls = 0x7f16f40d6060, off = 0, p = {_M_node =
>> 0x7f16f40d6060}, p_off = 0}}
>>
>>
>> Sorry, I don't have access to IRC from where I am at.
>
> No worries.
>
> Are you OSDs, by chance, running on 32bit machines?  This looks like a
> word size encoding thing.
>
> sage
>
>
>>
>> Thanks,
>> Dyweni
>>
>>
>>
>>
>> > f 9  (or 8?)
>> > p n
>> > p s
>> >
>> > (BTW this might be faster over irc, #ceph on irc.oftc.net)
>> >
>> > Thanks!
>> > sage
>> >
>> >
>> > On Fri, 6 May 2011, Dyweni - Qemu-Devel wrote:
>> >
>> >> Hi Sage/Lists!
>> >>
>> >>
>> >> (gdb) print c->bl._len
>> >> $1 = 20
>> >>
>> >>
>> >> And in case this is helpful:
>> >>
>> >> (gdb) print *c
>> >> $2 = {lock = {name = 0x7f1701430f8d "AioCompletionImpl lock", id =
>> -1,
>> >> recursive = false, lockdep = true, backtrace = false, _m = {__data =
>> >> {__lock = 1, __count = 0,
>> >> __owner = 25800, __nusers = 1, __kind = 2, __spins = 0,
>> __list =
>> >> {__prev = 0x0, __next = 0x0}},
>> >>   __size =
>> >> "\001\000\000\000\000\000\000\000\310d\000\000\001\000\000\000\002",
>> >> '\000' , __align = 1}, nlock = 1}, cond = {
>> >> _vptr.Cond = 0x7f1701952bd0, _c = {__data = {__lock = 0, __futex
>> =
>> >> 0,
>> >> __total_seq = 0, __wakeup_seq = 0, __woken_seq = 0, __mutex = 0x0,
>> >> __nwaiters = 0,
>> >> __broadcast_seq = 0}, __size = '\000' ,
>> >> __align
>> >> = 0}}, ref = 1, rval = 0, released = true, ack = true, safe =
>> >> false, objver = {version = 0,
>> >> epoch = 0, __pad = 0}, callback_complete = 0x7f170173de33
>> >> ,
>> >>   callback_safe = 0x7f170173d8bd
>> > >> void*)>, callback_arg = 0x7f16f40d6010, bl = {
>> >> _buffers = {> >> std::allocator >> = {
>> >> _M_impl = {
>> >>
>> >> =
>> >> {<__gnu_cxx::new_allocator >> =
>> >> {}, }, _M_node = {_M_next =
>> >> 0x1350530, _M_prev = 0x1350530}}}, }, _len = 20,
>> >> append_buffer = {_raw = 0x0, _off = 0, _len = 0}, last_p = {
>> >>   bl = 0x7f16f40d6170, ls = 0x7f16f40d6170, off = 0, p = {_M_node
>> =
>> >> 0x7f16f40d6170}, p_off = 0}}, pbl = 0x0, buf = 0x0, maxlen = 0}
>> >>
>> >>
>> >>
>> >> Thanks,
>> >> Dyweni
>> >>
>> >>
>> >>
>> >>
>> >> > On Fri, 6 May 2011, Dyweni - Qemu-Devel wrote:
>> >> >> Hi Josh/Lists!
>> >> >>
>> >> >> 463 ::decode(*data_bl, iter);
>> >> >> (gdb) print r
>> >> >> $1 = 0
>> >> >> (gdb) print data_bl
>> >> >> $2 = (ceph::bufferlist *) 0x7f16f40d6060
>> >> >> (gdb) print data_bl->_len
>> >> >> $3 = 0
>> >> >
>> >> > What about c->bl._len?
>> >> >
>> >> > sage
>> >> >
>> >> >
>> >> >> (gdb) print iter->off
>> >> >> $4 = 20
>> >> >>
>> >> >>
>> >> >> Thanks,
>> >> >> Dyweni
>> >> >>
>> >> >>
>> >> >>
>> >> >> > CCing the ceph list.
>> >> >> >
>> >> >> > On 05/06/2011 12:23 PM, Dyweni - Qemu-Devel wrote:
>> >> >> >> Hi List!
>> >> >> >>
>> >> >> >> I upgraded Ceph to the latest development version
>> >> >> >> Commit: 0edbc75a5fe8c3028faf85546f3264d28653ea3f
>> >> >> >> Pulled from: git://ceph.newdream.net/ceph.git
>> >> >> >>
>> >> >> >> I recompiled the latest GIT version of QEMU-KVM (with Josh
>> >> Durgin's
>> >> >> >> patches) against the latest git version of Ceph.
>> >> >> >>
>> >> >> >> However, this error is still occurring:
>> >> >> >>
>> >> >> >> terminate called after throwing an instance of
>> >> >> >> 'ceph::buffer::end_of_buffer'
>> >> >> >>what():  buffer::end_of_buffer
>> >> >> >> Aborted (core dumped)
>> >> >> >>
>> >> >> >>
>> >> >> >>
>> >> >> >> Here's another backtrace from GDB:
>> >> >> >>
>> >> >> >> #0  0x7f16ff829495 in raise (sig=) at
>> >> >> >> ../nptl/sysdeps/unix/sysv/linux/raise.c:64
>> >> >> >> #1  0x7f16ff82a81f in abort () at abort.c:92
>> >> >> >> #2  0x7f16fed74a25 in
>> __gnu_cxx::__verbose_terminate_handler
>> >> ()
>> >> >> at
>> >> >> >> /usr/src/debug/sys-devel/gcc-4.4.5/gcc-4.4.5/libstdc++-v3/libsupc++/vterminate.cc:93
>> >> >> >> #3  0x7f16fed71c64 in __cxxabiv1::__terminate
>> >> >> >> (handler=0x7f16fed74817
>> >> >> >> <__gnu_cxx::__verbose_terminate_handler()>)
>> >

Re: [Qemu-devel] Qemu + RBD = ceph::buffer::end_of_buffer

[Sage Weil]

On Fri, 6 May 2011, Dyweni - Qemu-Devel wrote:
> Hi Sage/Lists!
> 
> Yes!  The entire Ceph cluster (1 Mon, 1 MSD, 3 OSD) are 32bit linux.
> 
> The machine running Qemu is 64bit linux.

Excellent.  This is now fixed by 48d94f6e34da8ace2b5cb128de1bcfb34b3c40b5 
in the stable and master branches of ceph.git.  Or you can apply the patch 
below.

Thanks!
sage



>From 48d94f6e34da8ace2b5cb128de1bcfb34b3c40b5 Mon Sep 17 00:00:00 2001
From: Sage Weil 
Date: Fri, 6 May 2011 13:42:23 -0700
Subject: [PATCH] osd: used fixed size types for fiemap/mapext/sparseread 
encoding

The client expects , so this breaks on any 32-bit osd.

Signed-off-by: Sage Weil 
---
 src/os/FileStore.cc |4 ++--
 src/osd/ReplicatedPG.cc |4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/os/FileStore.cc b/src/os/FileStore.cc
index 81bc91d..ca11435 100644
--- a/src/os/FileStore.cc
+++ b/src/os/FileStore.cc
@@ -2198,7 +2198,7 @@ int FileStore::fiemap(coll_t cid, const sobject_t& oid,
 {
 
   if (!ioctl_fiemap) {
-map m;
+map m;
 m[offset] = len;
 ::encode(m, bl);
 return 0;
@@ -2207,7 +2207,7 @@ int FileStore::fiemap(coll_t cid, const sobject_t& oid,
 
   char fn[PATH_MAX];
   struct fiemap *fiemap = NULL;
-  map extmap;
+  map extmap;
 
   get_coname(cid, oid, fn, sizeof(fn));
 
diff --git a/src/osd/ReplicatedPG.cc b/src/osd/ReplicatedPG.cc
index ba76b05..84399ca 100644
--- a/src/osd/ReplicatedPG.cc
+++ b/src/osd/ReplicatedPG.cc
@@ -1142,10 +1142,10 @@ int ReplicatedPG::do_osd_ops(OpContext *ctx, 
vector& ops,
  result = r;
   break;
}
-map m;
+map m;
 bufferlist::iterator iter = bl.begin();
 ::decode(m, iter);
-map::iterator miter;
+map::iterator miter;
 bufferlist data_bl;
 for (miter = m.begin(); miter != m.end(); ++miter) {
   bufferlist tmpbl;
-- 
1.7.0

Re: [Qemu-devel] [PATCH V4 01/10] Support for TPM command line options

[Stefan Berger]

On 05/06/2011 04:23 PM, Serge E. Hallyn wrote:

Quoting Stefan Berger (stef...@linux.vnet.ibm.com):

This patch adds support for TPM command line options.
The command line supported here (considering the libtpms based
backend) are

./qemu-... -tpm type=,path=,

and

./qemu-... -tpm ?

where the latter works similar to -soundhw ? and shows a list of
available TPM backends (i.e., libtpms-based, Xen).

Only the 'type' is interpreted in arch_init.c. Using this parameter,
the backend is chosen, i.e., 'builtin' for the libtpms-based
builtin TPM. The interpretation of the other parameters along with
determining whether enough parameters were provided is pushed into
the backend driver, which needs to implement the interface function
'handle_options' and return true if the VM can be started or 'false'
if not enough or bad parameters were provided.

v4:
  - coding style fixes

v3:
  - added hw/tpm_tis.h to this patch so Qemu compiles at this stage

Signed-off-by: Stefan Berger

Thanks, Stefan.  Two nits:


+static QemuOptsList qemu_tpm_opts = {
+.name = "tpm",
+.head = QTAILQ_HEAD_INITIALIZER(qemu_tpm_opts.head),
+.desc = {
+{
+.name = "type",
+.type = QEMU_OPT_STRING,
+.help = "Type of TPM backend",
+},
+{
+.name = "path",
+.type = QEMU_OPT_STRING,
+.help = "Persitent storage for TPM state",

Persistent.

Oh, typo. Will fix it.

...


+# else /* CONFIG_TPM */
+
+void select_tpm(const char *optarg)
+{
+(void)optarg;
+}

I realize this should never get called if !CONFIG_TPM, but that still
doesn't seem like cause to go directly calling a potentially NULL
string.
(void)optarg is just there to make the compiler not put out a warning 
about an unused parameter. I could have used __attribute__((unused)) 
instead but chose this one here. It's not calling anything.


Thanks!

   Stefan

Otherwise, fwiw

Acked-by: Serge Hallyn

thanks,
-serge

Re: [Qemu-devel] [PATCH V4 01/10] Support for TPM command line options

[Serge E. Hallyn]

Quoting Stefan Berger (stef...@linux.vnet.ibm.com):
> On 05/06/2011 04:23 PM, Serge E. Hallyn wrote:
> >Quoting Stefan Berger (stef...@linux.vnet.ibm.com):
> >>This patch adds support for TPM command line options.
> >>The command line supported here (considering the libtpms based
> >>backend) are
> >>
> >>./qemu-... -tpm type=,path=,
> >>
> >>and
> >>
> >>./qemu-... -tpm ?
> >>
> >>where the latter works similar to -soundhw ? and shows a list of
> >>available TPM backends (i.e., libtpms-based, Xen).
> >>
> >>Only the 'type' is interpreted in arch_init.c. Using this parameter,
> >>the backend is chosen, i.e., 'builtin' for the libtpms-based
> >>builtin TPM. The interpretation of the other parameters along with
> >>determining whether enough parameters were provided is pushed into
> >>the backend driver, which needs to implement the interface function
> >>'handle_options' and return true if the VM can be started or 'false'
> >>if not enough or bad parameters were provided.
> >>
> >>v4:
> >>  - coding style fixes
> >>
> >>v3:
> >>  - added hw/tpm_tis.h to this patch so Qemu compiles at this stage
> >>
> >>Signed-off-by: Stefan Berger
> >Thanks, Stefan.  Two nits:
> >
> >>+static QemuOptsList qemu_tpm_opts = {
> >>+.name = "tpm",
> >>+.head = QTAILQ_HEAD_INITIALIZER(qemu_tpm_opts.head),
> >>+.desc = {
> >>+{
> >>+.name = "type",
> >>+.type = QEMU_OPT_STRING,
> >>+.help = "Type of TPM backend",
> >>+},
> >>+{
> >>+.name = "path",
> >>+.type = QEMU_OPT_STRING,
> >>+.help = "Persitent storage for TPM state",
> >Persistent.
> Oh, typo. Will fix it.
> >...
> >
> >>+# else /* CONFIG_TPM */
> >>+
> >>+void select_tpm(const char *optarg)
> >>+{
> >>+(void)optarg;
> >>+}
> >I realize this should never get called if !CONFIG_TPM, but that still
> >doesn't seem like cause to go directly calling a potentially NULL
> >string.
> (void)optarg is just there to make the compiler not put out a
> warning about an unused parameter. I could have used
> __attribute__((unused)) instead but chose this one here. It's not
> calling anything.

Haha, yes, I must have been seeing things :)

-serge

[Qemu-devel] [OT]where is qemu-users list?

[Alessandro]

I'm sorry for the OT message.
I have a _simple_ question that is more appropriate for qemu-users ml.
Strangely, Qemu-users ml appears to be unavailable. 
Anyone knows anything?

IT

Re: [Qemu-devel] [PATCH 5/7] PPC: Implement e500 (FSL) MMU

[Scott Wood]

On Fri, 6 May 2011 14:00:35 +0200
Alexander Graf  wrote:

> +static inline void booke206_flush_tlb_one(ppc_tlb_t *tlb, int num)
> +{
> +int i;
> +
> +for (i = 0; i < num; i++) {
> +if ((tlb[i].tlbe.prot & PAGE_VALID) &&
> +   !(tlb[i].tlbe.attr & MAS1_IPROT)) {
> +tlb[i].tlbe.prot = 0;
> +}
> +}
> +}

When this is called on reset, we want to ignore IPROT.

The valid check seems unnecessary.

> +void helper_booke206_tlbsx(target_ulong address_hi, target_ulong address_lo)

What is address_hi?

From gen_tlbsx_booke206() it looks like these two arguments correspond to
the two operands, so shouldn't they be added together?  I only see
address_lo used.

-Scott

Re: [Qemu-devel] [OT]where is qemu-users list?

[Mulyadi Santosa]

Hi :)

On Sat, May 7, 2011 at 04:20, Alessandro  wrote:
> I'm sorry for the OT message.
> I have a _simple_ question that is more appropriate for qemu-users ml.
> Strangely, Qemu-users ml appears to be unavailable.
> Anyone knows anything?

Few years ago, several people in this list decided that by that time,
there was no need to separate between -devel and -user list (due to
low traffic, I guess). So here we are, just qemu-devel exist.

-- 
regards,

Mulyadi Santosa
Freelance Linux trainer and consultant

blog: the-hydra.blogspot.com
training: mulyaditraining.blogspot.com

Re: [Qemu-devel] [PATCH V4 02/10] Add TPM (frontend) hardware interface (TPM TIS) to Qemu

[Serge E. Hallyn]

Quoting Stefan Berger (stef...@linux.vnet.ibm.com):
> This patch adds the main code of the TPM frontend driver, the TPM TIS
> interface, to Qemu. The code is largely based on my previous implementation
> for Xen but has been significantly extended to meet the standard's
> requirements, such as the support for changing of localities and all the
> functionality of the available flags.
> 
> Communication with the backend (i.e., for Xen or the libtpms-based one)
> is cleanly separated through an interface which the backend driver needs
> to implement.
> 
> The TPM TIS driver's backend was previously chosen in the code added
> to arch_init. The frontend holds a pointer to the chosen backend (interface).
> 
> Communication with the backend is largely based on signals and conditions.
> Whenever the frontend has collected a complete packet, it will signal
> the backend, which then starts processing the command. Once the result
> has been returned, the backend invokes a callback function
> (tis_tpm_receive_cb()).
> 
> The one tricky part is support for VM suspend while the TPM is processing
> a command. In this case the frontend driver is waiting for the backend
> to return the result of the last command before shutting down. It waits
> on a condition for a signal from the backend, which is delivered in 
> tis_tpm_receive_cb().
> 
> Testing the proper functioning of the different flags and localities 
> cannot be done from user space when running in Linux for example, since
> access to the address space of the TPM TIS interface is not possible. Also
> the Linux driver itself does not exercise all functionality. So, for
> testing there is a fairly extensive test suite as part of the SeaBIOS patches
> since from within the BIOS one can have full access to all the TPM's 
> registers.
> 
> v3:
>   - prefixing functions with tis_
>   - added a function to the backend interface 'early_startup_tpm' that
> allows to detect the presence of the block storage and gracefully fails
> Qemu if it's not available. This works with migration using shared
> storage but doesn't support migration with block storage migration.
> For encyrypted QCoW2 and in case of a snapshot resue the late_startup_tpm
> interface function is called
> 
> Signed-off-by: Stefan Berger 

Most of this is pretty foreign to me so this doesn't mean much, but

Acked-by: Serge Hallyn 

> +/*

Worth pointing out here that this is called with mutex held.

> + * read a byte of response data
> + */
> +static uint32_t tis_data_read(TPMState *s, uint8_t locty)
> +{
> +uint32_t ret = TPM_NO_DATA_BYTE;
> +uint16_t len;
> +
> +if ((s->loc[locty].sts & STS_DATA_AVAILABLE)) {
> +len = tis_get_size_from_buffer(&s->loc[locty].r_buffer);
> +
> +ret = s->loc[locty].r_buffer.buffer[s->loc[locty].r_offset++];
> +if (s->loc[locty].r_offset >= len) {
> +/* got last byte */
> +s->loc[locty].sts = STS_VALID;

[Qemu-devel] iSCSI support for QEMU, update

[Ronnie Sahlberg]

This is a patch that adds support for iSCSI to QEMU when QEMU is built against 
libiscsi (https://github.com/sahlberg/libiscsi)

ISCSI devices are specified using a special iSCSI URL:
  -drive file=iscsi://10.1.1.1:3260/iqn.ronnie.test/1


This allows a guest to access iscsi devices without exposing these devices to 
the host. Which is nice if/when you have a very large number of LUNs that are 
dedicated for virtual guests.


This patch is updated to set FUA for all writes, unless any of the NOCACHE or 
CACHE_WB flags are set for the device.


Please review and/or apply.



regards
ronnie sahlberg

[Qemu-devel] [PATCH] iSCSI block driver support

[Ronnie Sahlberg]

This patch adds a new block driver : block.iscsi.c
This driver interfaces with the multiplatform posix library
for iscsi initiator/client access to iscsi devices hosted at
git://github.com/sahlberg/libiscsi.git

The patch adds the driver to interface with the iscsi library.
It also updated the configure script to
* by default, probe is libiscsi is available and if so, build
  qemu against libiscsi.
* --enable-libiscsi
  Force a build against libiscsi. If libiscsi is not available
  the build will fail.
* --disable-libiscsi
  Do not link against libiscsi, even if it is available.

When linked with libiscsi, qemu gains support to access iscsi resources
such as disks and cdrom directly, without having to make the devices visible
to the host.

You can specify devices using a iscsi url of the form :
iscsi://[[:@]][:/
When using authentication, the password can optionally be set with
LIBISCSI_CHAP_PASSWORD="password" to avoid it showing up in the process list

Example:
-drive file=iscsi://10.1.1.1:3260/iqn.ronnie.test/1

Signed-off-by: Ronnie Sahlberg 
---
 Makefile.objs |1 +
 block/iscsi.c |  596 +
 configure |   31 +++
 trace-events  |6 +
 4 files changed, 634 insertions(+), 0 deletions(-)
 create mode 100644 block/iscsi.c

diff --git a/Makefile.objs b/Makefile.objs
index 9d8851e..fd394e0 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -25,6 +25,7 @@ block-nested-y += qed-check.o
 block-nested-y += parallels.o nbd.o blkdebug.o sheepdog.o blkverify.o
 block-nested-$(CONFIG_WIN32) += raw-win32.o
 block-nested-$(CONFIG_POSIX) += raw-posix.o
+block-nested-$(CONFIG_LIBISCSI) += iscsi.o
 block-nested-$(CONFIG_CURL) += curl.o
 block-nested-$(CONFIG_RBD) += rbd.o
 
diff --git a/block/iscsi.c b/block/iscsi.c
new file mode 100644
index 000..76773db
--- /dev/null
+++ b/block/iscsi.c
@@ -0,0 +1,596 @@
+/*
+ * QEMU Block driver for iSCSI images
+ *
+ * Copyright (c) 2010 Ronnie Sahlberg 
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "config-host.h"
+
+#include 
+#include "sysemu.h"
+#include "qemu-common.h"
+#include "qemu-error.h"
+#include "block_int.h"
+#include "trace.h"
+
+#include 
+#include 
+
+
+typedef struct IscsiLun {
+struct iscsi_context *iscsi;
+int lun;
+int block_size;
+unsigned long num_blocks;
+} IscsiLun;
+
+typedef struct IscsiAIOCB {
+BlockDriverAIOCB common;
+QEMUIOVector *qiov;
+QEMUBH *bh;
+IscsiLun *iscsilun;
+struct scsi_task *task;
+uint8_t *buf;
+int canceled;
+int status;
+size_t read_size;
+size_t read_offset;
+} IscsiAIOCB;
+
+struct IscsiTask {
+IscsiLun *iscsilun;
+int status;
+int complete;
+};
+
+static void
+iscsi_abort_task_cb(struct iscsi_context *iscsi, int status, void 
*command_data,
+   void *private_data)
+{
+}
+
+static void
+iscsi_aio_cancel(BlockDriverAIOCB *blockacb)
+{
+IscsiAIOCB *acb = (IscsiAIOCB *)blockacb;
+IscsiLun *iscsilun = acb->iscsilun;
+
+acb->status = -ECANCELED;
+acb->common.cb(acb->common.opaque, acb->status);
+acb->canceled = 1;
+
+iscsi_task_mgmt_abort_task_async(iscsilun->iscsi, acb->task,
+   iscsi_abort_task_cb, NULL);
+}
+
+static AIOPool iscsi_aio_pool = {
+.aiocb_size = sizeof(IscsiAIOCB),
+.cancel = iscsi_aio_cancel,
+};
+
+
+static void iscsi_process_read(void *arg);
+static void iscsi_process_write(void *arg);
+
+static int iscsi_process_flush(void *arg)
+{
+IscsiLun *iscsilun = arg;
+
+return iscsi_queue_length(iscsilun->iscsi) > 0;
+}
+
+static void
+iscsi_set_events(IscsiLun *iscsilun)
+{
+struct iscsi_context *iscsi = iscsilun->iscsi;
+
+qemu_aio_set_fd_handler(iscsi_get_fd(iscsi), iscsi_process_read,
+   (iscsi_which_events(iscsi) & POLLOUT)
+   ? iscsi_process_write : NULL,
+   iscsi