[Qemu-devel] Bluffakturor - så skyddar du dig

2010-06-29 Thread DokuMera Nyhetsbrev

Om du har problem med att läsa detta e-postmeddelande, klicka här 
(http://www.anp.se/newsletter/765761/444059437941455D4B7142445C43) för en 
webb-version.

Vårt nyhetsbrev skickas automatiskt till våra kunder och intressenter. Vill du 
inte ha detta nyhetsbrev framöver, klicka här för att avprenumerera 
(http://www.anp.se/oa/765761/444059437941455D4B7142445C43).

Nyhetsbrev 26/2010Detta nyhetsbrev är skickat till: qemu-devel@nongnu.org

 
(http://www.dokumera.se/newsletter_redirect.asp?to=http://www.dokumera.se&from=172191075&prefix=dm)
 (http://www.anp.se/taf/765761/444059437941455D4B7142445C43)  
(http://www.dokumera.se/newsletter_redirect.asp?to=http://www.dokumera.se/out_newsletters.asp&from=172191075&prefix=dm)
  
(http://www.anp.se/newsletter.asp?sqid=765761&sid=444059437941455D4B7142445C43&print=true)


Undvik att drabbas av bedragare i sommar

Med sommaren kommer bluffakturorna som ett brev på posten. I semestertider är 
många företag extra sårbara. När ordinarie personal är på semester, och 
fakturorna hanteras av vikarier eller annan personal som inte vanligtvis har 
detta som sitt arbete, ser många bedragare sin chans.

Det kan vara svårt att särskilja bluffakturor från äkta fakturor då bedragarna 
ofta använder sig av välkända företags varumärken på ett eller annat sätt. 
Därför är det viktigt att arbeta förebyggande mot bluffakturor. När bedragarna 
väl har slagit klorna i dig kan det bli kostsamt.

Du minskar risken att drabbas genom att:
- kritiskt granska alla fakturor som kommer in,
- se över attestrutinerna, 
- ge vikarier och annan personal som jobbar med fakturahantering tydliga 
instruktioner ,
- ordinarie personal, när de åter är på plats, kontrollerar fakturor som är 
lagda till betalning, 
- aldrig betala för något som du inte har beställt.

Hos DokuMera finner du bl. a. en checklista för hantering av bluffakturor. 
Dokumentet hjälper dig att hantera och förebygga bluffakturor. 

Med DokuMeras Företagspaket 
(http://www.dokumera.se/visa-kategorier.asp?id=1321) får du tillgång till fler 
än 1300 avtal, checklistor, policys, expertsvar och mycket mer som underlättar 
och juridiskt säkerställer arbetet i ditt företag. 


Veckans dokument 

Checklista - hantering av bluffakturor >> 
(http://www.dokumera.se/checklista_hantering_av_bluffakturor_4289_dd.html)

Bestridande av inkassokrav >> 
(http://www.dokumera.se/bestridande_av_inkassokrav_455_dd.html)

Attestinstruktion >> (http://www.dokumera.se/attestinstruktion_2646_dd.html)

Checklista attestrutiner >> 
(http://www.dokumera.se/checklista_attestrutiner_2648_dd.html)

Faktura >> (http://www.dokumera.se/faktura_i_set_om_25_1104_dd.html)


Sommarerbjudande

Just nu erbjuder vi dig att köpa Första dokumenthjälpen, boken som är 
fullspäckad med dokumentmallar och tips, för endast 199 kr (ord. pris 349 kr).

Erbjudandet gäller t.o.m. 31 augusti 2010.

Beställ här! (http://www.dokumera.se/forsta_dokumenthjalpen_4245_dd.html)


(http://www.dokumera.se/newsletter_redirect.asp?to=http://www.dokumera.se/foretagspaketet_1321_dc.html&from=172191075&prefix=dm)
 
(http://www.dokumera.se/newsletter_redirect.asp?to=http://www.dokumera.se/out_atq_ppdviewer.asp&from=172191075&prefix=dm)
 
(http://www.dokumera.se/newsletter_redirect.asp?to=http://www.dokumera.se/styckvisa_dokumentmallar_1330_dc.html&from=172191075&prefix=dm)

(http://www.dokumera.se/newsletter_redirect.asp?to=http://www.dokumera.se/out_contactusmessage.asp&from=172191075&prefix=dm)

För en kostnadsfri exklusiv presentation av hur DokuMera kan spara tiotusentals 
kronor åt just mitt företag.
Givetvis är du varmt välkommen att ringa oss på 08-664 04 50.

Innehållet i nyhetsbrev ska inte tolkas som ett åtagande från DokuMeras sida. 
Informationen sänds ut i befintligt skick, utan garantier och digitala 
signaturer.


[Qemu-devel] Re: [PATCH] Don't reset bs->is_temporary in bdrv_open_common

2010-06-29 Thread Kevin Wolf
Am 28.06.2010 16:38, schrieb Ryan Harper:
> To fix https://bugs.launchpad.net/qemu/+bug/597402 where qemu fails to
> call unlink() on temporary snapshots due to bs->is_temporary getting clobbered
> in bdrv_open_common() after being set in bdrv_open() which calls the former.
> 
> We don't need to initialize bs->is_temporary in bdrv_open_common().
> 
> Signed-off-by: Ryan Harper 

Thanks, applied to the block branch.

Kevin



[Qemu-devel] Re: [PATCH] device-assignment: Rework "name" of assigned pci device

2010-06-29 Thread Hidetoshi Seto
Thanks Markus,

(2010/06/29 14:28), Markus Armbruster wrote:
> Hidetoshi Seto  writes:
> 
>> "Hao, Xudong"  writes:
 When assign one PCI device, qemu fail to parse the command line:
 qemu-system_x86 -smp 2 -m 1024 -hda /path/to/img -pcidevice host=00:19.0
 Error:
 qemu-system-x86_64: Parameter 'id' expects an identifier
 Identifiers consist of letters, digits, '-', '.', '_', starting with a 
 letter.
 pcidevice argument parse error; please check the help text for usage
 Could not add assigned device host=00:19.0

 https://bugs.launchpad.net/qemu/+bug/597932

 This issue caused by qemu-kvm commit 
 b560a9ab9be06afcbb78b3791ab836dad208a239.
>>
>> This patch is a response to the above report.
>>
>> Thanks,
>> H.Seto
>>
>> =
>>
>> Because use of some characters in "id" is restricted recently, assigned
>> device start to fail having implicit "id" that uses address string of
>> host device, like "00:19.0" which includes restricted character ':'.
>>
>> It seems that this implicit "id" is intended to be run as "name" that
>> could be passed with option "-pcidevice ... ,name=..." to specify a
>> string to be used in log outputs.  In other words it seems that
>> dev->dev.qdev.id of assigned device had been used to have such
>> "name", that is user-defined string or address string of "host".
> 
> As far as I can tell, option "name" is just a leftover from pre-qdev
> days, kept for compatibility.

Yea, I see.
I don't know well about the history of such pre-qdev days...

>> The problem is that "name" for specific use is not equal to "id" for
>> universal use.  So it is better to remove this tricky mix up here.
>>
>> This patch introduces new function assigned_dev_name() that returns
>> proper name string for the device.
>> Now property "name" is explicitly defined in struct AssignedDevice.
>>
>> When if the device have neither "name" nor "id", address string like
>> ":00:19.0" will be created and passed instead.  Once created, new
>> field r_name holds the string to be reused and to be released later.
>>
>> Signed-off-by: Hidetoshi Seto 
> 
> Comments inline.
> 
>> ---
>>  hw/device-assignment.c |   59 
>> ++-
>>  hw/device-assignment.h |2 +
>>  2 files changed, 44 insertions(+), 17 deletions(-)
>>
>> diff --git a/hw/device-assignment.c b/hw/device-assignment.c
>> index 585162b..d73516f 100644
>> --- a/hw/device-assignment.c
>> +++ b/hw/device-assignment.c
>> @@ -62,6 +62,25 @@ static void assigned_dev_load_option_rom(AssignedDevice 
>> *dev);
>>  
>>  static void assigned_dev_unregister_msix_mmio(AssignedDevice *dev);
>>  
>> +static const char *assigned_dev_name(AssignedDevice *dev)
>> +{
>> +/* use user-defined "name" if specified */
>> +if (dev->u_name)
>> +return dev->u_name;
>> +/* else use "id" if available */
>> +if (dev->dev.qdev.id)
>> +return dev->dev.qdev.id;
>> +/* otherwise use address of host device instead */
>> +if (!dev->r_name) {
>> +char buf[32];
>> +
>> +snprintf(buf, sizeof(buf), "%04x:%02x:%02x.%01x",
>> + dev->host.seg, dev->host.bus, dev->host.dev, 
>> dev->host.func);
>> +dev->r_name = qemu_strdup(buf);
>> +}
>> +return dev->r_name;
>> +}
>> +
>>  static uint32_t guest_to_host_ioport(AssignedDevRegion *region, uint32_t 
>> addr)
>>  {
>>  return region->u.r_baseport + (addr - region->e_physbase);
>> @@ -798,6 +817,10 @@ static void free_assigned_device(AssignedDevice *dev)
>>  dev->real_device.config_fd = 0;
>>  }
>>  
>> +if (dev->r_name) {
>> +qemu_free(dev->r_name);
>> +}
>> +
>>  #ifdef KVM_CAP_IRQ_ROUTING
>>  free_dev_irq_entries(dev);
>>  #endif
>> @@ -885,7 +908,7 @@ static int assign_device(AssignedDevice *dev)
>>  if (dev->use_iommu) {
>>  if (!kvm_check_extension(kvm_state, KVM_CAP_IOMMU)) {
>>  fprintf(stderr, "No IOMMU found.  Unable to assign device 
>> \"%s\"\n",
>> -dev->dev.qdev.id);
>> +assigned_dev_name(dev));
>>  return -ENODEV;
>>  }
>>  assigned_dev_data.flags |= KVM_DEV_ASSIGN_ENABLE_IOMMU;
>> @@ -897,7 +920,7 @@ static int assign_device(AssignedDevice *dev)
>>  r = kvm_assign_pci_device(kvm_context, &assigned_dev_data);
>>  if (r < 0) {
>>  fprintf(stderr, "Failed to assign device \"%s\" : %s\n",
>> -dev->dev.qdev.id, strerror(-r));
>> +assigned_dev_name(dev), strerror(-r));
>>  
>>  switch (r) {
>>  case -EBUSY:
>> @@ -953,7 +976,7 @@ static int assign_irq(AssignedDevice *dev)
>>  r = kvm_assign_irq(kvm_context, &assigned_irq_data);
>>  if (r < 0) {
>>  fprintf(stderr, "Failed to assign irq for \"%s\": %s\n",
>> -dev->dev.qdev.id, strerror(-r));
>> +assigned_dev_name(dev), strerror(-r));
>>  fprintf(stderr, "Perhaps yo

[Qemu-devel] [Bug 584143] Re: qemu fails to set hdd serial number

2010-06-29 Thread Коренберг Марк
-drive file=/dev/mmwork/testl7,if=ide,index=0,boot=on,format=raw,serial
=WD-WMAM9TD73523,cache=writeback

Yeah, bug exists. only WD-W serial is seen inside VM.

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

Status in QEMU: In Progress
Status in “qemu-kvm” package in Ubuntu: Triaged
Status in “qemu-kvm” source package in Lucid: Confirmed
Status in “qemu-kvm” package in Debian: Unknown

Bug description:
The -drive ...,serial=xyz option is broken, at least in 0.12.  See Debian 
bug#573439, http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=573439 for details.

The proposed fix from the original reporter:

--- qemu-kvm-0.12.3+dfsg/vl.c   2010-02-26 11:34:00.0 +0900
+++ qemu-kvm-0.12.3+dfsg.old/vl.c   2010-03-11 02:26:00.134217787 +0900
@@ -2397,7 +2397,7 @@
 dinfo->on_write_error = on_write_error;
 dinfo->opts = opts;
 if (serial)
-strncpy(dinfo->serial, serial, sizeof(serial));
+strncpy(dinfo->serial, serial, sizeof(dinfo->serial));
 QTAILQ_INSERT_TAIL(&drives, dinfo, next);
 if (is_extboot) {
 extboot_drive = dinfo;





[Qemu-devel] Re: [PATCH 4/4] require #define NEED_GLOBAL_ENV for files that need the global register variable

2010-06-29 Thread Paolo Bonzini

On 06/28/2010 10:29 PM, Paul Brook wrote:

diff --git a/exec-all.h b/exec-all.h
index a775582..ebe88ad 100644
--- a/exec-all.h
+++ b/exec-all.h
@@ -353,4 +353,8 @@ extern int singlestep;
  /* cpu-exec.c */
  extern volatile sig_atomic_t exit_request;

+#ifdef NEED_GLOBAL_ENV
+register CPUState *env asm(AREG0);
+#endif


Wouldn't it be better to just put this in dyngen-exec.h ?
AFAICT there's a direct correlation between NEED_GLOBAL_ENV and #include
"exec.h".


True, see cover letter in 0/4.  I was told to make each file request 
explicitly the global variable though.  So I'd have to leave the #ifdef 
even if I moved it into dyngen-exec.h.


Paolo



[Qemu-devel] Re: [PATCH 1/4] remove unused stuff from */exec.h

2010-06-29 Thread Paolo Bonzini

On 06/28/2010 07:43 PM, Blue Swirl wrote:

IIRC these #undefs were added because some system includes on some
hosts also defined them. I think they should be replaced by open coded
versions (rather than using the QEMU_ prefix). Or just leave the
#undefs in place.


Right, I confused them with reg_EAX/reg_EBX/etc.  They were introduced in:

commit aba1d00a41d5aff5fc78bf924627b89a298a5a5a
Author: Blue Swirl 
Date:   Sat Sep 12 12:36:11 2009 +

Work around OpenSolaris sys/regset.h namespace pollution

Signed-off-by: Blue Swirl 

Paolo



[Qemu-devel] [PATCH v2 2/4] move cpu_pc_from_tb to target-*/exec.h

2010-06-29 Thread Paolo Bonzini
Signed-off-by: Paolo Bonzini 
---
 target-alpha/cpu.h   |5 -
 target-alpha/exec.h  |5 +
 target-arm/cpu.h |5 -
 target-arm/exec.h|6 ++
 target-cris/cpu.h|5 -
 target-cris/exec.h   |6 ++
 target-i386/cpu.h|5 -
 target-i386/exec.h   |6 ++
 target-m68k/cpu.h|5 -
 target-m68k/exec.h   |6 ++
 target-microblaze/cpu.h  |5 -
 target-microblaze/exec.h |6 ++
 target-mips/cpu.h|7 ---
 target-mips/exec.h   |7 +++
 target-ppc/cpu.h |5 -
 target-ppc/exec.h|5 +
 target-s390x/cpu.h   |5 -
 target-s390x/exec.h  |6 ++
 target-sh4/cpu.h |6 --
 target-sh4/exec.h|6 ++
 target-sparc/cpu.h   |6 --
 target-sparc/exec.h  |6 ++
 22 files changed, 65 insertions(+), 59 deletions(-)

diff --git a/target-alpha/cpu.h b/target-alpha/cpu.h
index 314d6ac..c96 100644
--- a/target-alpha/cpu.h
+++ b/target-alpha/cpu.h
@@ -512,11 +512,6 @@ void pal_init (CPUState *env);
 void call_pal (CPUState *env);
 #endif
 
-static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
-{
-env->pc = tb->pc;
-}
-
 static inline void cpu_get_tb_cpu_state(CPUState *env, target_ulong *pc,
 target_ulong *cs_base, int *flags)
 {
diff --git a/target-alpha/exec.h b/target-alpha/exec.h
index 0fb459d..a8a38d2 100644
--- a/target-alpha/exec.h
+++ b/target-alpha/exec.h
@@ -53,4 +53,9 @@ static inline int cpu_halted(CPUState *env)
 return EXCP_HALTED;
 }
 
+static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
+{
+env->pc = tb->pc;
+}
+
 #endif /* !defined (__ALPHA_EXEC_H__) */
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index f3d138d..ddf764e 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -437,11 +437,6 @@ static inline void cpu_clone_regs(CPUState *env, 
target_ulong newsp)
 #include "cpu-all.h"
 #include "exec-all.h"
 
-static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
-{
-env->regs[15] = tb->pc;
-}
-
 static inline void cpu_get_tb_cpu_state(CPUState *env, target_ulong *pc,
 target_ulong *cs_base, int *flags)
 {
diff --git a/target-arm/exec.h b/target-arm/exec.h
index 0225c3f..e4c35a3 100644
--- a/target-arm/exec.h
+++ b/target-arm/exec.h
@@ -50,3 +50,9 @@ static inline int cpu_halted(CPUState *env) {
 #endif
 
 void raise_exception(int);
+
+static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
+{
+env->regs[15] = tb->pc;
+}
+
diff --git a/target-cris/cpu.h b/target-cris/cpu.h
index a62d57c..f86c52a 100644
--- a/target-cris/cpu.h
+++ b/target-cris/cpu.h
@@ -252,11 +252,6 @@ static inline void cpu_set_tls(CPUCRISState *env, 
target_ulong newtls)
 #include "cpu-all.h"
 #include "exec-all.h"
 
-static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
-{
-env->pc = tb->pc;
-}
-
 static inline void cpu_get_tb_cpu_state(CPUState *env, target_ulong *pc,
 target_ulong *cs_base, int *flags)
 {
diff --git a/target-cris/exec.h b/target-cris/exec.h
index 55776ba..93ce768 100644
--- a/target-cris/exec.h
+++ b/target-cris/exec.h
@@ -45,3 +45,9 @@ static inline int cpu_halted(CPUState *env) {
}
return EXCP_HALTED;
 }
+
+static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
+{
+env->pc = tb->pc;
+}
+
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 8dafa0d..e320c80 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -936,11 +936,6 @@ static inline void cpu_clone_regs(CPUState *env, 
target_ulong newsp)
 #include "hw/apic.h"
 #endif
 
-static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
-{
-env->eip = tb->pc - tb->cs_base;
-}
-
 static inline void cpu_get_tb_cpu_state(CPUState *env, target_ulong *pc,
 target_ulong *cs_base, int *flags)
 {
diff --git a/target-i386/exec.h b/target-i386/exec.h
index 4ff3c57..fc8945b 100644
--- a/target-i386/exec.h
+++ b/target-i386/exec.h
@@ -327,3 +327,9 @@ static inline void cpu_load_efer(CPUState *env, uint64_t 
val)
 if (env->efer & MSR_EFER_SVME)
 env->hflags |= HF_SVME_MASK;
 }
+
+static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
+{
+env->eip = tb->pc - tb->cs_base;
+}
+
diff --git a/target-m68k/cpu.h b/target-m68k/cpu.h
index b2f37ec..0f84514 100644
--- a/target-m68k/cpu.h
+++ b/target-m68k/cpu.h
@@ -244,11 +244,6 @@ static inline void cpu_clone_regs(CPUState *env, 
target_ulong newsp)
 #include "cpu-all.h"
 #include "exec-all.h"
 
-static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
-{
-env->pc = tb->pc;
-}
-
 static inline void cpu_get_tb_cpu_state(CPUState *env, target_ulong *pc,
 target_ulong *cs_base, int

[Qemu-devel] [PATCH v2 0/4] introduce NEED_GLOBAL_ENV

2010-06-29 Thread Paolo Bonzini
Let's start the cleanups from the feature required by Blue Swirl.
I also include here a baby step towards removing eminently TCG-related
stuff from cpu.h.

After this series, only a bunch of files will include exec-all.h,
instead of getting it indirectly from cpu.h.

Note that (as sworn in the previous submission) exec.h is only included
by files that need the global register variable (i.e. cpu-exec.c and
target-*/op_helper.c), and this is the same subset that gets
NEED_GLOBAL_ENV in this patchset.

i386 and sparc have functions declared in cpu.h that are in op_helper.c.
I checked that these do not need the global variable, but it would be
nice to cleanup those too.

v1->v2: leave in #undefs, add static to cpu_mips_tlb_flush

Paolo Bonzini (4):
  remove unused stuff from */exec.h
  move cpu_pc_from_tb to target-*/exec.h
  remove exec-all.h inclusion from cpu.h
  require #define NEED_GLOBAL_ENV for files that need the global
register variable

 cpu-exec.c|2 ++
 exec-all.h|4 
 gdbstub.c |1 +
 hw/xen_domainbuild.c  |1 +
 kvm-stub.c|1 +
 monitor.c |1 +
 target-alpha/cpu.h|6 --
 target-alpha/exec.h   |9 +
 target-alpha/op_helper.c  |1 +
 target-arm/cpu.h  |6 --
 target-arm/exec.h |8 ++--
 target-arm/op_helper.c|1 +
 target-cris/cpu.h |6 --
 target-cris/exec.h|   11 ++-
 target-cris/op_helper.c   |1 +
 target-i386/cpu.h |7 ---
 target-i386/exec.h|8 ++--
 target-i386/op_helper.c   |3 ++-
 target-m68k/cpu.h |6 --
 target-m68k/exec.h|8 ++--
 target-m68k/op_helper.c   |2 ++
 target-microblaze/cpu.h   |6 --
 target-microblaze/exec.h  |   10 ++
 target-microblaze/op_helper.c |1 +
 target-mips/cpu.h |8 
 target-mips/exec.h|   17 +++--
 target-mips/op_helper.c   |9 -
 target-ppc/cpu.h  |6 --
 target-ppc/exec.h |7 +--
 target-ppc/op_helper.c|2 ++
 target-s390x/cpu.h|6 --
 target-s390x/exec.h   |8 ++--
 target-s390x/op_helper.c  |1 +
 target-sh4/cpu.h  |7 ---
 target-sh4/exec.h |8 ++--
 target-sh4/op_helper.c|2 ++
 target-sparc/cpu.h|7 ---
 target-sparc/exec.h   |8 ++--
 target-sparc/op_helper.c  |1 +
 39 files changed, 97 insertions(+), 110 deletions(-)




[Qemu-devel] [PATCH v2 1/4] remove unused stuff from */exec.h

2010-06-29 Thread Paolo Bonzini
Signed-off-by: Paolo Bonzini 
---
 target-alpha/exec.h  |2 --
 target-cris/exec.h   |3 ---
 target-i386/op_helper.c  |2 +-
 target-microblaze/exec.h |2 --
 target-mips/exec.h   |8 
 target-mips/op_helper.c  |7 ++-
 6 files changed, 7 insertions(+), 17 deletions(-)

diff --git a/target-alpha/exec.h b/target-alpha/exec.h
index 66526e2..0fb459d 100644
--- a/target-alpha/exec.h
+++ b/target-alpha/exec.h
@@ -28,8 +28,6 @@
 
 register struct CPUAlphaState *env asm(AREG0);
 
-#define PARAM(n) ((uint64_t)PARAM##n)
-#define SPARAM(n) ((int32_t)PARAM##n)
 #define FP_STATUS (env->fp_status)
 
 #include "cpu.h"
diff --git a/target-cris/exec.h b/target-cris/exec.h
index 728aa80..55776ba 100644
--- a/target-cris/exec.h
+++ b/target-cris/exec.h
@@ -28,9 +28,6 @@ register struct CPUCRISState *env asm(AREG0);
 #include "softmmu_exec.h"
 #endif
 
-void cpu_cris_flush_flags(CPUCRISState *env, int cc_op);
-void helper_movec(CPUCRISState *env, int reg, uint32_t val);
-
 static inline int cpu_has_work(CPUState *env)
 {
 return (env->interrupt_request & (CPU_INTERRUPT_HARD | CPU_INTERRUPT_NMI));
diff --git a/target-i386/op_helper.c b/target-i386/op_helper.c
index c1256f4..00fc671 100644
--- a/target-i386/op_helper.c
+++ b/target-i386/op_helper.c
@@ -16,7 +16,7 @@
  * You should have received a copy of the GNU Lesser General Public
  * License along with this library; if not, see .
  */
-#define CPU_NO_GLOBAL_REGS
+
 #include "exec.h"
 #include "exec-all.h"
 #include "host-utils.h"
diff --git a/target-microblaze/exec.h b/target-microblaze/exec.h
index 646701c..db1c99e 100644
--- a/target-microblaze/exec.h
+++ b/target-microblaze/exec.h
@@ -27,8 +27,6 @@ register struct CPUMBState *env asm(AREG0);
 #include "softmmu_exec.h"
 #endif
 
-void cpu_mb_flush_flags(CPUMBState *env, int cc_op);
-
 static inline int cpu_has_work(CPUState *env)
 {
 return (env->interrupt_request & (CPU_INTERRUPT_HARD | CPU_INTERRUPT_NMI));
diff --git a/target-mips/exec.h b/target-mips/exec.h
index 01e9c4d..a07761d 100644
--- a/target-mips/exec.h
+++ b/target-mips/exec.h
@@ -17,14 +17,6 @@ register struct CPUMIPSState *env asm(AREG0);
 #include "softmmu_exec.h"
 #endif /* !defined(CONFIG_USER_ONLY) */
 
-void dump_fpu(CPUState *env);
-void fpu_dump_state(CPUState *env, FILE *f,
-int (*fpu_fprintf)(FILE *f, const char *fmt, ...),
-int flags);
-
-void cpu_mips_clock_init (CPUState *env);
-void cpu_mips_tlb_flush (CPUState *env, int flush_global);
-
 static inline int cpu_has_work(CPUState *env)
 {
 return (env->interrupt_request &
diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
index d09d6ed..8ae510a 100644
--- a/target-mips/op_helper.c
+++ b/target-mips/op_helper.c
@@ -22,6 +22,11 @@
 #include "host-utils.h"
 
 #include "helper.h"
+
+#ifndef CONFIG_USER_ONLY
+static inline void cpu_mips_tlb_flush (CPUState *env, int flush_global);
+#endif
+
 /*/
 /* Exceptions processing helpers */
 
@@ -1635,7 +1640,7 @@ target_ulong helper_yield(target_ulong arg1)
 
 #ifndef CONFIG_USER_ONLY
 /* TLB management */
-void cpu_mips_tlb_flush (CPUState *env, int flush_global)
+static void cpu_mips_tlb_flush (CPUState *env, int flush_global)
 {
 /* Flush qemu's TLB and discard all shadowed entries.  */
 tlb_flush (env, flush_global);
-- 
1.7.0.1





[Qemu-devel] [PATCH v2 3/4] remove exec-all.h inclusion from cpu.h

2010-06-29 Thread Paolo Bonzini
Signed-off-by: Paolo Bonzini 
---
 gdbstub.c   |1 +
 hw/xen_domainbuild.c|1 +
 kvm-stub.c  |1 +
 monitor.c   |1 +
 target-alpha/cpu.h  |1 -
 target-arm/cpu.h|1 -
 target-cris/cpu.h   |1 -
 target-i386/cpu.h   |2 --
 target-m68k/cpu.h   |1 -
 target-microblaze/cpu.h |1 -
 target-mips/cpu.h   |1 -
 target-ppc/cpu.h|1 -
 target-s390x/cpu.h  |1 -
 target-sh4/cpu.h|1 -
 target-sparc/cpu.h  |1 -
 15 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index c1852c2..2b03ef2 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -37,6 +37,7 @@
 
 #define MAX_PACKET_LENGTH 4096
 
+#include "exec-all.h"
 #include "qemu_socket.h"
 #include "kvm.h"
 
diff --git a/hw/xen_domainbuild.c b/hw/xen_domainbuild.c
index 2f59856..7f1fd66 100644
--- a/hw/xen_domainbuild.c
+++ b/hw/xen_domainbuild.c
@@ -3,6 +3,7 @@
 #include "xen_domainbuild.h"
 #include "sysemu.h"
 #include "qemu-timer.h"
+#include "qemu-log.h"
 
 #include 
 
diff --git a/kvm-stub.c b/kvm-stub.c
index 7be5f5d..3378bd3 100644
--- a/kvm-stub.c
+++ b/kvm-stub.c
@@ -13,6 +13,7 @@
 #include "qemu-common.h"
 #include "sysemu.h"
 #include "hw/hw.h"
+#include "exec-all.h"
 #include "gdbstub.h"
 #include "kvm.h"
 
diff --git a/monitor.c b/monitor.c
index 170b269..8657c86 100644
--- a/monitor.c
+++ b/monitor.c
@@ -55,6 +55,7 @@
 #include "json-streamer.h"
 #include "json-parser.h"
 #include "osdep.h"
+#include "exec-all.h"
 
 //#define DEBUG
 //#define DEBUG_COMPLETION
diff --git a/target-alpha/cpu.h b/target-alpha/cpu.h
index c96..686fb4a 100644
--- a/target-alpha/cpu.h
+++ b/target-alpha/cpu.h
@@ -413,7 +413,6 @@ static inline int cpu_mmu_index (CPUState *env)
 }
 
 #include "cpu-all.h"
-#include "exec-all.h"
 
 enum {
 FEATURE_ASN= 0x0001,
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index ddf764e..39c4a0e 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -435,7 +435,6 @@ static inline void cpu_clone_regs(CPUState *env, 
target_ulong newsp)
 #endif
 
 #include "cpu-all.h"
-#include "exec-all.h"
 
 static inline void cpu_get_tb_cpu_state(CPUState *env, target_ulong *pc,
 target_ulong *cs_base, int *flags)
diff --git a/target-cris/cpu.h b/target-cris/cpu.h
index f86c52a..fce0804 100644
--- a/target-cris/cpu.h
+++ b/target-cris/cpu.h
@@ -250,7 +250,6 @@ static inline void cpu_set_tls(CPUCRISState *env, 
target_ulong newtls)
 #define SFR_RW_MM_TLB_HI   env->pregs[PR_SRS]][6
 
 #include "cpu-all.h"
-#include "exec-all.h"
 
 static inline void cpu_get_tb_cpu_state(CPUState *env, target_ulong *pc,
 target_ulong *cs_base, int *flags)
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index e320c80..e0a039e 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -928,8 +928,6 @@ static inline void cpu_clone_regs(CPUState *env, 
target_ulong newsp)
 #endif
 
 #include "cpu-all.h"
-#include "exec-all.h"
-
 #include "svm.h"
 
 #if !defined(CONFIG_USER_ONLY)
diff --git a/target-m68k/cpu.h b/target-m68k/cpu.h
index 0f84514..33c41b2 100644
--- a/target-m68k/cpu.h
+++ b/target-m68k/cpu.h
@@ -242,7 +242,6 @@ static inline void cpu_clone_regs(CPUState *env, 
target_ulong newsp)
 #endif
 
 #include "cpu-all.h"
-#include "exec-all.h"
 
 static inline void cpu_get_tb_cpu_state(CPUState *env, target_ulong *pc,
 target_ulong *cs_base, int *flags)
diff --git a/target-microblaze/cpu.h b/target-microblaze/cpu.h
index a2677cf..360ac0a 100644
--- a/target-microblaze/cpu.h
+++ b/target-microblaze/cpu.h
@@ -305,7 +305,6 @@ static inline int cpu_interrupts_enabled(CPUState *env)
 }
 
 #include "cpu-all.h"
-#include "exec-all.h"
 
 static inline target_ulong cpu_get_pc(CPUState *env)
 {
diff --git a/target-mips/cpu.h b/target-mips/cpu.h
index 1aaca77..81051aa 100644
--- a/target-mips/cpu.h
+++ b/target-mips/cpu.h
@@ -526,7 +526,6 @@ static inline void cpu_clone_regs(CPUState *env, 
target_ulong newsp)
 }
 
 #include "cpu-all.h"
-#include "exec-all.h"
 
 /* Memory access type :
  * may be needed for precise access rights control and precise exceptions.
diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index ca0eb1e..9c8d774 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -849,7 +849,6 @@ static inline void cpu_clone_regs(CPUState *env, 
target_ulong newsp)
 #endif
 
 #include "cpu-all.h"
-#include "exec-all.h"
 
 /*/
 /* CRF definitions */
diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index 49d3128..8d73fad 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -116,7 +116,6 @@ extern CPUState *s390_cpu_addr2state(uint16_t cpu_addr);
 #define cpu_gen_code cpu_s390x_gen_code
 
 #include "cpu-all.h"
-#include "exec-all.h"
 
 #define EXCP_OPEX 1 /* operation exception (sigill) */
 #define EXCP_SVC

[Qemu-devel] [PATCH v2 4/4] require #define NEED_GLOBAL_ENV for files that need the global register variable

2010-06-29 Thread Paolo Bonzini
Signed-off-by: Paolo Bonzini 
---
 cpu-exec.c|2 ++
 exec-all.h|4 
 target-alpha/exec.h   |2 --
 target-alpha/op_helper.c  |1 +
 target-arm/exec.h |2 --
 target-arm/op_helper.c|1 +
 target-cris/exec.h|2 --
 target-cris/op_helper.c   |1 +
 target-i386/exec.h|2 --
 target-i386/op_helper.c   |1 +
 target-m68k/exec.h|2 --
 target-m68k/op_helper.c   |2 ++
 target-microblaze/exec.h  |2 --
 target-microblaze/op_helper.c |1 +
 target-mips/exec.h|2 --
 target-mips/op_helper.c   |2 ++
 target-ppc/exec.h |2 --
 target-ppc/op_helper.c|2 ++
 target-s390x/exec.h   |2 --
 target-s390x/op_helper.c  |1 +
 target-sh4/exec.h |2 --
 target-sh4/op_helper.c|2 ++
 target-sparc/exec.h   |2 --
 target-sparc/op_helper.c  |1 +
 24 files changed, 21 insertions(+), 22 deletions(-)

diff --git a/cpu-exec.c b/cpu-exec.c
index 026980a..5d5170f 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -16,7 +16,9 @@
  * You should have received a copy of the GNU Lesser General Public
  * License along with this library; if not, see .
  */
+
 #include "config.h"
+#define NEED_GLOBAL_ENV
 #include "exec.h"
 #include "disas.h"
 #include "tcg.h"
diff --git a/exec-all.h b/exec-all.h
index a775582..ebe88ad 100644
--- a/exec-all.h
+++ b/exec-all.h
@@ -353,4 +353,8 @@ extern int singlestep;
 /* cpu-exec.c */
 extern volatile sig_atomic_t exit_request;
 
+#ifdef NEED_GLOBAL_ENV
+register CPUState *env asm(AREG0);
+#endif
+
 #endif
diff --git a/target-alpha/exec.h b/target-alpha/exec.h
index a8a38d2..1950f83 100644
--- a/target-alpha/exec.h
+++ b/target-alpha/exec.h
@@ -26,8 +26,6 @@
 
 #define TARGET_LONG_BITS 64
 
-register struct CPUAlphaState *env asm(AREG0);
-
 #define FP_STATUS (env->fp_status)
 
 #include "cpu.h"
diff --git a/target-alpha/op_helper.c b/target-alpha/op_helper.c
index ff5ae26..9870d97 100644
--- a/target-alpha/op_helper.c
+++ b/target-alpha/op_helper.c
@@ -17,6 +17,7 @@
  * License along with this library; if not, see .
  */
 
+#define NEED_GLOBAL_ENV
 #include "exec.h"
 #include "host-utils.h"
 #include "softfloat.h"
diff --git a/target-arm/exec.h b/target-arm/exec.h
index e4c35a3..eda5632 100644
--- a/target-arm/exec.h
+++ b/target-arm/exec.h
@@ -19,8 +19,6 @@
 #include "config.h"
 #include "dyngen-exec.h"
 
-register struct CPUARMState *env asm(AREG0);
-
 #define M0   env->iwmmxt.val
 
 #include "cpu.h"
diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index 9b1a014..235cffc 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -16,6 +16,7 @@
  * You should have received a copy of the GNU Lesser General Public
  * License along with this library; if not, see .
  */
+#define NEED_GLOBAL_ENV
 #include "exec.h"
 #include "helpers.h"
 
diff --git a/target-cris/exec.h b/target-cris/exec.h
index 93ce768..360f45a 100644
--- a/target-cris/exec.h
+++ b/target-cris/exec.h
@@ -19,8 +19,6 @@
  */
 #include "dyngen-exec.h"
 
-register struct CPUCRISState *env asm(AREG0);
-
 #include "cpu.h"
 #include "exec-all.h"
 
diff --git a/target-cris/op_helper.c b/target-cris/op_helper.c
index a60da94..8bb3876 100644
--- a/target-cris/op_helper.c
+++ b/target-cris/op_helper.c
@@ -18,6 +18,7 @@
  * License along with this library; if not, see .
  */
 
+#define NEED_GLOBAL_ENV
 #include "exec.h"
 #include "mmu.h"
 #include "helper.h"
diff --git a/target-i386/exec.h b/target-i386/exec.h
index fc8945b..b002805 100644
--- a/target-i386/exec.h
+++ b/target-i386/exec.h
@@ -28,8 +28,6 @@
 
 #include "cpu-defs.h"
 
-register struct CPUX86State *env asm(AREG0);
-
 #include "qemu-common.h"
 #include "qemu-log.h"
 
diff --git a/target-i386/op_helper.c b/target-i386/op_helper.c
index 00fc671..05b6340 100644
--- a/target-i386/op_helper.c
+++ b/target-i386/op_helper.c
@@ -17,6 +17,7 @@
  * License along with this library; if not, see .
  */
 
+#define NEED_GLOBAL_ENV
 #include "exec.h"
 #include "exec-all.h"
 #include "host-utils.h"
diff --git a/target-m68k/exec.h b/target-m68k/exec.h
index f31e06e..ac1cf12 100644
--- a/target-m68k/exec.h
+++ b/target-m68k/exec.h
@@ -19,8 +19,6 @@
  */
 #include "dyngen-exec.h"
 
-register struct CPUM68KState *env asm(AREG0);
-
 #include "cpu.h"
 #include "exec-all.h"
 
diff --git a/target-m68k/op_helper.c b/target-m68k/op_helper.c
index 0711107..df4489f 100644
--- a/target-m68k/op_helper.c
+++ b/target-m68k/op_helper.c
@@ -16,6 +16,8 @@
  * You should have received a copy of the GNU Lesser General Public
  * License along with this library; if not, see .
  */
+
+#define NEED_GLOBAL_ENV
 #include "exec.h"
 #include "helpers.h"
 
diff --git a/ta

Re: [Qemu-devel] Re: [PATCH 08/12] block: Catch attempt to attach multiple devices to a blockdev

2010-06-29 Thread Gerd Hoffmann

  Hi,


Note that currently the usb storage emulation is extremly broken anyway,
just writing to it produces I/O errors after a short while.  This means
it can't be used very much at all.


When I tried last time, it did produce lots of kernel error messages in
the guest, but in the end the data was written correctly. So it doesn't
seem to be completely unusable.


I think it is actually the linux kernel being at fault here.  I've seen 
I/O errors on real hardware too, on my pretty old laptop which has usb 
1.1 only.  Probably something like timeouts being *way* too low for usb 
1.1 xfer speeds.


cheers,
  Gerd



[Qemu-devel] [PATCH v2] device-assignment: Rework "name" of assigned pci device

2010-06-29 Thread Hidetoshi Seto
Because use of some characters in "id" is restricted recently,
assigned device start to fail having implicit "id" that uses
address string of host device, like "00:19.0" which includes
restricted character ':'.

It seems that this implicit "id" is intended to be run as "name"
that could be passed with option "-pcidevice ... ,name=..." to
specify a string to be used in log outputs.  In other words it
seems that dev->dev.qdev.id of assigned device had been used to
point such "name", that is user-defined string or address string
of "host".

The problem is that "name" for specific use is not equal to "id"
for universal use.  So it is better to remove this tricky mix up.

This patch introduces new function assigned_dev_name() that
returns proper name string for the device.
Now property "name" is explicitly defined in struct AssignedDevice.

When if the device have neither "name" nor "id", address string
like ":00:19.0" will be created and passed instead.
Once created, new field r_name holds the string to be reused and
to be released later.

v2: - Pass id to qemu_opts_create if "id=" is specified even it is
  an undocumented option for -pcidevice and pci_add.
- Drop minor cleanup & unrelated changes.

Signed-off-by: Hidetoshi Seto 
---
 hw/device-assignment.c |   52 
 hw/device-assignment.h |2 +
 2 files changed, 41 insertions(+), 13 deletions(-)

diff --git a/hw/device-assignment.c b/hw/device-assignment.c
index 585162b..cda6ee3 100644
--- a/hw/device-assignment.c
+++ b/hw/device-assignment.c
@@ -62,6 +62,25 @@ static void assigned_dev_load_option_rom(AssignedDevice 
*dev);
 
 static void assigned_dev_unregister_msix_mmio(AssignedDevice *dev);
 
+static const char *assigned_dev_name(AssignedDevice *dev)
+{
+/* use user-defined "name" if specified */
+if (dev->u_name)
+return dev->u_name;
+/* else use "id" if available */
+if (dev->dev.qdev.id)
+return dev->dev.qdev.id;
+/* otherwise use address of host device instead */
+if (!dev->r_name) {
+char buf[32];
+
+snprintf(buf, sizeof(buf), "%04x:%02x:%02x.%01x",
+ dev->host.seg, dev->host.bus, dev->host.dev, dev->host.func);
+dev->r_name = qemu_strdup(buf);
+}
+return dev->r_name;
+}
+
 static uint32_t guest_to_host_ioport(AssignedDevRegion *region, uint32_t addr)
 {
 return region->u.r_baseport + (addr - region->e_physbase);
@@ -798,6 +817,10 @@ static void free_assigned_device(AssignedDevice *dev)
 dev->real_device.config_fd = 0;
 }
 
+if (dev->r_name) {
+qemu_free(dev->r_name);
+}
+
 #ifdef KVM_CAP_IRQ_ROUTING
 free_dev_irq_entries(dev);
 #endif
@@ -885,7 +908,7 @@ static int assign_device(AssignedDevice *dev)
 if (dev->use_iommu) {
 if (!kvm_check_extension(kvm_state, KVM_CAP_IOMMU)) {
 fprintf(stderr, "No IOMMU found.  Unable to assign device 
\"%s\"\n",
-dev->dev.qdev.id);
+assigned_dev_name(dev));
 return -ENODEV;
 }
 assigned_dev_data.flags |= KVM_DEV_ASSIGN_ENABLE_IOMMU;
@@ -897,7 +920,7 @@ static int assign_device(AssignedDevice *dev)
 r = kvm_assign_pci_device(kvm_context, &assigned_dev_data);
 if (r < 0) {
 fprintf(stderr, "Failed to assign device \"%s\" : %s\n",
-dev->dev.qdev.id, strerror(-r));
+assigned_dev_name(dev), strerror(-r));
 
 switch (r) {
 case -EBUSY:
@@ -953,7 +976,7 @@ static int assign_irq(AssignedDevice *dev)
 r = kvm_assign_irq(kvm_context, &assigned_irq_data);
 if (r < 0) {
 fprintf(stderr, "Failed to assign irq for \"%s\": %s\n",
-dev->dev.qdev.id, strerror(-r));
+assigned_dev_name(dev), strerror(-r));
 fprintf(stderr, "Perhaps you are assigning a device "
 "that shares an IRQ with another device?\n");
 return r;
@@ -977,7 +1000,7 @@ static void deassign_device(AssignedDevice *dev)
 r = kvm_deassign_pci_device(kvm_context, &assigned_dev_data);
 if (r < 0)
fprintf(stderr, "Failed to deassign device \"%s\" : %s\n",
-dev->dev.qdev.id, strerror(-r));
+assigned_dev_name(dev), strerror(-r));
 #endif
 }
 
@@ -1421,7 +1444,7 @@ static int assigned_initfn(struct PCIDevice *pci_dev)
 if (get_real_device(dev, dev->host.seg, dev->host.bus,
 dev->host.dev, dev->host.func)) {
 error_report("pci-assign: Error: Couldn't get real device (%s)!",
- dev->dev.qdev.id);
+ assigned_dev_name(dev));
 goto out;
 }
 
@@ -1520,6 +1543,7 @@ static PCIDeviceInfo assign_info = {
 DEFINE_PROP("host", AssignedDevice, host, qdev_prop_hostaddr, 
PCIHostDevice),
 DEFINE_PROP_UINT32("iommu", AssignedDevice, use_iommu, 1),
 DEFINE_PROP_STRING("configfd", AssignedDevice, configfd_nam

[Qemu-devel] [PATCH] make qemu.log name unique

2010-06-29 Thread Christophe LYON

Hello,

I propose this small patch so that the qemu log file has a unique name, 
to help running several QEmu processes at once (or with different users).


Is it OK?

Thanks

Christophe.

diff --git a/exec.c b/exec.c
index 5969eb2..5ba8d7e 100644
--- a/exec.c
+++ b/exec.c
@@ -1516,7 +1516,10 @@ void cpu_set_log(int log_flags)

 void cpu_set_log_filename(const char *filename)
 {
-logfilename = strdup(filename);
+/* Assume 10 chars is enough to hold pid */
+int len = strlen(filename) + 10 + 1;
+logfilename = malloc(len);
+sprintf((char*)logfilename, "%s.%d", filename, getpid());
 if (logfile) {
 fclose(logfile);
 logfile = NULL;



[Qemu-devel] [PATCH] Fix null pointer dereference when parsing chardevs without a backend option.

2010-06-29 Thread Mike McCormack


qemu_opt_get may return NULL, so handle that rather than crashing.

Signed-off-by: Mike McCormack 
---
qemu-char.c |9 -
1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index 9b69d92..f292ee7 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2434,6 +2434,7 @@ CharDriverState *qemu_chr_open_opts(QemuOpts *opts,
void (*init)(struct CharDriverState *s))
{
CharDriverState *chr;
+const char *backend;
int i;

if (qemu_opts_id(opts) == NULL) {
@@ -2441,8 +2442,14 @@ CharDriverState *qemu_chr_open_opts(QemuOpts *opts,
return NULL;
}

+backend = qemu_opt_get(opts, "backend");
+if (!backend) {
+fprintf(stderr, "chardev: backend option not specified\n");
+return NULL;
+}
+
for (i = 0; i < ARRAY_SIZE(backend_table); i++) {
-if (strcmp(backend_table[i].name, qemu_opt_get(opts, "backend")) == 0)
+if (strcmp(backend_table[i].name, backend) == 0)
break;
}
if (i == ARRAY_SIZE(backend_table)) {
--
1.5.4.3



[Qemu-devel] Re: [PATCH 04/12] blockdev: New drive_of_blockdev()

2010-06-29 Thread Markus Armbruster
Paolo Bonzini  writes:

> On 06/26/2010 04:46 PM, Markus Armbruster wrote:
>> drive_get_by_blockdev()?  blockdev_to_drive()?
>
> I like drive_find_by_blockdev or drive_get_by_blockdev, in any case
> I'll make sure my own cpu_get_by_id is named in the same style that
> you choose.

drive_get_by_blockdev() it is.

As long as this is the only change, I don't think another post to the
list is necessary.  Instead, I'll push to my tree and let Kevin pick it
up from there.



Re: [Qemu-devel] [Bug 599574] [NEW] qemu-kvm: -no-reboot option broken in 12.x

2010-06-29 Thread Stefan Hajnoczi
On Mon, Jun 28, 2010 at 10:46 PM, Zach Carter  wrote:
> Public bug reported:
>
> When using the "-no-reboot" qemu option with kvm, qemu does nothing and
> immediately exits with no output or error message.   If I add the --no-
> kvm option to the command line, it works as expected.
>
> It works fine in 11.0 and 11.1, but I tested all versions of 12.X, and
> they all have this problem.
>
> ** Affects: qemu
>     Importance: Undecided
>         Status: New
>

Please post the qemu-kvm command-line used to start the VM.  I am
unable to reproduce this with qemu.git, qemu-kvm.git, or Debian kvm
0.12.4+dfsg-1.

Either the issue is fixed or I wasn't able to reproduce it with:

$ kvm -no-reboot -m 512 -cdrom myiso.iso

Stefan



Re: [Qemu-devel] [PATCH] make qemu.log name unique

2010-06-29 Thread Kevin Wolf
Am 29.06.2010 10:46, schrieb Christophe LYON:
> Hello,
> 
> I propose this small patch so that the qemu log file has a unique name, 
> to help running several QEmu processes at once (or with different users).
> 
> Is it OK?

I don't think it's a good idea. When debugging things I usually run qemu
several times, and each time I'd have to look up which of all the
/tmp/qemu.log.* files is the current one instead of just refreshing the
qemu.log that I'm viewing.

Maybe adding a -logfile option would allow what you're trying to achieve
without affecting other use cases? I've always thought that it's strange
that you can only change the logfile location in the monitor and not on
the command line.

Kevin



[Qemu-devel] [PATCH] trace: Use GENERATED_HEADERS dependencies

2010-06-29 Thread Stefan Hajnoczi
Generated header files including trace.h and config-host.h are required
for building trace code.  Instead of explictly specifying these files,
use the GENERATED_HEADERS variable that is also used elsewhere.

Signed-off-by: Stefan Hajnoczi 
---
This patch applies against the tracing branch:

http://repo.or.cz/w/qemu/stefanha.git/shortlog/refs/heads/tracing

 Makefile |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index de9b175..8831174 100644
--- a/Makefile
+++ b/Makefile
@@ -138,9 +138,9 @@ trace.h: $(SRC_PATH)/trace-events
 trace.c: $(SRC_PATH)/trace-events
$(call quiet-command,sh $(SRC_PATH)/tracetool --$(TRACE_BACKEND) -c < 
$< > $@,"  GEN   $@")
 
-trace.o: trace.c trace.h
+trace.o: trace.c $(GENERATED_HEADERS)
 
-simpletrace.o: simpletrace.c trace.h
+simpletrace.o: simpletrace.c $(GENERATED_HEADERS)
 
 ##
 
-- 
1.7.1




Re: [Qemu-devel] Re: [PATCH 4/4] require #define NEED_GLOBAL_ENV for files that need the global register variable

2010-06-29 Thread Paul Brook
> On 06/28/2010 10:29 PM, Paul Brook wrote:
> >> diff --git a/exec-all.h b/exec-all.h
> >> index a775582..ebe88ad 100644
> >> --- a/exec-all.h
> >> +++ b/exec-all.h
> >> @@ -353,4 +353,8 @@ extern int singlestep;
> >> 
> >>   /* cpu-exec.c */
> >>   extern volatile sig_atomic_t exit_request;
> >> 
> >> +#ifdef NEED_GLOBAL_ENV
> >> +register CPUState *env asm(AREG0);
> >> +#endif
> > 
> > Wouldn't it be better to just put this in dyngen-exec.h ?
> > AFAICT there's a direct correlation between NEED_GLOBAL_ENV and #include
> > "exec.h".
> 
> True, see cover letter in 0/4.  I was told to make each file request
> explicitly the global variable though.  So I'd have to leave the #ifdef
> even if I moved it into dyngen-exec.h.

I don't understand what this is supposed to achieve.
The inclusion of exec.h is what defines whether this global variable is 
available.  Just as importantly, it also prevents code clobbering this value.  
Having one without the other makes no sense.

Making "env" available without including exec.h would be a different problem, 
but we never do that and would probably indicate we're doing something else 
wrong.

Paul



[Qemu-devel] Re: [PATCH 5/7] add qdev property type "cpu"

2010-06-29 Thread Markus Armbruster
Paolo Bonzini  writes:

>> Hmm.  Parse method doesn't accept output of the print method.  Not so
>> nice.  Is the "CPU #" decoration essential?
>
> I noticed the same in parse/print string:
>
> static int parse_string(DeviceState *dev, Property *prop, const char *str)
> {
> char **ptr = qdev_get_prop_ptr(dev, prop);
>
> if (*ptr)
> qemu_free(*ptr);
> *ptr = qemu_strdup(str);
> return 0;
> }
>
> static int print_string(DeviceState *dev, Property *prop, char *dest,
> size_t len)
> {
> char **ptr = qdev_get_prop_ptr(dev, prop);
> if (!*ptr)
> return snprintf(dest, len, "");
> return snprintf(dest, len, "\"%s\"", *ptr);
> }
>
> It looks like printing representation is chosen "for the user", not
> for parsing.

Yes, but does that mean we *should* add such decorations?

For me, the print method should be reasonably close to what the parse
method accepts.  A bit like Lisp's princ, whose ouput is close, but not
identical to what the reader accepts (for output acceptable to the
reader, use prin1).

As to "for the user":

dev-prop: cpu_env = CPU #0
dev-prop: cpu_env = 0

Both are equally intelligible, in my opinion: obvious if you know what
the property is about, gibberish if you don't.  The latter is slightly
easier to parse with simple tools like AWK.

By the way, print_string() should escape double-quote and funny
characters.



[Qemu-devel] Warning messages

2010-06-29 Thread malc
Hello,

After a fresh checkout and recompilation phase i did:

master$ i386-softmmu/qemu ~/x/img/linux-0.2.img

And (after a while) was greeted with:

pthread_create failed: Resource temporarily unavailable

Please, if you are printing error/warning message - make them useful,
the message should at the very least should print the caller of the
failing function..

-- 
mailto:av1...@comtv.ru



[Qemu-devel] Dumb question using qemu in full-screen mode

2010-06-29 Thread Wilhelm

Hi,

this is surely an often asked question, but i did not find an 
appropriate answer:


I have a ubuntu 10.04 host on which I would like to use qemu in 
full-screen mode at minumum 1024x768.


On the host I start the xorg-server directly, that gives me video modes 
up to 1280x1024.


Then I start qemu -full-screen, what switches the host-xserver back to 
720x400 and later on to 800x600, when the guest starts X.


In the guest I use an OpenBSD. If I use xrandr -q in the guest it gives 
me only 800x600 and 640x480.


Any hints?

--
Wilhelm





[Qemu-devel] [Bug 599617] Re: qemu fail to parse command "-net none"

2010-06-29 Thread Anthony Liguori
** Also affects: qemu-kvm (Ubuntu)
   Importance: Undecided
   Status: New

** Changed in: qemu
   Status: New => Invalid

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

Status in QEMU: Invalid
Status in “qemu-kvm” package in Ubuntu: New

Bug description:
Host OS:ia32e
Guest OS :32e and pae
kvm.git Commit:a63e16c655f9e68d49d6fae4275ffda16b1888b2
qemu-kvm Commit:97011c7fce92f8c0928c9e94e9896f0dca1bdeb9
Host Kernel Version:2.6.35-rc3


Bug detailed description:
--
when use command "qemu-system_x86 -smp 2 -m 1024 -hda /path/to/img -net none"
to boot up a guest, guest cannot boot up. and no error message displayed.





[Qemu-devel] [PATCH] trace: Support trace events with no arguments

2010-06-29 Thread Stefan Hajnoczi
Trace events with no arguments are not parsed correctly.  For example:
foo(void) ""

This patch fixes the trace-events parsing code and simple trace backend.

Signed-off-by: Stefan Hajnoczi 
---
This patch applies against the tracing branch:

http://repo.or.cz/w/qemu/stefanha.git/shortlog/refs/heads/tracing

 simpletrace.c |4 
 tracetool |   24 +++-
 2 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/simpletrace.c b/simpletrace.c
index 2d65114..5c327af 100644
--- a/simpletrace.c
+++ b/simpletrace.c
@@ -55,6 +55,10 @@ static void trace(TraceEventID event, unsigned long x1,
 }
 }
 
+void trace0(TraceEventID event) {
+trace(event, 0, 0, 0, 0, 0);
+}
+
 void trace1(TraceEventID event, unsigned long x1) {
 trace(event, x1, 0, 0, 0, 0);
 }
diff --git a/tracetool b/tracetool
index c77280d..9ce8e74 100755
--- a/tracetool
+++ b/tracetool
@@ -39,8 +39,11 @@ get_args()
 # Get the argument name list of a trace event
 get_argnames()
 {
-local first field name
+local nfields field name
+nfields=0
 for field in $(get_args "$1"); do
+nfields=$((nfields + 1))
+
 # Drop pointer star
 field=${field#\*}
 
@@ -50,7 +53,12 @@ get_argnames()
 
 echo -n "$name, "
 done
-echo -n "$name"
+
+# Last argument name
+if [ "$nfields" -gt 1 ]
+then
+echo -n "$name"
+fi
 }
 
 # Get the number of arguments to a trace event
@@ -125,6 +133,7 @@ typedef struct {
 bool state;
 } TraceEvent;
 
+void trace0(TraceEventID event);
 void trace1(TraceEventID event, unsigned long x1);
 void trace2(TraceEventID event, unsigned long x1, unsigned long x2);
 void trace3(TraceEventID event, unsigned long x1, unsigned long x2, unsigned 
long x3);
@@ -148,15 +157,20 @@ cast_args_to_ulong()
 
 linetoh_simple()
 {
-local name args argc ulong_args
+local name args argc trace_args
 name=$(get_name "$1")
 args=$(get_args "$1")
 argc=$(get_argc "$1")
-ulong_args=$(cast_args_to_ulong "$1")
+
+trace_args="$simple_event_num"
+if [ "$argc" -gt 0 ]
+then
+trace_args="$trace_args, $(cast_args_to_ulong "$1")"
+fi
 
 cat <

[Qemu-devel] Re: [PATCH 08/12] block: Catch attempt to attach multiple devices to a blockdev

2010-06-29 Thread Kevin Wolf
Am 25.06.2010 18:53, schrieb Markus Armbruster:
> For instance, -device scsi-disk,drive=foo -device scsi-disk,drive=foo
> happily creates two SCSI disks connected to the same block device.
> It's all downhill from there.
> 
> Device usb-storage deliberately attaches twice to the same blockdev,
> which fails with the fix in place.  Detach before the second attach
> there.
> 
> Also catch attempt to delete while a guest device model is attached.
> 
> Signed-off-by: Markus Armbruster 
> ---
>  block.c  |   22 ++
>  block.h  |3 +++
>  block_int.h  |2 ++
>  hw/fdc.c |   10 +-
>  hw/ide/qdev.c|2 +-
>  hw/pci-hotplug.c |5 -
>  hw/qdev-properties.c |   21 -
>  hw/qdev.h|3 ++-
>  hw/s390-virtio.c |2 +-
>  hw/scsi-bus.c|4 +++-
>  hw/usb-msd.c |   11 +++
>  11 files changed, 70 insertions(+), 15 deletions(-)
> 
> diff --git a/block.c b/block.c
> index e71a771..5e0ffa0 100644
> --- a/block.c
> +++ b/block.c
> @@ -659,6 +659,8 @@ void bdrv_close_all(void)
>  
>  void bdrv_delete(BlockDriverState *bs)
>  {
> +assert(!bs->peer);
> +
>  /* remove from list, if necessary */
>  if (bs->device_name[0] != '\0') {
>  QTAILQ_REMOVE(&bdrv_states, bs, list);
> @@ -672,6 +674,26 @@ void bdrv_delete(BlockDriverState *bs)
>  qemu_free(bs);
>  }
>  
> +int bdrv_attach(BlockDriverState *bs, DeviceState *qdev)
> +{
> +if (bs->peer) {
> +return -EBUSY;
> +}
> +bs->peer = qdev;
> +return 0;
> +}
> +
> +void bdrv_detach(BlockDriverState *bs, DeviceState *qdev)
> +{
> +assert(bs->peer == qdev);
> +bs->peer = NULL;
> +}
> +
> +DeviceState *bdrv_get_attached(BlockDriverState *bs)
> +{
> +return bs->peer;
> +}
> +
>  /*
>   * Run consistency checks on an image
>   *
> diff --git a/block.h b/block.h
> index 6a157f4..88ac06e 100644
> --- a/block.h
> +++ b/block.h
> @@ -71,6 +71,9 @@ int bdrv_file_open(BlockDriverState **pbs, const char 
> *filename, int flags);
>  int bdrv_open(BlockDriverState *bs, const char *filename, int flags,
>BlockDriver *drv);
>  void bdrv_close(BlockDriverState *bs);
> +int bdrv_attach(BlockDriverState *bs, DeviceState *qdev);
> +void bdrv_detach(BlockDriverState *bs, DeviceState *qdev);
> +DeviceState *bdrv_get_attached(BlockDriverState *bs);
>  int bdrv_check(BlockDriverState *bs);
>  int bdrv_read(BlockDriverState *bs, int64_t sector_num,
>uint8_t *buf, int nb_sectors);
> diff --git a/block_int.h b/block_int.h
> index e60aed4..a94b801 100644
> --- a/block_int.h
> +++ b/block_int.h
> @@ -148,6 +148,8 @@ struct BlockDriverState {
>  BlockDriver *drv; /* NULL means no media */
>  void *opaque;
>  
> +DeviceState *peer;
> +
>  char filename[1024];
>  char backing_file[1024]; /* if non zero, the image is a diff of
>  this file image */
> diff --git a/hw/fdc.c b/hw/fdc.c
> index 08712bc..1496cfa 100644
> --- a/hw/fdc.c
> +++ b/hw/fdc.c
> @@ -1860,10 +1860,10 @@ FDCtrl *fdctrl_init_isa(DriveInfo **fds)
>  
>  dev = isa_create("isa-fdc");
>  if (fds[0]) {
> -qdev_prop_set_drive(&dev->qdev, "driveA", fds[0]->bdrv);
> +qdev_prop_set_drive_nofail(&dev->qdev, "driveA", fds[0]->bdrv);
>  }
>  if (fds[1]) {
> -qdev_prop_set_drive(&dev->qdev, "driveB", fds[1]->bdrv);
> +qdev_prop_set_drive_nofail(&dev->qdev, "driveB", fds[1]->bdrv);
>  }
>  if (qdev_init(&dev->qdev) < 0)
>  return NULL;
> @@ -1882,10 +1882,10 @@ FDCtrl *fdctrl_init_sysbus(qemu_irq irq, int 
> dma_chann,
>  fdctrl = &sys->state;
>  fdctrl->dma_chann = dma_chann; /* FIXME */
>  if (fds[0]) {
> -qdev_prop_set_drive(dev, "driveA", fds[0]->bdrv);
> +qdev_prop_set_drive_nofail(dev, "driveA", fds[0]->bdrv);
>  }
>  if (fds[1]) {
> -qdev_prop_set_drive(dev, "driveB", fds[1]->bdrv);
> +qdev_prop_set_drive_nofail(dev, "driveB", fds[1]->bdrv);
>  }
>  qdev_init_nofail(dev);
>  sysbus_connect_irq(&sys->busdev, 0, irq);
> @@ -1903,7 +1903,7 @@ FDCtrl *sun4m_fdctrl_init(qemu_irq irq, 
> target_phys_addr_t io_base,
>  
>  dev = qdev_create(NULL, "SUNW,fdtwo");
>  if (fds[0]) {
> -qdev_prop_set_drive(dev, "drive", fds[0]->bdrv);
> +qdev_prop_set_drive_nofail(dev, "drive", fds[0]->bdrv);
>  }
>  qdev_init_nofail(dev);
>  sys = DO_UPCAST(FDCtrlSysBus, busdev.qdev, dev);
> diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
> index 3bb94c6..b4bc5ac 100644
> --- a/hw/ide/qdev.c
> +++ b/hw/ide/qdev.c
> @@ -83,7 +83,7 @@ IDEDevice *ide_create_drive(IDEBus *bus, int unit, 
> DriveInfo *drive)
>  
>  dev = qdev_create(&bus->qbus, "ide-drive");
>  qdev_prop_set_uint32(dev, "unit", unit);
> -qdev_prop_set_drive(dev, "drive", drive->bdrv);
> +qdev_prop_set_drive_nofail(dev, "drive", drive->bdrv);
>  qdev_

Re: [Qemu-devel] KVM Call agenda for June 29

2010-06-29 Thread Anthony Liguori

On 06/28/2010 02:03 PM, Juan Quintela wrote:

Please send in any agenda items you are interested in covering.

If we have a lack of agenda items I'll cancel the week's call.

After last week debacle, I will wait until 10 mins before call to cancel
it.
   


Thanks for posting earlier this week.   I don't have anything to discuss 
so I'm in favor of cancelling this week.


Regards,

Anthony Liguori


thanks, Juan.

   





Re: [Qemu-devel] KVM Call agenda for June 29

2010-06-29 Thread Chris Wright
* Anthony Liguori (anth...@codemonkey.ws) wrote:
> On 06/28/2010 02:03 PM, Juan Quintela wrote:
> >Please send in any agenda items you are interested in covering.
> >
> >If we have a lack of agenda items I'll cancel the week's call.
> >
> >After last week debacle, I will wait until 10 mins before call to cancel
> >it.
> 
> Thanks for posting earlier this week.   I don't have anything to
> discuss so I'm in favor of cancelling this week.

OK, let's cancel this week.

thanks,
-chris



Re: [Qemu-devel] Re: [PATCH 4/4] require #define NEED_GLOBAL_ENV for files that need the global register variable

2010-06-29 Thread Paolo Bonzini

On 06/29/2010 01:30 PM, Paul Brook wrote:

I don't understand what this is supposed to achieve.
The inclusion of exec.h is what defines whether this global variable is
available.  Just as importantly, it also prevents code clobbering this value.
Having one without the other makes no sense.

Making "env" available without including exec.h would be a different problem,
but we never do that and would probably indicate we're doing something else
wrong.


Paul, I agree but I was told to do something different.

Paolo



[Qemu-devel] Re: [PATCH 5/7] add qdev property type "cpu"

2010-06-29 Thread Paolo Bonzini

On 06/29/2010 01:42 PM, Markus Armbruster wrote:

As to "for the user":

 dev-prop: cpu_env = CPU #0
 dev-prop: cpu_env = 0

Both are equally intelligible, in my opinion: obvious if you know what
the property is about, gibberish if you don't.  The latter is slightly
easier to parse with simple tools like AWK.


I'll remove the decoration (though the overall problem remains).

Paolo



[Qemu-devel] Re: [PATCH 4/4] require #define NEED_GLOBAL_ENV for files that need the global register variable

2010-06-29 Thread Paolo Bonzini

On 06/29/2010 03:51 PM, Paolo Bonzini wrote:

On 06/29/2010 01:30 PM, Paul Brook wrote:

I don't understand what this is supposed to achieve. The inclusion
of exec.h is what defines whether this global variable is
available. Just as importantly, it also prevents code clobbering
this value. Having one without the other makes no sense.

Making "env" available without including exec.h would be a
different problem, but we never do that and would probably indicate
we're doing something else wrong.


Paul, I agree but I was told to do something different.


BTW, this may be useful for one thing: being able to include exec.h 
without bringing in the global variable.


I didn't really see a use right now for that, but cpu_get_tb_cpu_state 
could be something that may belong in target-*/exec.h more than in 
target-*/cpu.h (no, I'm not going to move it); yet it cannot be moved 
right now because it is called in exec.c.


That said, I think everybody at least agrees on the first 3 patches, is 
it possible to get at least (v2 of) thos applied?


Paolo



[Qemu-devel] [PATCH] AppleSMC device emulation

2010-06-29 Thread Alexander Graf
Intel Macs have a chip called the "AppleSMC" which they use to control
certain Apple specific parts of the hardware, like the keyboard background
light.

That chip is also used to store a key that Mac OS X uses to decrypt binaries.

This patch adds emulation for that chip, so we're getting one step further
to having Mac OS X run natively on Qemu.

Signed-off-by: Alexander Graf 
---
 Makefile.target |2 +-
 hw/applesmc.c   |  225 +++
 2 files changed, 226 insertions(+), 1 deletions(-)
 create mode 100644 hw/applesmc.c

diff --git a/Makefile.target b/Makefile.target
index f64702b..dcf0193 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -192,7 +192,7 @@ obj-y += e1000.o
 obj-i386-y += vga.o
 obj-i386-y += mc146818rtc.o i8259.o pc.o
 obj-i386-y += cirrus_vga.o apic.o ioapic.o piix_pci.o
-obj-i386-y += vmmouse.o vmport.o hpet.o
+obj-i386-y += vmmouse.o vmport.o hpet.o applesmc.o
 obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o
 obj-i386-y += debugcon.o multiboot.o
 obj-i386-y += pc_piix.o
diff --git a/hw/applesmc.c b/hw/applesmc.c
new file mode 100644
index 000..572b8f7
--- /dev/null
+++ b/hw/applesmc.c
@@ -0,0 +1,225 @@
+/*
+ *  Apple SMC controller
+ *
+ *  Copyright (c) 2007 Alexander Graf
+ *
+ *  Authors: Alexander Graf 
+ *   Susanne Graf 
+ *
+ * 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, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ *
+ * *
+ *
+ * In all Intel-based Apple hardware there is an SMC chip to control the
+ * backlight, fans and several other generic device parameters. It also
+ * contains the magic keys used to dongle Mac OS X to the device.
+ *
+ * This driver was mostly created by looking at the Linux AppleSMC driver
+ * implementation and does not support IRQ.
+ *
+ */
+
+#include "hw.h"
+#include "isa.h"
+#include "console.h"
+#include "qemu-timer.h"
+
+#define DEBUG_SMC
+
+#define APPLESMC_DEFAULT_IOBASE0x300
+/* data port used by Apple SMC */
+#define APPLESMC_DATA_PORT 0x0
+/* command/status port used by Apple SMC */
+#define APPLESMC_CMD_PORT  0x4
+#define APPLESMC_NR_PORTS  32
+#define APPLESMC_MAX_DATA_LENGTH   32
+
+#define APPLESMC_READ_CMD  0x10
+#define APPLESMC_WRITE_CMD 0x11
+#define APPLESMC_GET_KEY_BY_INDEX_CMD  0x12
+#define APPLESMC_GET_KEY_TYPE_CMD  0x13
+
+#ifdef DEBUG_SMC
+#define smc_debug(...) printf(__VA_ARGS__)
+#else
+#define smc_debug(...) do { } while(0)
+#endif
+
+static char default_osk[64] = "This is a dummy key. Enter the real key "
+  "using the -osk parameter";
+
+struct AppleSMCData {
+uint8_t len;
+const char *key;
+const char *data;
+QLIST_ENTRY(AppleSMCData) node;
+};
+
+struct AppleSMCStatus {
+ISADevice dev;
+uint32_t iobase;
+uint8_t cmd;
+uint8_t status;
+uint8_t key[4];
+uint8_t read_pos;
+uint8_t data_len;
+uint8_t data_pos;
+uint8_t data[255];
+uint8_t charactic[4];
+char *osk;
+QLIST_HEAD(, AppleSMCData) data_def;
+};
+
+static void applesmc_io_cmd_writeb(void *opaque, uint32_t addr, uint32_t val)
+{
+struct AppleSMCStatus *s = (struct AppleSMCStatus *)opaque;
+smc_debug("APPLESMC: CMD Write B: %#x = %#x\n", addr, val);
+switch(val) {
+case APPLESMC_READ_CMD:
+s->status = 0x0c;
+break;
+}
+s->cmd = val;
+s->read_pos = 0;
+s->data_pos = 0;
+}
+
+static void applesmc_fill_data(struct AppleSMCStatus *s)
+{
+struct AppleSMCData *d;
+
+QLIST_FOREACH(d, &s->data_def, node) {
+uint32_t key_data = *((uint32_t*)d->key);
+uint32_t key_current = *((uint32_t*)s->key);
+if(key_data == key_current) {
+smc_debug("APPLESMC: Key matched (%s Len=%d Data=%s)\n", d->key,
+  d->len, d->data);
+memcpy(s->data, d->data, d->len);
+return;
+}
+}
+}
+
+static void applesmc_io_data_writeb(void *opaque, uint32_t addr, uint32_t val)
+{
+struct AppleSMCStatus *s = (struct AppleSMCStatus *)opaque;
+smc_debug("APPLESMC: DATA Write B: %#x = %#x\n", addr, val);
+switch(s->cmd) {
+case APPLESMC_READ_CMD:
+if(s->read_pos < 4) {
+ 

[Qemu-devel] [PATCH] pcnet: Do not receive external frames in loopback mode

2010-06-29 Thread Jan Kiszka
While not explicitly stated in the spec, it was observed on real systems
that enabling loopback testing on the pcnet controller disables
reception of external frames. And some legacy software relies on it, so
provide this behavior.

Signed-off-by: Jan Kiszka 
---
 hw/pcnet.c |5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/pcnet.c b/hw/pcnet.c
index 5e63eb5..5c94dd2 100644
--- a/hw/pcnet.c
+++ b/hw/pcnet.c
@@ -1048,9 +1048,10 @@ ssize_t pcnet_receive(VLANClientState *nc, const uint8_t 
*buf, size_t size_)
 int crc_err = 0;
 int size = size_;
 
-if (CSR_DRX(s) || CSR_STOP(s) || CSR_SPND(s) || !size)
+if (CSR_DRX(s) || CSR_STOP(s) || CSR_SPND(s) || !size ||
+(CSR_LOOP(s) && !s->looptest)) {
 return -1;
-
+}
 #ifdef PCNET_DEBUG
 printf("pcnet_receive size=%d\n", size);
 #endif
-- 
1.7.1



[Qemu-devel] Re: [PATCH 08/12] block: Catch attempt to attach multiple devices to a blockdev

2010-06-29 Thread Markus Armbruster
Kevin Wolf  writes:

> Am 25.06.2010 18:53, schrieb Markus Armbruster:
>> For instance, -device scsi-disk,drive=foo -device scsi-disk,drive=foo
>> happily creates two SCSI disks connected to the same block device.
>> It's all downhill from there.
>> 
>> Device usb-storage deliberately attaches twice to the same blockdev,
>> which fails with the fix in place.  Detach before the second attach
>> there.
>> 
>> Also catch attempt to delete while a guest device model is attached.
[...]
>> diff --git a/hw/pci-hotplug.c b/hw/pci-hotplug.c
>> index d743192..b47e01e 100644
>> --- a/hw/pci-hotplug.c
>> +++ b/hw/pci-hotplug.c
>> @@ -214,7 +214,10 @@ static PCIDevice *qemu_pci_hot_add_storage(Monitor *mon,
>>  return NULL;
>>  }
>>  dev = pci_create(bus, devfn, "virtio-blk-pci");
>> -qdev_prop_set_drive(&dev->qdev, "drive", dinfo->bdrv);
>> +if (qdev_prop_set_drive(&dev->qdev, "drive", dinfo->bdrv) < 0) {
>> +dev = NULL;
>> +break;
>> +}
>
> I think in error cases we'll leak dev.

Correct.

>>  if (qdev_init(&dev->qdev) < 0)
>>  dev = NULL;
>>  break;
[...]
>> diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
>> index 2c58aca..9678328 100644
>> --- a/hw/scsi-bus.c
>> +++ b/hw/scsi-bus.c
>> @@ -91,7 +91,9 @@ SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, 
>> BlockDriverState *bdrv, int
>>  driver = bdrv_is_sg(bdrv) ? "scsi-generic" : "scsi-disk";
>>  dev = qdev_create(&bus->qbus, driver);
>>  qdev_prop_set_uint32(dev, "scsi-id", unit);
>> -qdev_prop_set_drive(dev, "drive", bdrv);
>> +if (qdev_prop_set_drive(dev, "drive", bdrv) < 0) {
>> +return NULL;
>> +}
>
> As we do here.

Yes.

>>  if (qdev_init(dev) < 0)
>>  return NULL;
>>  return DO_UPCAST(SCSIDevice, qdev, dev);
>> diff --git a/hw/usb-msd.c b/hw/usb-msd.c
>> index 19a14b4..008cc0f 100644
>> --- a/hw/usb-msd.c
>> +++ b/hw/usb-msd.c
>> @@ -532,12 +532,13 @@ static int usb_msd_initfn(USBDevice *dev)
>>  /*
>>   * Hack alert: this pretends to be a block device, but it's really
>>   * a SCSI bus that can serve only a single device, which it
>> - * creates automatically.  Two drive properties pointing to the
>> - * same drive is not good: free_drive() dies for the second one.
>> - * Zap the one we're not going to use.
>> + * creates automatically.  But first it needs to detach from its
>> + * blockdev, or else scsi_bus_legacy_add_drive() dies when it
>> + * attaches again.
>>   *
>>   * The hack is probably a bad idea.
>>   */
>> +bdrv_detach(bs, &s->dev.qdev);
>>  s->conf.bs = NULL;
>>  
>>  s->dev.speed = USB_SPEED_FULL;
>> @@ -609,7 +610,9 @@ static USBDevice *usb_msd_init(const char *filename)
>>  if (!dev) {
>>  return NULL;
>>  }
>> -qdev_prop_set_drive(&dev->qdev, "drive", dinfo->bdrv);
>> +if (qdev_prop_set_drive(&dev->qdev, "drive", dinfo->bdrv) < 0) {
>> +return NULL;
>> +}
>>  if (qdev_init(&dev->qdev) < 0)
>>  return NULL;
>
> And here.

Yes.

I double-checked the entire patch series, and could not find further
leaks.

Thanks a lot!



[Qemu-devel] [PATCH v2 08/12] block: Catch attempt to attach multiple devices to a blockdev

2010-06-29 Thread Markus Armbruster
For instance, -device scsi-disk,drive=foo -device scsi-disk,drive=foo
happily creates two SCSI disks connected to the same block device.
It's all downhill from there.

Device usb-storage deliberately attaches twice to the same blockdev,
which fails with the fix in place.  Detach before the second attach
there.

Also catch attempt to delete while a guest device model is attached.

Signed-off-by: Markus Armbruster 
---
v2: plug leaks pointed out by Kevin Wolf
fix qdev_prop_set_drive() to set the property only when attach
succeeded (free_drive() will die without this)

Also available at git://repo.or.cz/qemu/armbru.git
Tag block-fixes-v2

 block.c  |   22 ++
 block.h  |3 +++
 block_int.h  |2 ++
 hw/fdc.c |   10 +-
 hw/ide/qdev.c|2 +-
 hw/pci-hotplug.c |6 +-
 hw/qdev-properties.c |   22 +-
 hw/qdev.h|3 ++-
 hw/s390-virtio.c |2 +-
 hw/scsi-bus.c|5 -
 hw/usb-msd.c |   12 
 11 files changed, 74 insertions(+), 15 deletions(-)

diff --git a/block.c b/block.c
index e71a771..5e0ffa0 100644
--- a/block.c
+++ b/block.c
@@ -659,6 +659,8 @@ void bdrv_close_all(void)
 
 void bdrv_delete(BlockDriverState *bs)
 {
+assert(!bs->peer);
+
 /* remove from list, if necessary */
 if (bs->device_name[0] != '\0') {
 QTAILQ_REMOVE(&bdrv_states, bs, list);
@@ -672,6 +674,26 @@ void bdrv_delete(BlockDriverState *bs)
 qemu_free(bs);
 }
 
+int bdrv_attach(BlockDriverState *bs, DeviceState *qdev)
+{
+if (bs->peer) {
+return -EBUSY;
+}
+bs->peer = qdev;
+return 0;
+}
+
+void bdrv_detach(BlockDriverState *bs, DeviceState *qdev)
+{
+assert(bs->peer == qdev);
+bs->peer = NULL;
+}
+
+DeviceState *bdrv_get_attached(BlockDriverState *bs)
+{
+return bs->peer;
+}
+
 /*
  * Run consistency checks on an image
  *
diff --git a/block.h b/block.h
index 6a157f4..88ac06e 100644
--- a/block.h
+++ b/block.h
@@ -71,6 +71,9 @@ int bdrv_file_open(BlockDriverState **pbs, const char 
*filename, int flags);
 int bdrv_open(BlockDriverState *bs, const char *filename, int flags,
   BlockDriver *drv);
 void bdrv_close(BlockDriverState *bs);
+int bdrv_attach(BlockDriverState *bs, DeviceState *qdev);
+void bdrv_detach(BlockDriverState *bs, DeviceState *qdev);
+DeviceState *bdrv_get_attached(BlockDriverState *bs);
 int bdrv_check(BlockDriverState *bs);
 int bdrv_read(BlockDriverState *bs, int64_t sector_num,
   uint8_t *buf, int nb_sectors);
diff --git a/block_int.h b/block_int.h
index e60aed4..a94b801 100644
--- a/block_int.h
+++ b/block_int.h
@@ -148,6 +148,8 @@ struct BlockDriverState {
 BlockDriver *drv; /* NULL means no media */
 void *opaque;
 
+DeviceState *peer;
+
 char filename[1024];
 char backing_file[1024]; /* if non zero, the image is a diff of
 this file image */
diff --git a/hw/fdc.c b/hw/fdc.c
index 08712bc..1496cfa 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -1860,10 +1860,10 @@ FDCtrl *fdctrl_init_isa(DriveInfo **fds)
 
 dev = isa_create("isa-fdc");
 if (fds[0]) {
-qdev_prop_set_drive(&dev->qdev, "driveA", fds[0]->bdrv);
+qdev_prop_set_drive_nofail(&dev->qdev, "driveA", fds[0]->bdrv);
 }
 if (fds[1]) {
-qdev_prop_set_drive(&dev->qdev, "driveB", fds[1]->bdrv);
+qdev_prop_set_drive_nofail(&dev->qdev, "driveB", fds[1]->bdrv);
 }
 if (qdev_init(&dev->qdev) < 0)
 return NULL;
@@ -1882,10 +1882,10 @@ FDCtrl *fdctrl_init_sysbus(qemu_irq irq, int dma_chann,
 fdctrl = &sys->state;
 fdctrl->dma_chann = dma_chann; /* FIXME */
 if (fds[0]) {
-qdev_prop_set_drive(dev, "driveA", fds[0]->bdrv);
+qdev_prop_set_drive_nofail(dev, "driveA", fds[0]->bdrv);
 }
 if (fds[1]) {
-qdev_prop_set_drive(dev, "driveB", fds[1]->bdrv);
+qdev_prop_set_drive_nofail(dev, "driveB", fds[1]->bdrv);
 }
 qdev_init_nofail(dev);
 sysbus_connect_irq(&sys->busdev, 0, irq);
@@ -1903,7 +1903,7 @@ FDCtrl *sun4m_fdctrl_init(qemu_irq irq, 
target_phys_addr_t io_base,
 
 dev = qdev_create(NULL, "SUNW,fdtwo");
 if (fds[0]) {
-qdev_prop_set_drive(dev, "drive", fds[0]->bdrv);
+qdev_prop_set_drive_nofail(dev, "drive", fds[0]->bdrv);
 }
 qdev_init_nofail(dev);
 sys = DO_UPCAST(FDCtrlSysBus, busdev.qdev, dev);
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index a5fdab0..b34c473 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -83,7 +83,7 @@ IDEDevice *ide_create_drive(IDEBus *bus, int unit, DriveInfo 
*drive)
 
 dev = qdev_create(&bus->qbus, "ide-drive");
 qdev_prop_set_uint32(dev, "unit", unit);
-qdev_prop_set_drive(dev, "drive", drive->bdrv);
+qdev_prop_set_drive_nofail(dev, "drive", drive->bdrv);
 qdev_init_nofail(dev);
 return DO_UPCAST(IDEDevice, qdev, dev);
 }
diff --git a/hw/pci-hotplug.c b/hw/pci-h

[Qemu-devel] Re: [PATCH 4/4] require #define NEED_GLOBAL_ENV for files that need the global register variable

2010-06-29 Thread Paul Brook
> On 06/29/2010 03:51 PM, Paolo Bonzini wrote:
> > On 06/29/2010 01:30 PM, Paul Brook wrote:
> >> I don't understand what this is supposed to achieve. The inclusion
> >> of exec.h is what defines whether this global variable is
> >> available. Just as importantly, it also prevents code clobbering
> >> this value. Having one without the other makes no sense.
> >> 
> >> Making "env" available without including exec.h would be a
> >> different problem, but we never do that and would probably indicate
> >> we're doing something else wrong.
> > 
> > Paul, I agree but I was told to do something different.
> 
> BTW, this may be useful for one thing: being able to include exec.h
> without bringing in the global variable.

I'd argue that's not a desirable feature. I'd much rather have exec.h be the 
controlling factor than have all files include the same set of headers and 
have semantics determined by some combination of preprocessor macros.

I realise we already have this with NEED_CPU_H, however I don't think that's a 
precedent we want to encourage.

> I didn't really see a use right now for that, but cpu_get_tb_cpu_state
> could be something that may belong in target-*/exec.h more than in
> target-*/cpu.h (no, I'm not going to move it); yet it cannot be moved
> right now because it is called in exec.c.

How is putting it in exec.h better?

Paul



[Qemu-devel] Re: [PATCH v3 0/2] Add virtio-blk support to persistent-storage rules

2010-06-29 Thread Kay Sievers
On Fri, Jun 25, 2010 at 15:59, Ryan Harper  wrote:
> This patch series provides updates to udev to allow the creation symlinks for
> virtio-blk devices, specifically disk/by-id and disk/by-path.  This is most
> useful for virtio-blk devices that do not yet have any filesystem for which a
> UUID can be extracted (disk/by-uuid).  These patches (save the path_id fix)
> require an updated[1] qemu (on the host) and virtio-blk (in the guest)[2]  to
> generate the by-id path; however if the guest or host qemu isn't capable
> then no action is taken.

Thanks for solving it in this much simpler way. Both patches applied.

Kay



[Qemu-devel] Re: [PATCH 4/4] require #define NEED_GLOBAL_ENV for files that need the global register variable

2010-06-29 Thread Paolo Bonzini

On 06/29/2010 05:28 PM, Paul Brook wrote:

On 06/29/2010 03:51 PM, Paolo Bonzini wrote:

On 06/29/2010 01:30 PM, Paul Brook wrote:

I don't understand what this is supposed to achieve. The inclusion
of exec.h is what defines whether this global variable is
available. Just as importantly, it also prevents code clobbering
this value. Having one without the other makes no sense.

Making "env" available without including exec.h would be a
different problem, but we never do that and would probably indicate
we're doing something else wrong.


Paul, I agree but I was told to do something different.


BTW, this may be useful for one thing: being able to include exec.h
without bringing in the global variable.


I'd argue that's not a desirable feature. I'd much rather have exec.h be the
controlling factor than have all files include the same set of headers and
have semantics determined by some combination of preprocessor macros.

I realise we already have this with NEED_CPU_H, however I don't think that's a
precedent we want to encourage.


I didn't really see a use right now for that, but cpu_get_tb_cpu_state
could be something that may belong in target-*/exec.h more than in
target-*/cpu.h (no, I'm not going to move it); yet it cannot be moved
right now because it is called in exec.c.


How is putting it in exec.h better?


I see cpu.h as holding things related to the CPU as a hardware device, 
and exec.h as holding things related to emulation of the CPU.


In theory, a hypothetical KVM-only/no-TCG version of QEMU would not need 
to include neither exec-all.h nor target-*/exec.h.


Paolo



[Qemu-devel] Status update

2010-06-29 Thread Eduard - Gabriel Munteanu
Hi everybody,


Here's a status update (a public one this time) on my work:

---

Accomplishments:
- [DONE] Generic IOMMU API for devices
- [DONE] PIIX IDE uses the new API
- [DONE] Permissions handling

Bonus:
- [DONE] RTL8139 converted

Objectives:
- [WIP] Reporting IO page faults in the event log
- will require MSI support
- [WIP] Inject some errors for testing
- Relay IOMMU configuration data to SeaBIOS (last devfn etc.)
- Handle IOMMU updates correctly during AIO (see below)
- will require maintaining an internal cache

Absence:
- A couple of days. My last exam is on the 2nd of July, then school's
  over.

---

A few details...

I need to thank Paul Brook for helping me, during both e-mail and IRC
conversations, figure how to fit the IOMMU API with qdev.

Then there's the AIO issue. The DMA layer currently pretranslates
addresses, maps them into the host, then schedules AIO (which happens on
a separate thread). It's technically possible, though unlikely, that the
guest OS may change the IOMMU mappings before AIO completes, which can
result in incorrect behavior wrt real hardware.

I think we should implement some sort of IOTLB/caching in the IOMMU, to
prevent this issue from biting people. Then the IOMMU will signal to
the guest OS that page tables have been invalidated only when AIO
finishes. Caching is a good idea anyway. Here's how it goes:

IOMMU/hw emulation  | Guest | AIO
-
  start DMA transfer
start_transaction()
translate addresses
map addresses
start AIO >
  do I/O
  change mappings .
  invalidate command  .
process command   wait cmd completion .
wait AIO completion   .
  do I/O
  <
unmap addresses
end_transaction()
signal cmd completion
  use new mappings
  start other transfers

On the other hand, we could just leave it alone for now. Changing
mappings during DMA is stupid anyway: I don't think the guest can
recover the results of DMA safely, even though it might be used on
transfers in progress you simply don't care about anymore. Paul Brook
suggested we could update the cpu_physical_memory_map() mappings
somehow, but I think that's kinda difficult to accomplish.

I've included a draft of the generic IOMMU API, in case you think
something needs to be changed. Ideally, this should work for any bus and
IOMMU, and also handle bus bridges. No provisions have been made for
interrupt translation _yet_.

Will post patches soon. Feel free to suggest devices to convert, if you
consider any of them a priority. We currently have the PIIX IDE and
RTL8139 working, and Linux boots and works well with the IOMMU enabled.


Cheers,
Eduard

diff --git a/hw/iommu.c b/hw/iommu.c
new file mode 100644
index 000..410c88c
--- /dev/null
+++ b/hw/iommu.c
@@ -0,0 +1,83 @@
+/*
+ * IOMMU layer
+ *
+ * Copyright (c) 2010 Eduard - Gabriel Munteanu
+ *
+ * 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 
+
+#include "iommu.h"
+
+struct iommu *iommu_get(DeviceState *dev, DeviceState **real_dev)
+{
+BusState *bus;
+
+while (dev) {
+bus = dev->parent_bus;
+if (!bus)
+goto out;
+
+if (bus->iommu) {
+*real_dev = dev;
+return bus->iommu;
+}
+
+dev = bus->parent;
+}
+
+out:
+*real_dev = NULL;
+return NULL;
+}
+
+int __iommu_rw(struct iommu *iommu,
+   DeviceState *dev,
+   target_phys_addr_t addr,
+   uint8_t *buf,
+   int len,
+   int is_write)
+{
+int plen, err;
+  

[Qemu-devel] Re: Unusual physical address when using 64-bit BAR

2010-06-29 Thread Cam Macdonell
On Tue, Jun 29, 2010 at 12:50 AM, Avi Kivity  wrote:
> On 06/28/2010 11:38 PM, Cam Macdonell wrote:
>>
>>>
> Is this really the address the guest programmed, or is qemu
> misinterpreting
> it?
>
>
>>>
>>> Well, what's the answer?
>>>
>>
>> You're going to have to give me a hint on how to determine that.
>>
>> lspci in the guest shows the following
>>
>> Memory at c200 (64-bit, non-prefetchable) [size=1024M]
>>
>> does that demonstrate a guest generated address?
>>
>
> That's the result of a round trip: the guest programmed the address and then
> read it back.  It could have been screwed up in the first place, or perhaps
> qemu screwed it up.
>
> Add a printf() to the config space handlers in qemu (likely in your own
> code) on writes and reads, and show the relevant writes (and reads) for this
> BAR.
>
> That's the theory of deductive debugging; however browsing the code shows
> the guest is at fault:
>
>        for (i = 0; i < PCI_NUM_REGIONS; i++) {
>            int ofs;
>            if (i == PCI_ROM_SLOT)
>                ofs = PCI_ROM_ADDRESS;
>            else
>                ofs = PCI_BASE_ADDRESS_0 + i * 4;
>
>            u32 old = pci_config_readl(bdf, ofs);
>            u32 mask;
>            if (i == PCI_ROM_SLOT) {
>                mask = PCI_ROM_ADDRESS_MASK;
>                pci_config_writel(bdf, ofs, mask);
>            } else {
>                if (old & PCI_BASE_ADDRESS_SPACE_IO)
>                    mask = PCI_BASE_ADDRESS_IO_MASK;
>                else
>                    mask = PCI_BASE_ADDRESS_MEM_MASK;
>                pci_config_writel(bdf, ofs, ~0);
>            }
>            u32 val = pci_config_readl(bdf, ofs);
>            pci_config_writel(bdf, ofs, old);
>
>            if (val != 0) {
>                u32 size = (~(val & mask)) + 1;
>                if (val & PCI_BASE_ADDRESS_SPACE_IO)
>                    paddr = &pci_bios_io_addr;
>                else
>                    paddr = &pci_bios_mem_addr;
>                *paddr = ALIGN(*paddr, size);
>                pci_set_io_region_addr(bdf, i, *paddr);
>                *paddr += size;
>            }
>        }
>        break;
>    }
>
> Seabios completely ignore the 64-bitness of the BAR.  Looks like it also
> thinks the second half of the BAR is an I/O region instead of memory (hence
> the c200, that's part of the pci portio region.
>
> Do post those reads and writes, I think there's more than one thing wrong
> here.

Here it is, I added the debug statements to pci_read_config and
pci_default_write_config.

here are the reads and writes to offsets 0x18 and 0x1c where a 64-bit
BAR2 config would be configured

pci_read_config: (val) 0x4 <- 0x18 (addr)
pci_write_config: (val) 0x0 -> 0x18 (addr)
pci_read_config: (val) 0xc004 <- 0x18 (addr)
pci_write_config: (val) 0x0 -> 0x18 (addr)
pci_read_config: (val) 0x4 <- 0x18 (addr)
pci_write_config: (val) 0x0 -> 0x18 (addr)
pci_read_config: (val) 0x0 <- 0x1c (addr)
pci_write_config: (val) 0x0 -> 0x1c (addr)
pci_read_config: (val) 0x <- 0x1c (addr)
pci_write_config: (val) 0x0 -> 0x1c (addr)
pci_read_config: (val) 0x0 <- 0x1c (addr)
pci_write_config: (val) 0x0 -> 0x1c (addr)
pci_read_config: (val) 0x4 <- 0x18 (addr)
pci_write_config: (val) 0x0 -> 0x18 (addr)
pci_read_config: (val) 0xc004 <- 0x18 (addr)
pci_write_config: (val) 0x0 -> 0x18 (addr)
pci_read_config: (val) 0xc040 <- 0x1c (addr)
pci_write_config: (val) 0x0 -> 0x1c (addr)
pci_read_config: (val) 0x <- 0x1c (addr)
pci_write_config: (val) 0x0 -> 0x1c (addr)

the complete read/write profile is below along with debug statements
from the map functions for the BARs (prefixed with IVSHMEM)

pci_read_config: (val) 0x1af4 <- 0x0 (addr)
pci_read_config: (val) 0x0 <- 0xe (addr)
pci_read_config: (val) 0x1af4 <- 0x0 (addr)
pci_read_config: (val) 0x1110 <- 0x2 (addr)
pci_read_config: (val) 0x0 <- 0xe (addr)
pci_read_config: (val) 0x1af4 <- 0x0 (addr)
pci_read_config: (val) 0x0 <- 0xe (addr)
pci_read_config: (val) 0x500 <- 0xa (addr)
pci_read_config: (val) 0x1af4 <- 0x0 (addr)
pci_read_config: (val) 0x1110 <- 0x2 (addr)
pci_read_config: (val) 0x0 <- 0x10 (addr)
pci_write_config: (val) 0x0 -> 0x10 (addr)
pci_read_config: (val) 0xff00 <- 0x10 (addr)
pci_write_config: (val) 0x0 -> 0x10 (addr)
pci_read_config: (val) 0x0 <- 0x10 (addr)
pci_write_config: (val) 0x0 -> 0x10 (addr)
pci_read_config: (val) 0x0 <- 0x14 (addr)
pci_write_config: (val) 0x0 -> 0x14 (addr)
pci_read_config: (val) 0xf000 <- 0x14 (addr)
pci_write_config: (val) 0x0 -> 0x14 (addr)
pci_read_config: (val) 0x0 <- 0x14 (addr)
pci_write_config: (val) 0x0 -> 0x14 (addr)
pci_read_config: (val) 0x4 <- 0x18 (addr)
pci_write_config: (val) 0x0 -> 0x18 (addr)
pci_read_config: (val) 0xc004 <- 0x18 (addr)
pci_write_config: (val) 0x0 -> 0x18 (addr)
pci_read_config: (val) 0x4 <- 0x18 (addr)
pci_write_config: (val) 0x0 -> 0x18 (addr)
pci_read_config: (val) 0x0 <- 0x1c (addr)
pci_write_config: (val) 0x0 -> 0x1c (addr)
pci_read_config: (

Re: [Qemu-devel] virtio block device and sysfs

2010-06-29 Thread Marc Haber
Hi,

I had lost this topic for more than a few weeks...

On Mon, Mar 22, 2010 at 10:59:20AM -0400, john cooper wrote:
> Marc Haber wrote:
>> On Mon, Mar 22, 2010 at 02:26:11AM -0400, john cooper wrote:
>>> This is a tad ironic as that is how this saga begun.  Namely stuffing
>>> 20 bytes of serial number string into the virtio-blk PCI config space
>>> on qemu's side and pushing it over to the guest driver.  I exposed this
>>> to the guest app via a new ioctl cmd which itself was the original
>>> point of contention.  Someone took issue with introducing a new
>>> interface citing the existence of ATA and SCSI counterparts.  However
>>> dragging in the associated baggage in order to emulate those interfaces
>>> unintentionally bloated usage of the config space to the point of breakage.
>>> To address this I'd moved from using config space to an unused BAR which
>>> (understandably) didn't go over too well.  Somewhere along the line Rusty
>>> posted a minimal alternative version which directly used a virtio request
>>> to retrieve the data from qemu which is arguably the right way to do the
>>> job.
>>
>> *argh* That sounds like politics.
>
> Well, maybe.  But it is hard to argue with anyone calling the
> ATA_IDENTIFY interface ugly -- it is.

Indeed. It was designed to interact with Hardware, not with software
trying to look like hardware.

> Concerning qemu->virtio_blk communication, I don't think an
> argument to use PCI config space exists after one looks at
> the implementation of adding a new virtio request.

Indeed. Frankly, I don't care too much about how the information is
passed into the guest as long as the guest can read what the host
said. (mis)using ATA data was only a naive approach since I know that
there is an existing interface. If there is a better one, even better.

>>> That said we still had a dispute over what interface would be used to
>>> pass the S/N back to the guest: a new interface or reuse of an existing
>>> interface (eg: ATA IDENTIFY).  That's where things fizzled when we
>>> couldn't immediately resolve the issue.  So publishing the S/N in
>>> /sys would seem to side step this snag.
>>
>> Re-using an existing interface would probable make it easier for
>> non-Linux OSses to also take advantage of this, since their ATA driver
>> is already there.
>
> Exactly my singular motivation and honestly the only redeeming
> quality of propagating that crusty interface.

But otoh, I will only use Linux on the guest side, and it is
motivation for other developers to build a virtio driver for their
favorite OS.

>>> I could have swore I sent out a guest-driver-app-interface-less
>>> version of the patch using virtio to pass the S/N but didn't find it in
>>> the archives.  I did however locate it and can bring it forward as a
>>> reference for the above if interest exists.
>>
>> If it brings the issue forward and gives me hope to be able to do what
>> I want to do in a reasonable time frame, why not.
>
> Just time.  But I'll resurrect the patch so we don't have to go through
> all of this again.

Did that work?

Greetings
Marc

-- 
-
Marc Haber | "I don't trust Computers. They | Mailadresse im Header
Mannheim, Germany  |  lose things."Winona Ryder | Fon: *49 621 72739834
Nordisch by Nature |  How to make an American Quilt | Fax: *49 3221 2323190



[Qemu-devel] Re: [PATCH 4/4] require #define NEED_GLOBAL_ENV for files that need the global register variable

2010-06-29 Thread Blue Swirl
On Tue, Jun 29, 2010 at 7:44 AM, Paolo Bonzini  wrote:
> On 06/28/2010 10:29 PM, Paul Brook wrote:
>>>
>>> diff --git a/exec-all.h b/exec-all.h
>>> index a775582..ebe88ad 100644
>>> --- a/exec-all.h
>>> +++ b/exec-all.h
>>> @@ -353,4 +353,8 @@ extern int singlestep;
>>>  /* cpu-exec.c */
>>>  extern volatile sig_atomic_t exit_request;
>>>
>>> +#ifdef NEED_GLOBAL_ENV
>>> +register CPUState *env asm(AREG0);
>>> +#endif
>>
>> Wouldn't it be better to just put this in dyngen-exec.h ?
>> AFAICT there's a direct correlation between NEED_GLOBAL_ENV and #include
>> "exec.h".
>
> True, see cover letter in 0/4.  I was told to make each file request
> explicitly the global variable though.  So I'd have to leave the #ifdef even
> if I moved it into dyngen-exec.h.

Well, I only said I'd rename global 'env' to 'global_reg_env', not
something about each file requesting it. But NEED_GLOBAL_ENV wasn't so
bad idea in my opinion.



Re: [Qemu-devel] virtio block device and sysfs

2010-06-29 Thread john cooper
Marc Haber wrote:

 I could have swore I sent out a guest-driver-app-interface-less
 version of the patch using virtio to pass the S/N but didn't find it in
 the archives.  I did however locate it and can bring it forward as a
 reference for the above if interest exists.
>>> If it brings the issue forward and gives me hope to be able to do what
>>> I want to do in a reasonable time frame, why not.
>> Just time.  But I'll resurrect the patch so we don't have to go through
>> all of this again.
> 
> Did that work?

Yes, Ryan picked up the guest-user interface and exported
the data as /sys.  We just need the qemu side patch updated
and we should have this finally put to bed.

Thanks,

-john

-- 
john.coo...@third-harmonic.com



Re: [Qemu-devel] virtio block device and sysfs

2010-06-29 Thread Ryan Harper
* Marc Haber  [2010-06-29 13:16]:
> Hi,
> 
> I had lost this topic for more than a few weeks...
> 
> On Mon, Mar 22, 2010 at 10:59:20AM -0400, john cooper wrote:
> > Marc Haber wrote:
> >> On Mon, Mar 22, 2010 at 02:26:11AM -0400, john cooper wrote:
> >>> This is a tad ironic as that is how this saga begun.  Namely stuffing
> >>> 20 bytes of serial number string into the virtio-blk PCI config space
> >>> on qemu's side and pushing it over to the guest driver.  I exposed this
> >>> to the guest app via a new ioctl cmd which itself was the original
> >>> point of contention.  Someone took issue with introducing a new
> >>> interface citing the existence of ATA and SCSI counterparts.  However
> >>> dragging in the associated baggage in order to emulate those interfaces
> >>> unintentionally bloated usage of the config space to the point of 
> >>> breakage.
> >>> To address this I'd moved from using config space to an unused BAR which
> >>> (understandably) didn't go over too well.  Somewhere along the line Rusty
> >>> posted a minimal alternative version which directly used a virtio request
> >>> to retrieve the data from qemu which is arguably the right way to do the
> >>> job.
> >>
> >> *argh* That sounds like politics.
> >
> > Well, maybe.  But it is hard to argue with anyone calling the
> > ATA_IDENTIFY interface ugly -- it is.
> 
> Indeed. It was designed to interact with Hardware, not with software
> trying to look like hardware.
> 
> > Concerning qemu->virtio_blk communication, I don't think an
> > argument to use PCI config space exists after one looks at
> > the implementation of adding a new virtio request.
> 
> Indeed. Frankly, I don't care too much about how the information is
> passed into the guest as long as the guest can read what the host
> said. (mis)using ATA data was only a naive approach since I know that
> there is an existing interface. If there is a better one, even better.
> 
> >>> That said we still had a dispute over what interface would be used to
> >>> pass the S/N back to the guest: a new interface or reuse of an existing
> >>> interface (eg: ATA IDENTIFY).  That's where things fizzled when we
> >>> couldn't immediately resolve the issue.  So publishing the S/N in
> >>> /sys would seem to side step this snag.
> >>
> >> Re-using an existing interface would probable make it easier for
> >> non-Linux OSses to also take advantage of this, since their ATA driver
> >> is already there.
> >
> > Exactly my singular motivation and honestly the only redeeming
> > quality of propagating that crusty interface.
> 
> But otoh, I will only use Linux on the guest side, and it is
> motivation for other developers to build a virtio driver for their
> favorite OS.
> 
> >>> I could have swore I sent out a guest-driver-app-interface-less
> >>> version of the patch using virtio to pass the S/N but didn't find it in
> >>> the archives.  I did however locate it and can bring it forward as a
> >>> reference for the above if interest exists.
> >>
> >> If it brings the issue forward and gives me hope to be able to do what
> >> I want to do in a reasonable time frame, why not.
> >
> > Just time.  But I'll resurrect the patch so we don't have to go through
> > all of this again.
> 
> Did that work?

We've got a sysfs 'serial' attribute for virtio-blk devices upstream[1].
I've got udev support for using this attribute to create disk/by-id (and
a fix for by-path) symlinks[2].  All that remains is to
re-spin/post the qemu virtio-blk serial patches[3] and get that in and
we'll have the full stack working as I've already tested libvirt (which
has disk serial support).


1. 
https://lists.linux-foundation.org/pipermail/virtualization/2010-June/015324.html
2. http://www.spinics.net/lists/hotplug/msg03931.html
3. http://lists.gnu.org/archive/html/qemu-devel/2010-03/msg01870.html



> 
> Greetings
> Marc
> 
> -- 
> -
> Marc Haber | "I don't trust Computers. They | Mailadresse im Header
> Mannheim, Germany  |  lose things."Winona Ryder | Fon: *49 621 72739834
> Nordisch by Nature |  How to make an American Quilt | Fax: *49 3221 2323190

-- 
Ryan Harper
Software Engineer; Linux Technology Center
IBM Corp., Austin, Tx
ry...@us.ibm.com



Re: [Qemu-devel] virtio block device and sysfs

2010-06-29 Thread Marc Haber
Hi,

On Tue, Jun 29, 2010 at 01:33:33PM -0500, Ryan Harper wrote:
> We've got a sysfs 'serial' attribute for virtio-blk devices upstream[1].
> I've got udev support for using this attribute to create disk/by-id (and
> a fix for by-path) symlinks[2].  All that remains is to
> re-spin/post the qemu virtio-blk serial patches[3] and get that in and
> we'll have the full stack working as I've already tested libvirt (which
> has disk serial support).

That sounds really good. Do you want me to give some code a try?

Greetings
Marc

-- 
-
Marc Haber | "I don't trust Computers. They | Mailadresse im Header
Mannheim, Germany  |  lose things."Winona Ryder | Fon: *49 621 72739834
Nordisch by Nature |  How to make an American Quilt | Fax: *49 3221 2323190



Re: [Qemu-devel] [PATCH] AppleSMC device emulation

2010-06-29 Thread Blue Swirl
On Tue, Jun 29, 2010 at 2:35 PM, Alexander Graf  wrote:
> Intel Macs have a chip called the "AppleSMC" which they use to control
> certain Apple specific parts of the hardware, like the keyboard background
> light.
>
> That chip is also used to store a key that Mac OS X uses to decrypt binaries.
>
> This patch adds emulation for that chip, so we're getting one step further
> to having Mac OS X run natively on Qemu.
>
> Signed-off-by: Alexander Graf 
> ---
>  Makefile.target |    2 +-
>  hw/applesmc.c   |  225 
> +++
>  2 files changed, 226 insertions(+), 1 deletions(-)
>  create mode 100644 hw/applesmc.c
>
> diff --git a/Makefile.target b/Makefile.target
> index f64702b..dcf0193 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -192,7 +192,7 @@ obj-y += e1000.o
>  obj-i386-y += vga.o
>  obj-i386-y += mc146818rtc.o i8259.o pc.o
>  obj-i386-y += cirrus_vga.o apic.o ioapic.o piix_pci.o
> -obj-i386-y += vmmouse.o vmport.o hpet.o
> +obj-i386-y += vmmouse.o vmport.o hpet.o applesmc.o
>  obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o
>  obj-i386-y += debugcon.o multiboot.o
>  obj-i386-y += pc_piix.o
> diff --git a/hw/applesmc.c b/hw/applesmc.c
> new file mode 100644
> index 000..572b8f7
> --- /dev/null
> +++ b/hw/applesmc.c
> @@ -0,0 +1,225 @@
> +/*
> + *  Apple SMC controller
> + *
> + *  Copyright (c) 2007 Alexander Graf
> + *
> + *  Authors: Alexander Graf 
> + *           Susanne Graf 
> + *
> + * 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, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA

Please use the modern version with 'if not, see .'

> + *
> + * *
> + *
> + * In all Intel-based Apple hardware there is an SMC chip to control the
> + * backlight, fans and several other generic device parameters. It also
> + * contains the magic keys used to dongle Mac OS X to the device.
> + *
> + * This driver was mostly created by looking at the Linux AppleSMC driver
> + * implementation and does not support IRQ.
> + *
> + */
> +
> +#include "hw.h"
> +#include "isa.h"
> +#include "console.h"
> +#include "qemu-timer.h"
> +
> +#define DEBUG_SMC
> +
> +#define APPLESMC_DEFAULT_IOBASE        0x300
> +/* data port used by Apple SMC */
> +#define APPLESMC_DATA_PORT             0x0
> +/* command/status port used by Apple SMC */
> +#define APPLESMC_CMD_PORT              0x4
> +#define APPLESMC_NR_PORTS              32
> +#define APPLESMC_MAX_DATA_LENGTH       32
> +
> +#define APPLESMC_READ_CMD              0x10
> +#define APPLESMC_WRITE_CMD             0x11
> +#define APPLESMC_GET_KEY_BY_INDEX_CMD  0x12
> +#define APPLESMC_GET_KEY_TYPE_CMD      0x13
> +
> +#ifdef DEBUG_SMC
> +#define smc_debug(...) printf(__VA_ARGS__)

How about:
#define smc_debug(fmt, ...) printf("APPLESMC: " fmt, ## __VA_ARGS__)

> +#else
> +#define smc_debug(...) do { } while(0)
> +#endif
> +
> +static char default_osk[64] = "This is a dummy key. Enter the real key "
> +                              "using the -osk parameter";
> +
> +struct AppleSMCData {
> +    uint8_t len;
> +    const char *key;
> +    const char *data;
> +    QLIST_ENTRY(AppleSMCData) node;
> +};
> +
> +struct AppleSMCStatus {
> +    ISADevice dev;
> +    uint32_t iobase;
> +    uint8_t cmd;
> +    uint8_t status;
> +    uint8_t key[4];
> +    uint8_t read_pos;
> +    uint8_t data_len;
> +    uint8_t data_pos;
> +    uint8_t data[255];
> +    uint8_t charactic[4];
> +    char *osk;
> +    QLIST_HEAD(, AppleSMCData) data_def;
> +};
> +
> +static void applesmc_io_cmd_writeb(void *opaque, uint32_t addr, uint32_t val)
> +{
> +    struct AppleSMCStatus *s = (struct AppleSMCStatus *)opaque;

Useless cast in C, also in other places.

> +    smc_debug("APPLESMC: CMD Write B: %#x = %#x\n", addr, val);
> +    switch(val) {
> +        case APPLESMC_READ_CMD:
> +            s->status = 0x0c;
> +            break;
> +    }
> +    s->cmd = val;
> +    s->read_pos = 0;
> +    s->data_pos = 0;
> +}
> +
> +static void applesmc_fill_data(struct AppleSMCStatus *s)
> +{
> +    struct AppleSMCData *d;
> +
> +    QLIST_FOREACH(d, &s->data_def, node) {
> +        uint32_t key_data = *((uint32_t*)d->key);
> +        uint32_t key_current = *((uint32_t*)s->key);
> +        i

Re: [Qemu-devel] virtio block device and sysfs

2010-06-29 Thread Marc Haber
[re-send with correct sender]

Hi,

On Tue, Jun 29, 2010 at 01:33:33PM -0500, Ryan Harper wrote:
> We've got a sysfs 'serial' attribute for virtio-blk devices upstream[1].
> I've got udev support for using this attribute to create disk/by-id (and
> a fix for by-path) symlinks[2].  All that remains is to
> re-spin/post the qemu virtio-blk serial patches[3] and get that in and
> we'll have the full stack working as I've already tested libvirt (which
> has disk serial support).

That sounds really good. Do you want me to give some code a try?

Greetings
Marc

-- 
-
Marc Haber | "I don't trust Computers. They | Mailadresse im Header
Mannheim, Germany  |  lose things."Winona Ryder | Fon: *49 621 72739834
Nordisch by Nature |  How to make an American Quilt | Fax: *49 3221 2323190



[Qemu-devel] [PATCH] piix4: compile only once

2010-06-29 Thread Blue Swirl
Compile piix4 in hwlib. Two compilations less for the full build.

Signed-off-by: Blue Swirl 
---
 Makefile.objs|3 +++
 Makefile.target  |2 +-
 default-configs/mips-softmmu.mak |1 +
 default-configs/mips64-softmmu.mak   |1 +
 default-configs/mips64el-softmmu.mak |1 +
 default-configs/mipsel-softmmu.mak   |1 +
 6 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/Makefile.objs b/Makefile.objs
index 53fb68e..3acf59e 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -180,6 +180,9 @@ hw-obj-$(CONFIG_DEC_PCI) += dec_pci.o
 # PowerPC E500 boards
 hw-obj-$(CONFIG_PPCE500_PCI) += ppce500_pci.o

+# MIPS devices
+hw-obj-$(CONFIG_PIIX4) += piix4.o
+
 # PCI watchdog devices
 hw-obj-y += wdt_i6300esb.o

diff --git a/Makefile.target b/Makefile.target
index f64702b..3957cfe 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -220,7 +220,7 @@ obj-mips-y += mips_addr.o mips_timer.o mips_int.o
 obj-mips-y += vga.o i8259.o
 obj-mips-y += g364fb.o jazz_led.o
 obj-mips-y += gt64xxx.o mc146818rtc.o
-obj-mips-y += piix4.o cirrus_vga.o
+obj-mips-y += cirrus_vga.o

 obj-microblaze-y = petalogix_s3adsp1800_mmu.o

diff --git a/default-configs/mips-softmmu.mak b/default-configs/mips-softmmu.mak
index 29be52e..3d0af83 100644
--- a/default-configs/mips-softmmu.mak
+++ b/default-configs/mips-softmmu.mak
@@ -16,6 +16,7 @@ CONFIG_FDC=y
 CONFIG_ACPI=y
 CONFIG_APM=y
 CONFIG_DMA=y
+CONFIG_PIIX4=y
 CONFIG_IDE_CORE=y
 CONFIG_IDE_QDEV=y
 CONFIG_IDE_PCI=y
diff --git a/default-configs/mips64-softmmu.mak
b/default-configs/mips64-softmmu.mak
index 9bae8a7..0030de4 100644
--- a/default-configs/mips64-softmmu.mak
+++ b/default-configs/mips64-softmmu.mak
@@ -16,6 +16,7 @@ CONFIG_FDC=y
 CONFIG_ACPI=y
 CONFIG_APM=y
 CONFIG_DMA=y
+CONFIG_PIIX4=y
 CONFIG_IDE_CORE=y
 CONFIG_IDE_QDEV=y
 CONFIG_IDE_PCI=y
diff --git a/default-configs/mips64el-softmmu.mak
b/default-configs/mips64el-softmmu.mak
index b372c1d..7157e26 100644
--- a/default-configs/mips64el-softmmu.mak
+++ b/default-configs/mips64el-softmmu.mak
@@ -16,6 +16,7 @@ CONFIG_FDC=y
 CONFIG_ACPI=y
 CONFIG_APM=y
 CONFIG_DMA=y
+CONFIG_PIIX4=y
 CONFIG_IDE_CORE=y
 CONFIG_IDE_QDEV=y
 CONFIG_IDE_PCI=y
diff --git a/default-configs/mipsel-softmmu.mak
b/default-configs/mipsel-softmmu.mak
index 10ef483..238b73a 100644
--- a/default-configs/mipsel-softmmu.mak
+++ b/default-configs/mipsel-softmmu.mak
@@ -16,6 +16,7 @@ CONFIG_FDC=y
 CONFIG_ACPI=y
 CONFIG_APM=y
 CONFIG_DMA=y
+CONFIG_PIIX4=y
 CONFIG_IDE_CORE=y
 CONFIG_IDE_QDEV=y
 CONFIG_IDE_PCI=y
-- 
1.7.1



Re: [Qemu-devel] virtio block device and sysfs

2010-06-29 Thread Marc Haber
Hi,

On Tue, Jun 29, 2010 at 02:03:52PM -0400, john cooper wrote:
> Marc Haber wrote:
>  I could have swore I sent out a guest-driver-app-interface-less
>  version of the patch using virtio to pass the S/N but didn't find it in
>  the archives.  I did however locate it and can bring it forward as a
>  reference for the above if interest exists.
> >>> If it brings the issue forward and gives me hope to be able to do what
> >>> I want to do in a reasonable time frame, why not.
> >> Just time.  But I'll resurrect the patch so we don't have to go through
> >> all of this again.
> > 
> > Did that work?
> 
> Yes, Ryan picked up the guest-user interface and exported
> the data as /sys.  We just need the qemu side patch updated
> and we should have this finally put to bed.

Sounds really great!

Greetings
Marc

-- 
-
Marc Haber | "I don't trust Computers. They | Mailadresse im Header
Mannheim, Germany  |  lose things."Winona Ryder | Fon: *49 621 72739834
Nordisch by Nature |  How to make an American Quilt | Fax: *49 3221 2323190



Re: [Qemu-devel] [PATCH v2] lsi53c895a: fix Phase Mismatch Jump

2010-06-29 Thread Aurelien Jarno
On Mon, Jun 14, 2010 at 07:11:54PM +0200, Paolo Bonzini wrote:
> lsi_bad_phase has a bug in the choice of pmjad1/pmjad2.  This does
> not matter with Linux guests because it uses just one routine for
> both, but it breaks Windows 64-bit guests.  This is the text
> from the spec:
> 
>"[The PMJCTL] bit controls which decision mechanism is used
>when jumping on phase mismatch. When this bit is cleared the
>LSI53C895A will use Phase Mismatch Jump Address 1 (PMJAD1) when
>the WSR bit is cleared and Phase Mismatch Jump Address 2 (PMJAD2)
>when the WSR bit is set.  When this bit is set the LSI53C895A will
>use jump address one (PMJAD1) on data out (data out, command,
>message out) transfers and jump address two (PMJAD2) on data in
>(data in, status, message in) transfers."
> 
> Which means:
> 
> CCNTL0.PMJCTL
> 0  SCNTL2.WSR = 0 PMJAD1
> 0  SCNTL2.WSR = 1 PMJAD2
> 1out  PMJAD1
> 1in   PMJAD2
> 
> In qemu, what you get instead is:
> 
> CCNTL0.PMJCTL
> 0out  PMJAD1
> 0in   PMJAD2<
> 1out  PMJAD1
> 1in   PMJAD1<
> 
> Considering that qemu always has SCNTL2.WSR cleared, the two marked cases
> (corresponding to phase mismatch on input) are always jumping to the
> wrong PMJAD register.  The patch implements the correct semantics.
> 
> Signed-off-by: Paolo Bonzini 
> ---
> > Looks correct. But why not assigning s->pmjad[12] directly? Would
> > improve readability IMO.
> 
> No particular reason, hence fine by me.

Thanks, applied.

>  hw/lsi53c895a.c |6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
> index f5a91ba..9a37fed 100644
> --- a/hw/lsi53c895a.c
> +++ b/hw/lsi53c895a.c
> @@ -490,10 +490,10 @@ static void lsi_bad_phase(LSIState *s, int out, int 
> new_phase)
>  {
>  /* Trigger a phase mismatch.  */
>  if (s->ccntl0 & LSI_CCNTL0_ENPMJ) {
> -if ((s->ccntl0 & LSI_CCNTL0_PMJCTL) || out) {
> -s->dsp = s->pmjad1;
> +if ((s->ccntl0 & LSI_CCNTL0_PMJCTL)) {
> +s->dsp = out ? s->pmjad1 : s->pmjad2;
>  } else {
> -s->dsp = s->pmjad2;
> +s->dsp = (s->scntl2 & LSI_SCNTL2_WSR ? s->pmjad2 : s->pmjad1);
>  }
>  DPRINTF("Data phase mismatch jump to %08x\n", s->dsp);
>  } else {
> -- 
> 1.7.0.1
> 
> 
> 

-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net



Re: [Qemu-devel] [PATCH v4 6/7] MIPS: Initial support of fulong mini pc (machine construction)

2010-06-29 Thread Aurelien Jarno
On Mon, Jun 21, 2010 at 02:55:33PM +0800, chen huacai wrote:
> Loongson-specific instructions haven't implememted now. So, if want to
> boot a linux kernel, we should built a 32bit one and drop
> -march=loongson2e compiler flags. For simplification, please use the
> kernel patch (for 2.6.33) in the attachment and then use
> arch/mips/configs/fuloong2e_defconfig to compile a kernel. Besides,
> the disk image I used is debian-lenny built by you which is available
> at 
> http://people.debian.org/~aurel32/qemu/mipsel/debian_lenny_mipsel_small.qcow2.
> 

It indeed works, however I still think there is a problem. 64-bit
kernels should also boot when dropping -march=loongson2e. Also after
adding the Loongson-specific integer instructions, I am able to boot a
32-bit kernel built with -march=loongson2e, but not a 64-bit kernel.

-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net



[Qemu-devel] Re: [PATCH v6 0/7] MIPS: Initial support for fulong (Loongson-2E based) mini pc

2010-06-29 Thread Aurelien Jarno
On Tue, Jun 29, 2010 at 10:46:35AM +0800, chen huacai wrote:
> Changes from V5:
> Clean up old style save/restore code
> 

Thanks, all applied. However as said in answer to one of the previous
version of this series, 64-bit kernels fail to work. It would be nice to
follow up with patches to fix that. Also it would be nice to introduce
Loongson specific instructions (I can help on that, I already have a
quick and dirty patch to add them).

-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net



[Qemu-devel] [Bug 599958] [NEW] Timedrift problems with Win7 + qemu-kvm

2010-06-29 Thread Lucas Meneghel Rodrigues
Public bug reported:

We've been finding timedrift issues witth Win7 under qemu-kvm on our
daily testing

kvm.qemu-kvm-git.smp2.Win7.64.timedrift.with_load   FAIL1   Time 
drift too large after rest period: 38.63%
kvm.qemu-kvm-git.smp2.Win7.64.timedrift.with_reboot FAIL1   Time 
drift too large at iteration 1: 17.77 seconds
kvm.qemu-kvm-git.smp2.Win7.64.timedrift.with_migration  FAIL1   Time 
drift too large at iteration 2: 3.08 seconds

Steps to reproduce:

timedrift.with_load

1) Log into a guest.
2) Take a time reading from the guest and host.
3) Run load on the guest and host.
4) Take a second time reading.
5) Stop the load and rest for a while.
6) Take a third time reading.
7) If the drift immediately after load is higher than a user-
specified value (in %), fail.
If the drift after the rest period is higher than a user-specified value,
fail.

timedrift.with_migration

1) Log into a guest.
2) Take a time reading from the guest and host.
3) Migrate the guest.
4) Take a second time reading.
5) If the drift (in seconds) is higher than a user specified value, fail.

timedrift.with_reboot

1) Log into a guest.
2) Take a time reading from the guest and host.
3) Reboot the guest.
4) Take a second time reading.
5) If the drift (in seconds) is higher than a user specified value, fail.

This bug is to register those issues and keep an eye on them.

Attached, some logs from the autotest tests executed on the guest

** Affects: qemu
 Importance: Undecided
 Status: New

-- 
Timedrift problems with Win7 + qemu-kvm
https://bugs.launchpad.net/bugs/599958
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.

Status in QEMU: New

Bug description:
We've been finding timedrift issues witth Win7 under qemu-kvm on our daily 
testing

kvm.qemu-kvm-git.smp2.Win7.64.timedrift.with_load   FAIL1   Time 
drift too large after rest period: 38.63%
kvm.qemu-kvm-git.smp2.Win7.64.timedrift.with_reboot FAIL1   Time 
drift too large at iteration 1: 17.77 seconds
kvm.qemu-kvm-git.smp2.Win7.64.timedrift.with_migration  FAIL1   Time 
drift too large at iteration 2: 3.08 seconds

Steps to reproduce:

timedrift.with_load

1) Log into a guest.
2) Take a time reading from the guest and host.
3) Run load on the guest and host.
4) Take a second time reading.
5) Stop the load and rest for a while.
6) Take a third time reading.
7) If the drift immediately after load is higher than a user-
specified value (in %), fail.
If the drift after the rest period is higher than a user-specified value,
fail.

timedrift.with_migration

1) Log into a guest.
2) Take a time reading from the guest and host.
3) Migrate the guest.
4) Take a second time reading.
5) If the drift (in seconds) is higher than a user specified value, fail.

timedrift.with_reboot

1) Log into a guest.
2) Take a time reading from the guest and host.
3) Reboot the guest.
4) Take a second time reading.
5) If the drift (in seconds) is higher than a user specified value, fail.

This bug is to register those issues and keep an eye on them.

Attached, some logs from the autotest tests executed on the guest





[Qemu-devel] [Bug 599958] Re: Timedrift problems with Win7 + qemu-kvm

2010-06-29 Thread Lucas Meneghel Rodrigues

** Attachment added: "timedrift with migration log"
   
http://launchpadlibrarian.net/51133264/kvm.qemu-kvm-git.smp2.Win7.64.timedrift.with_migration.DEBUG

-- 
Timedrift problems with Win7 + qemu-kvm
https://bugs.launchpad.net/bugs/599958
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.

Status in QEMU: New

Bug description:
We've been finding timedrift issues witth Win7 under qemu-kvm on our daily 
testing

kvm.qemu-kvm-git.smp2.Win7.64.timedrift.with_load   FAIL1   Time 
drift too large after rest period: 38.63%
kvm.qemu-kvm-git.smp2.Win7.64.timedrift.with_reboot FAIL1   Time 
drift too large at iteration 1: 17.77 seconds
kvm.qemu-kvm-git.smp2.Win7.64.timedrift.with_migration  FAIL1   Time 
drift too large at iteration 2: 3.08 seconds

Steps to reproduce:

timedrift.with_load

1) Log into a guest.
2) Take a time reading from the guest and host.
3) Run load on the guest and host.
4) Take a second time reading.
5) Stop the load and rest for a while.
6) Take a third time reading.
7) If the drift immediately after load is higher than a user-
specified value (in %), fail.
If the drift after the rest period is higher than a user-specified value,
fail.

timedrift.with_migration

1) Log into a guest.
2) Take a time reading from the guest and host.
3) Migrate the guest.
4) Take a second time reading.
5) If the drift (in seconds) is higher than a user specified value, fail.

timedrift.with_reboot

1) Log into a guest.
2) Take a time reading from the guest and host.
3) Reboot the guest.
4) Take a second time reading.
5) If the drift (in seconds) is higher than a user specified value, fail.

This bug is to register those issues and keep an eye on them.

Attached, some logs from the autotest tests executed on the guest





[Qemu-devel] [Bug 599958] Re: Timedrift problems with Win7 + qemu-kvm

2010-06-29 Thread Lucas Meneghel Rodrigues

** Attachment added: "timedrift with load log"
   
http://launchpadlibrarian.net/51133239/kvm.qemu-kvm-git.smp2.Win7.64.timedrift.with_load.DEBUG

-- 
Timedrift problems with Win7 + qemu-kvm
https://bugs.launchpad.net/bugs/599958
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.

Status in QEMU: New

Bug description:
We've been finding timedrift issues witth Win7 under qemu-kvm on our daily 
testing

kvm.qemu-kvm-git.smp2.Win7.64.timedrift.with_load   FAIL1   Time 
drift too large after rest period: 38.63%
kvm.qemu-kvm-git.smp2.Win7.64.timedrift.with_reboot FAIL1   Time 
drift too large at iteration 1: 17.77 seconds
kvm.qemu-kvm-git.smp2.Win7.64.timedrift.with_migration  FAIL1   Time 
drift too large at iteration 2: 3.08 seconds

Steps to reproduce:

timedrift.with_load

1) Log into a guest.
2) Take a time reading from the guest and host.
3) Run load on the guest and host.
4) Take a second time reading.
5) Stop the load and rest for a while.
6) Take a third time reading.
7) If the drift immediately after load is higher than a user-
specified value (in %), fail.
If the drift after the rest period is higher than a user-specified value,
fail.

timedrift.with_migration

1) Log into a guest.
2) Take a time reading from the guest and host.
3) Migrate the guest.
4) Take a second time reading.
5) If the drift (in seconds) is higher than a user specified value, fail.

timedrift.with_reboot

1) Log into a guest.
2) Take a time reading from the guest and host.
3) Reboot the guest.
4) Take a second time reading.
5) If the drift (in seconds) is higher than a user specified value, fail.

This bug is to register those issues and keep an eye on them.

Attached, some logs from the autotest tests executed on the guest





[Qemu-devel] [Bug 599958] Re: Timedrift problems with Win7 + qemu-kvm

2010-06-29 Thread Lucas Meneghel Rodrigues

** Attachment added: "timedrift with reboot log"
   
http://launchpadlibrarian.net/51133275/kvm.qemu-kvm-git.smp2.Win7.64.timedrift.with_reboot.DEBUG

-- 
Timedrift problems with Win7 + qemu-kvm
https://bugs.launchpad.net/bugs/599958
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.

Status in QEMU: New

Bug description:
We've been finding timedrift issues witth Win7 under qemu-kvm on our daily 
testing

kvm.qemu-kvm-git.smp2.Win7.64.timedrift.with_load   FAIL1   Time 
drift too large after rest period: 38.63%
kvm.qemu-kvm-git.smp2.Win7.64.timedrift.with_reboot FAIL1   Time 
drift too large at iteration 1: 17.77 seconds
kvm.qemu-kvm-git.smp2.Win7.64.timedrift.with_migration  FAIL1   Time 
drift too large at iteration 2: 3.08 seconds

Steps to reproduce:

timedrift.with_load

1) Log into a guest.
2) Take a time reading from the guest and host.
3) Run load on the guest and host.
4) Take a second time reading.
5) Stop the load and rest for a while.
6) Take a third time reading.
7) If the drift immediately after load is higher than a user-
specified value (in %), fail.
If the drift after the rest period is higher than a user-specified value,
fail.

timedrift.with_migration

1) Log into a guest.
2) Take a time reading from the guest and host.
3) Migrate the guest.
4) Take a second time reading.
5) If the drift (in seconds) is higher than a user specified value, fail.

timedrift.with_reboot

1) Log into a guest.
2) Take a time reading from the guest and host.
3) Reboot the guest.
4) Take a second time reading.
5) If the drift (in seconds) is higher than a user specified value, fail.

This bug is to register those issues and keep an eye on them.

Attached, some logs from the autotest tests executed on the guest





[Qemu-devel] [Bug 599958] Re: Timedrift problems with Win7 + qemu-kvm

2010-06-29 Thread Anthony Liguori
Does adding -no-hpet make a difference?  I assume that Win7 is using
hpet by default and we currently don't do any time drift mitigation with
hpet.

-- 
Timedrift problems with Win7 + qemu-kvm
https://bugs.launchpad.net/bugs/599958
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.

Status in QEMU: New

Bug description:
We've been finding timedrift issues witth Win7 under qemu-kvm on our daily 
testing

kvm.qemu-kvm-git.smp2.Win7.64.timedrift.with_load   FAIL1   Time 
drift too large after rest period: 38.63%
kvm.qemu-kvm-git.smp2.Win7.64.timedrift.with_reboot FAIL1   Time 
drift too large at iteration 1: 17.77 seconds
kvm.qemu-kvm-git.smp2.Win7.64.timedrift.with_migration  FAIL1   Time 
drift too large at iteration 2: 3.08 seconds

Steps to reproduce:

timedrift.with_load

1) Log into a guest.
2) Take a time reading from the guest and host.
3) Run load on the guest and host.
4) Take a second time reading.
5) Stop the load and rest for a while.
6) Take a third time reading.
7) If the drift immediately after load is higher than a user-
specified value (in %), fail.
If the drift after the rest period is higher than a user-specified value,
fail.

timedrift.with_migration

1) Log into a guest.
2) Take a time reading from the guest and host.
3) Migrate the guest.
4) Take a second time reading.
5) If the drift (in seconds) is higher than a user specified value, fail.

timedrift.with_reboot

1) Log into a guest.
2) Take a time reading from the guest and host.
3) Reboot the guest.
4) Take a second time reading.
5) If the drift (in seconds) is higher than a user specified value, fail.

This bug is to register those issues and keep an eye on them.

Attached, some logs from the autotest tests executed on the guest





[Qemu-devel] [Bug 599958] Re: Timedrift problems with Win7 + qemu-kvm

2010-06-29 Thread Lucas Meneghel Rodrigues
I will try that on a separate test job and will let you know about the
results.

-- 
Timedrift problems with Win7 + qemu-kvm
https://bugs.launchpad.net/bugs/599958
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.

Status in QEMU: New

Bug description:
We've been finding timedrift issues witth Win7 under qemu-kvm on our daily 
testing

kvm.qemu-kvm-git.smp2.Win7.64.timedrift.with_load   FAIL1   Time 
drift too large after rest period: 38.63%
kvm.qemu-kvm-git.smp2.Win7.64.timedrift.with_reboot FAIL1   Time 
drift too large at iteration 1: 17.77 seconds
kvm.qemu-kvm-git.smp2.Win7.64.timedrift.with_migration  FAIL1   Time 
drift too large at iteration 2: 3.08 seconds

Steps to reproduce:

timedrift.with_load

1) Log into a guest.
2) Take a time reading from the guest and host.
3) Run load on the guest and host.
4) Take a second time reading.
5) Stop the load and rest for a while.
6) Take a third time reading.
7) If the drift immediately after load is higher than a user-
specified value (in %), fail.
If the drift after the rest period is higher than a user-specified value,
fail.

timedrift.with_migration

1) Log into a guest.
2) Take a time reading from the guest and host.
3) Migrate the guest.
4) Take a second time reading.
5) If the drift (in seconds) is higher than a user specified value, fail.

timedrift.with_reboot

1) Log into a guest.
2) Take a time reading from the guest and host.
3) Reboot the guest.
4) Take a second time reading.
5) If the drift (in seconds) is higher than a user specified value, fail.

This bug is to register those issues and keep an eye on them.

Attached, some logs from the autotest tests executed on the guest





Re: [Qemu-devel] Re: [PATCH 4/4] require #define NEED_GLOBAL_ENV for files that need the global register variable

2010-06-29 Thread Paul Brook
> >> BTW, this may be useful for one thing: being able to include exec.h
> >> without bringing in the global variable.
> > 
> > I'd argue that's not a desirable feature. I'd much rather have exec.h be
> > the controlling factor than have all files include the same set of
> > headers and have semantics determined by some combination of
> > preprocessor macros.
> > 
> > I realise we already have this with NEED_CPU_H, however I don't think
> > that's a precedent we want to encourage.
> > 
> >> I didn't really see a use right now for that, but cpu_get_tb_cpu_state
> >> could be something that may belong in target-*/exec.h more than in
> >> target-*/cpu.h (no, I'm not going to move it); yet it cannot be moved
> >> right now because it is called in exec.c.
> > 
> > How is putting it in exec.h better?
> 
> I see cpu.h as holding things related to the CPU as a hardware device,
> and exec.h as holding things related to emulation of the CPU.

If you really want NEED_GLOBAL_ENV then your implementation is nowhere near 
sufficient.

One of the main reasons we have exec.h is that you may not call GLOBAL_ENV 
code from regular code[1]. Doing so will result in qemu crashing.  The current 
system is simple to understand and audit: All prototypes for code that 
includes exec.h must go in exec.h.

Once you introduce NEED_GLOBAL_ENV you have both safe and unsafe code in 
exec.h.  All unsafe code should be protected by proper #idefs. I have low 
confidence in being able to get this right.

If you want to split out the TCG specific bits of cpu.h (including things like 
cpu_halted which are currently in exec.h but don't use global env) then I 
suggest creating a new header file for this.

Paul

[1] cpu_exec() is a special case, and jumps though hoops to make this work.



[Qemu-devel] RE: [PATCH] Another fix to VMware depth computation

2010-06-29 Thread Tian, Kevin
Looks no response here. Could anyone let me know anything else I need do to make
this patch ready for acceptance? I'd like to follow any guideline if there is.

I also CC Anthony here since he worked on a similar fix before. If any other 
stakeholder
I should CC, please let me know.

Thanks a lot,
Kevin

>From: Tian, Kevin
>Sent: Monday, June 28, 2010 3:59 PM
>
>(patch made upon 0.12.4 release; tested on 0.12 branch; build test on master)
>
> console.h   |   13 -
> hw/vmware_vga.c |   10 ++
> qemu-common.h   |1 -
> vl.c|8 
> 4 files changed, 2 insertions(+), 30 deletions(-)
>
>=
>Another fix to VMware depth computation
>
>VMware SVGA presents to the guest with the depth of the host surface it 
> renders
>to, and rejects to work if the two sides are mismatched. One problem is 
> that
>current VMware VGA may calculate a wrong host depth, and then memcpy from 
> virtual
>framebuffer to host surface may trigger segmentation fault. For example, 
> when
>launching Qemu in a VNC connection, VMware SVGA thinks depth as '32', 
> however the
>actual depth of VNC is '16'. The fault also happens when the host depth is 
> not
>32 bit.
>
>4b5db3749c5fdba93e1ac0e8748c9a9a1064319f tempts to fix a similar issue, by
>changing from hard-coded 24bit depth to instead query the surface allocator
>(e.g. sdl). However it doesn't really work, because the point where query
>is invoked is earlier than the point where sdl is initialized. At query 
> time,
>qemu uses a default surface allocator which, again, provides another 
> hard-coded
>depth value - 32bit. So it happens to make VMware SVGA working on some 
> hosts,
>but still fails in others.
>
>To solve this issue, this commit introduces a postcall interface to display
>surface, which is walked after surface allocators are actually initialized.
>At that point it's then safe to query host depth and present to the guest.
>
>Signed-off-by Kevin Tian 
>
>diff --git a/console.h b/console.h
>index dfc8ae4..05fbf17 100644
>--- a/console.h
>+++ b/console.h
>@@ -122,6 +122,12 @@ struct DisplayAllocator {
> void (*free_displaysurface)(DisplaySurface *surface);
> };
>
>+struct DisplayPostCallback {
>+void (*postcall) (void *);
>+void *parm;
>+struct DisplayPostCallback *next;
>+};
>+
> struct DisplayState {
> struct DisplaySurface *surface;
> void *opaque;
>@@ -129,6 +135,7 @@ struct DisplayState {
>
> struct DisplayAllocator* allocator;
> struct DisplayChangeListener* listeners;
>+struct DisplayPostCallback* postcalls;
>
> void (*mouse_set)(int x, int y, int on);
> void (*cursor_define)(int width, int height, int bpp, int hot_x, int 
> hot_y,
>@@ -185,6 +192,12 @@ static inline void 
>register_displaychangelistener(DisplayState *ds,
>DisplayChang
> ds->listeners = dcl;
> }
>
>+static inline void register_displaypostcallback(DisplayState *ds, 
>DisplayPostCallback *dpc)
>+{
>+dpc->next = ds->postcalls;
>+ds->postcalls = dpc;
>+}
>+
> static inline void dpy_update(DisplayState *s, int x, int y, int w, int h)
> {
> struct DisplayChangeListener *dcl = s->listeners;
>diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
>index 01bb85b..d73cca6 100644
>--- a/hw/vmware_vga.c
>+++ b/hw/vmware_vga.c
>@@ -927,8 +927,9 @@ static void vmsvga_update_display(void *opaque)
> }
> }
>
>-static void vmsvga_reset(struct vmsvga_state_s *s)
>+static void vmsvga_reset(void *parm)
> {
>+struct vmsvga_state_s *s = (struct vmsvga_state_s *)parm;
> s->index = 0;
> s->enable = 0;
> s->config = 0;
>@@ -1133,6 +1134,8 @@ static const VMStateDescription vmstate_vmware_vga = {
>
> static void vmsvga_init(struct vmsvga_state_s *s, int vga_ram_size)
> {
>+DisplayPostCallback *dpc;
>+
> s->scratch_size = SVGA_SCRATCH_SIZE;
> s->scratch = qemu_malloc(s->scratch_size * 4);
>
>@@ -1160,7 +1163,10 @@ static void vmsvga_init(struct vmsvga_state_s *s, int
>vga_ram_size)
>
> rom_add_vga(VGABIOS_FILENAME);
>
>-vmsvga_reset(s);
>+dpc = qemu_mallocz(sizeof(DisplayPostCallback));
>+dpc->postcall = vmsvga_reset;
>+dpc->parm = s;
>+register_displaypostcallback(s->vga.ds, dpc);
> }
>
> static void pci_vmsvga_map_ioport(PCIDevice *pci_dev, int region_num,
>diff --git a/qemu-common.h b/qemu-common.h
>index a23afbc..19f107a 100644
>--- a/qemu-common.h
>+++ b/qemu-common.h
>@@ -198,6 +198,7 @@ typedef struct DisplayState DisplayState;
> typedef struct DisplayChangeListener DisplayChangeListener;
> typedef struct DisplaySurface DisplaySurface;
> typedef struct DisplayAllocator DisplayAllocator;
>+typedef struct DisplayPostCallback DisplayPostCallback;
> typedef struct PixelFormat PixelFormat;
> typedef struct TextConsole TextConsole;
> typedef TextConsole QEMUConsole;
>diff --git a/vl.c b/vl.c
>index 39182ea..9a3e9fd 100644
>--- a/vl.c
>+++ b/vl.c
>@@ -4863,6 +4863,7 @@ int main(int argc, char **argv, cha

Re: [Qemu-devel] Re: Unusual physical address when using 64-bit BAR

2010-06-29 Thread Isaku Yamahata
On Tue, Jun 29, 2010 at 11:48:13AM -0600, Cam Macdonell wrote:
> On Tue, Jun 29, 2010 at 12:50 AM, Avi Kivity  wrote:
> > On 06/28/2010 11:38 PM, Cam Macdonell wrote:
> >>
> >>>
> > Is this really the address the guest programmed, or is qemu
> > misinterpreting
> > it?
> >
> >
> >>>
> >>> Well, what's the answer?
> >>>
> >>
> >> You're going to have to give me a hint on how to determine that.
> >>
> >> lspci in the guest shows the following
> >>
> >> Memory at c200 (64-bit, non-prefetchable) [size=1024M]
> >>
> >> does that demonstrate a guest generated address?
> >>
> >
> > That's the result of a round trip: the guest programmed the address and then
> > read it back. ?It could have been screwed up in the first place, or perhaps
> > qemu screwed it up.
> >
> > Add a printf() to the config space handlers in qemu (likely in your own
> > code) on writes and reads, and show the relevant writes (and reads) for this
> > BAR.
> >
> > That's the theory of deductive debugging; however browsing the code shows
> > the guest is at fault:
> >
> > ? ? ? ?for (i = 0; i < PCI_NUM_REGIONS; i++) {
> > ? ? ? ? ? ?int ofs;
> > ? ? ? ? ? ?if (i == PCI_ROM_SLOT)
> > ? ? ? ? ? ? ? ?ofs = PCI_ROM_ADDRESS;
> > ? ? ? ? ? ?else
> > ? ? ? ? ? ? ? ?ofs = PCI_BASE_ADDRESS_0 + i * 4;
> >
> > ? ? ? ? ? ?u32 old = pci_config_readl(bdf, ofs);
> > ? ? ? ? ? ?u32 mask;
> > ? ? ? ? ? ?if (i == PCI_ROM_SLOT) {
> > ? ? ? ? ? ? ? ?mask = PCI_ROM_ADDRESS_MASK;
> > ? ? ? ? ? ? ? ?pci_config_writel(bdf, ofs, mask);
> > ? ? ? ? ? ?} else {
> > ? ? ? ? ? ? ? ?if (old & PCI_BASE_ADDRESS_SPACE_IO)
> > ? ? ? ? ? ? ? ? ? ?mask = PCI_BASE_ADDRESS_IO_MASK;
> > ? ? ? ? ? ? ? ?else
> > ? ? ? ? ? ? ? ? ? ?mask = PCI_BASE_ADDRESS_MEM_MASK;
> > ? ? ? ? ? ? ? ?pci_config_writel(bdf, ofs, ~0);
> > ? ? ? ? ? ?}
> > ? ? ? ? ? ?u32 val = pci_config_readl(bdf, ofs);
> > ? ? ? ? ? ?pci_config_writel(bdf, ofs, old);
> >
> > ? ? ? ? ? ?if (val != 0) {
> > ? ? ? ? ? ? ? ?u32 size = (~(val & mask)) + 1;
> > ? ? ? ? ? ? ? ?if (val & PCI_BASE_ADDRESS_SPACE_IO)
> > ? ? ? ? ? ? ? ? ? ?paddr = &pci_bios_io_addr;
> > ? ? ? ? ? ? ? ?else
> > ? ? ? ? ? ? ? ? ? ?paddr = &pci_bios_mem_addr;
> > ? ? ? ? ? ? ? ?*paddr = ALIGN(*paddr, size);
> > ? ? ? ? ? ? ? ?pci_set_io_region_addr(bdf, i, *paddr);
> > ? ? ? ? ? ? ? ?*paddr += size;
> > ? ? ? ? ? ?}
> > ? ? ? ?}
> > ? ? ? ?break;
> > ? ?}
> >
> > Seabios completely ignore the 64-bitness of the BAR. ?Looks like it also
> > thinks the second half of the BAR is an I/O region instead of memory (hence
> > the c200, that's part of the pci portio region.

I've sent the patches to address it. But they haven't been merged yet.
seabios doesn't map BARs beyond 4GB.
If bar is mapped beyond 4GB, guest BIOS does it.


To see how seabios works, it would help to increase CONFIG_DEBUG_LEVEL
in config.h of seabios


> > Do post those reads and writes, I think there's more than one thing wrong
> > here.
> 
> Here it is, I added the debug statements to pci_read_config and
> pci_default_write_config.
> 
> here are the reads and writes to offsets 0x18 and 0x1c where a 64-bit
> BAR2 config would be configured

It seems that 0x1c is accessed as BAR3.
The write value in the log is always 0.

Some comment in the log.

> 
> pci_read_config: (val) 0x4 <- 0x18 (addr)
> pci_write_config: (val) 0x0 -> 0x18 (addr)
> pci_read_config: (val) 0xc004 <- 0x18 (addr)
> pci_write_config: (val) 0x0 -> 0x18 (addr)
> pci_read_config: (val) 0x4 <- 0x18 (addr)
> pci_write_config: (val) 0x0 -> 0x18 (addr)
> pci_read_config: (val) 0x0 <- 0x1c (addr)
> pci_write_config: (val) 0x0 -> 0x1c (addr)
> pci_read_config: (val) 0x <- 0x1c (addr)
> pci_write_config: (val) 0x0 -> 0x1c (addr)
> pci_read_config: (val) 0x0 <- 0x1c (addr)
> pci_write_config: (val) 0x0 -> 0x1c (addr)
> pci_read_config: (val) 0x4 <- 0x18 (addr)
> pci_write_config: (val) 0x0 -> 0x18 (addr)
> pci_read_config: (val) 0xc004 <- 0x18 (addr)
> pci_write_config: (val) 0x0 -> 0x18 (addr)
> pci_read_config: (val) 0xc040 <- 0x1c (addr)
> pci_write_config: (val) 0x0 -> 0x1c (addr)
> pci_read_config: (val) 0x <- 0x1c (addr)
> pci_write_config: (val) 0x0 -> 0x1c (addr)
> 
> the complete read/write profile is below along with debug statements
> from the map functions for the BARs (prefixed with IVSHMEM)
> 
> pci_read_config: (val) 0x1af4 <- 0x0 (addr)
> pci_read_config: (val) 0x0 <- 0xe (addr)
> pci_read_config: (val) 0x1af4 <- 0x0 (addr)
> pci_read_config: (val) 0x1110 <- 0x2 (addr)
> pci_read_config: (val) 0x0 <- 0xe (addr)
> pci_read_config: (val) 0x1af4 <- 0x0 (addr)
> pci_read_config: (val) 0x0 <- 0xe (addr)
> pci_read_config: (val) 0x500 <- 0xa (addr)
> pci_read_config: (val) 0x1af4 <- 0x0 (addr)
> pci_read_config: (val) 0x1110 <- 0x2 (addr)
> pci_read_config: (val) 0x0 <- 0x10 (addr)
> pci_write_config: (val) 0x0 -> 0x10 (addr)
> pci_read_config: (val) 0xff00 <- 0x10 (addr)
> pci_write_config: (val) 0x0 -> 0x10 (addr)
> pci_read_config: (val) 0x0 <- 0x10 (addr)
> pci

[Qemu-devel] [Bug 600063] [NEW] target-arm/translate.c

2010-06-29 Thread tadpole
Public bug reported:

when i use srsdb instruction in arm simulator(qumu), it will not be translated 
and run.
after looked over target-arm/translate.c file , i found the deal of srs 
instruction won't return, 
is that right?

** Affects: qemu
 Importance: Undecided
 Status: New

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

Status in QEMU: New

Bug description:
when i use srsdb instruction in arm simulator(qumu), it will not be translated 
and run.
after looked over target-arm/translate.c file , i found the deal of srs 
instruction won't return, 
is that right?





[Qemu-devel] [PATCH] virtio-9p: Add SM_NONE security model

2010-06-29 Thread Aneesh Kumar K.V
This is equivalent to SM_PASSTHROUGH security model.
The only exception is, failure of privilige operation like chown
are ignored. This makes a passthrough like security model usable
for people who runs kvm as non root

Signed-off-by: Aneesh Kumar K.V 
---
 hw/file-op-9p.h  |   15 +--
 hw/virtio-9p-local.c |   40 ++--
 hw/virtio-9p.c   |   13 ++---
 qemu-options.hx  |4 ++--
 vl.c |2 +-
 5 files changed, 56 insertions(+), 18 deletions(-)

diff --git a/hw/file-op-9p.h b/hw/file-op-9p.h
index 3d5f761..fcaa3ae 100644
--- a/hw/file-op-9p.h
+++ b/hw/file-op-9p.h
@@ -24,8 +24,19 @@
 
 typedef enum
 {
-SM_PASSTHROUGH = 1, /* uid/gid set on fileserver files */
-SM_MAPPED,  /* uid/gid part of xattr */
+/*
+ * Server will try to set uid/gid.
+ * On failure ignore the error.
+ */
+SM_NONE = 0,
+/*
+ * uid/gid set on fileserver files
+ */
+SM_PASSTHROUGH = 1,
+/*
+ * uid/gid part of xattr
+ */
+SM_MAPPED,
 } SecModel;
 
 typedef struct FsCred
diff --git a/hw/virtio-9p-local.c b/hw/virtio-9p-local.c
index c41193c..bc6793a 100644
--- a/hw/virtio-9p-local.c
+++ b/hw/virtio-9p-local.c
@@ -102,7 +102,13 @@ static int local_post_create_passthrough(FsContext 
*fs_ctx, const char *path,
 return -1;
 }
 if (chown(rpath(fs_ctx, path), credp->fc_uid, credp->fc_gid) < 0) {
-return -1;
+/*
+ * If we fail to change ownership and if we are
+ * using security model none. Ignore the error
+ */
+if (fs_ctx->fs_sm != SM_NONE) {
+return -1;
+}
 }
 return 0;
 }
@@ -122,7 +128,8 @@ static ssize_t local_readlink(FsContext *fs_ctx, const char 
*path,
 } while (tsize == -1 && errno == EINTR);
 close(fd);
 return tsize;
-} else if (fs_ctx->fs_sm == SM_PASSTHROUGH) {
+} else if ((fs_ctx->fs_sm == SM_PASSTHROUGH) ||
+   (fs_ctx->fs_sm == SM_NONE)) {
 tsize = readlink(rpath(fs_ctx, path), buf, bufsz);
 }
 return tsize;
@@ -189,7 +196,8 @@ static int local_chmod(FsContext *fs_ctx, const char *path, 
FsCred *credp)
 {
 if (fs_ctx->fs_sm == SM_MAPPED) {
 return local_set_xattr(rpath(fs_ctx, path), credp);
-} else if (fs_ctx->fs_sm == SM_PASSTHROUGH) {
+} else if ((fs_ctx->fs_sm == SM_PASSTHROUGH) ||
+   (fs_ctx->fs_sm == SM_NONE)) {
 return chmod(rpath(fs_ctx, path), credp->fc_mode);
 }
 return -1;
@@ -211,7 +219,8 @@ static int local_mknod(FsContext *fs_ctx, const char *path, 
FsCred *credp)
 serrno = errno;
 goto err_end;
 }
-} else if (fs_ctx->fs_sm == SM_PASSTHROUGH) {
+} else if ((fs_ctx->fs_sm == SM_PASSTHROUGH) ||
+   (fs_ctx->fs_sm == SM_NONE)) {
 err = mknod(rpath(fs_ctx, path), credp->fc_mode, credp->fc_rdev);
 if (err == -1) {
 return err;
@@ -247,7 +256,8 @@ static int local_mkdir(FsContext *fs_ctx, const char *path, 
FsCred *credp)
 serrno = errno;
 goto err_end;
 }
-} else if (fs_ctx->fs_sm == SM_PASSTHROUGH) {
+} else if ((fs_ctx->fs_sm == SM_PASSTHROUGH) ||
+   (fs_ctx->fs_sm == SM_NONE)) {
 err = mkdir(rpath(fs_ctx, path), credp->fc_mode);
 if (err == -1) {
 return err;
@@ -316,7 +326,8 @@ static int local_open2(FsContext *fs_ctx, const char *path, 
int flags,
 serrno = errno;
 goto err_end;
 }
-} else if (fs_ctx->fs_sm == SM_PASSTHROUGH) {
+} else if ((fs_ctx->fs_sm == SM_PASSTHROUGH) ||
+   (fs_ctx->fs_sm == SM_NONE)) {
 fd = open(rpath(fs_ctx, path), flags, credp->fc_mode);
 if (fd == -1) {
 return fd;
@@ -372,15 +383,23 @@ static int local_symlink(FsContext *fs_ctx, const char 
*oldpath,
 serrno = errno;
 goto err_end;
 }
-} else if (fs_ctx->fs_sm == SM_PASSTHROUGH) {
+} else if ((fs_ctx->fs_sm == SM_PASSTHROUGH) ||
+   (fs_ctx->fs_sm == SM_NONE)) {
 err = symlink(oldpath, rpath(fs_ctx, newpath));
 if (err) {
 return err;
 }
 err = lchown(rpath(fs_ctx, newpath), credp->fc_uid, credp->fc_gid);
 if (err == -1) {
-serrno = errno;
-goto err_end;
+/*
+ * If we fail to change ownership and if we are
+ * using security model none. Ignore the error
+ */
+if (fs_ctx->fs_sm != SM_NONE) {
+serrno = errno;
+goto err_end;
+} else
+err = 0;
 }
 }
 return err;
@@ -450,7 +469,8 @@ static int local_chown(FsContext *fs_ctx, const char *path, 
FsCred *credp)
 return lchown(rpath(fs_ctx, path), credp->fc_uid, credp->fc_gid);
 } else if (fs_ctx->fs_sm == SM_MAPPED) {
 return local_set_xattr(

[Qemu-devel] [PATCH] virtio-9p: Use lchown which won't follow symlink

2010-06-29 Thread Aneesh Kumar K.V
We should always use functions which don't follow
symlink on the server

Signed-off-by: Aneesh Kumar K.V 
---
 hw/virtio-9p-local.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/virtio-9p-local.c b/hw/virtio-9p-local.c
index bc6793a..30e1bda 100644
--- a/hw/virtio-9p-local.c
+++ b/hw/virtio-9p-local.c
@@ -101,7 +101,7 @@ static int local_post_create_passthrough(FsContext *fs_ctx, 
const char *path,
 if (chmod(rpath(fs_ctx, path), credp->fc_mode & 0) < 0) {
 return -1;
 }
-if (chown(rpath(fs_ctx, path), credp->fc_uid, credp->fc_gid) < 0) {
+if (lchown(rpath(fs_ctx, path), credp->fc_uid, credp->fc_gid) < 0) {
 /*
  * If we fail to change ownership and if we are
  * using security model none. Ignore the error
-- 
1.7.2.rc0




[Qemu-devel] [PATCH] virtio-9p: Fix the memset usage

2010-06-29 Thread Aneesh Kumar K.V
The arguments are wrong. Use qemu_mallocz directly

Signed-off-by: Aneesh Kumar K.V 
---
 hw/virtio-9p.c |3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
index 5c5a34b..fba6619 100644
--- a/hw/virtio-9p.c
+++ b/hw/virtio-9p.c
@@ -2782,8 +2782,7 @@ static void v9fs_wstat_post_chown(V9fsState *s, 
V9fsWstatState *vs, int err)
 if (vs->v9stat.name.size != 0) {
V9fsRenameState *vr;
 
-   vr = qemu_malloc(sizeof(V9fsRenameState));
-   memset(vr, sizeof(*vr), 0);
+   vr = qemu_mallocz(sizeof(V9fsRenameState));
vr->newdirfid= -1;
vr->pdu = vs->pdu;
vr->fidp = vs->fidp;
-- 
1.7.2.rc0




[Qemu-devel] merge spelling typo into stable-0.12

2010-06-29 Thread Vagrant Cascadian
please consider merging from master into stable-0.12:

 commit f093feb735ab57171b6fe16f54b7d3b989907d98
 Author: Vagrant Cascadian 
 Date:   Fri Feb 26 13:39:46 2010 -0800

audio/alsa: Spelling typo (paramters)

Trivial patch to fix the spelling of "parameters".

Signed-off-by: malc 

thanks!

live well,
  vagrant



Re: [Qemu-devel] [PATCH] virtio-9p: Fix the memset usage

2010-06-29 Thread Sripathi Kodi
On Wed, 30 Jun 2010 10:15:29 +0530
"Aneesh Kumar K.V"  wrote:

> The arguments are wrong. Use qemu_mallocz directly
> 
> Signed-off-by: Aneesh Kumar K.V 

Reviewed-by: Sripathi Kodi 

> ---
>  hw/virtio-9p.c |3 +--
>  1 files changed, 1 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
> index 5c5a34b..fba6619 100644
> --- a/hw/virtio-9p.c
> +++ b/hw/virtio-9p.c
> @@ -2782,8 +2782,7 @@ static void v9fs_wstat_post_chown(V9fsState *s, 
> V9fsWstatState *vs, int err)
>  if (vs->v9stat.name.size != 0) {
>   V9fsRenameState *vr;
> 
> - vr = qemu_malloc(sizeof(V9fsRenameState));
> - memset(vr, sizeof(*vr), 0);
> + vr = qemu_mallocz(sizeof(V9fsRenameState));
>   vr->newdirfid= -1;
>   vr->pdu = vs->pdu;
>   vr->fidp = vs->fidp;
> -- 
> 1.7.2.rc0
> 
> 



Re: [Qemu-devel] [PATCH v6 2/6] Initial support of vt82686b south bridge used by fulong mini pc

2010-06-29 Thread Isaku Yamahata
VT686PMState related code seems copy from acpi_piix4.c.
pmsts, pmen, pmcntrl, ...
Can it be consolidated?


On Tue, Jun 29, 2010 at 10:49:29AM +0800, Huacai Chen wrote:
> Signed-off-by: Huacai Chen 
> ---
>  Makefile.target |2 +-
>  hw/pci_ids.h|8 +
>  hw/vt82c686.c   |  597 
> +++
>  hw/vt82c686.h   |   11 +
>  4 files changed, 617 insertions(+), 1 deletions(-)
>  create mode 100644 hw/vt82c686.c
>  create mode 100644 hw/vt82c686.h
> 
> diff --git a/Makefile.target b/Makefile.target
> index eb852d4..caabacd 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -221,7 +221,7 @@ obj-mips-y += vga.o i8259.o
>  obj-mips-y += g364fb.o jazz_led.o
>  obj-mips-y += gt64xxx.o mc146818rtc.o
>  obj-mips-y += piix4.o cirrus_vga.o
> -obj-mips-$(CONFIG_FULONG) += bonito.o
> +obj-mips-$(CONFIG_FULONG) += bonito.o vt82c686.o
>  
>  obj-microblaze-y = petalogix_s3adsp1800_mmu.o
>  
> diff --git a/hw/pci_ids.h b/hw/pci_ids.h
> index fe7a121..39e9f1d 100644
> --- a/hw/pci_ids.h
> +++ b/hw/pci_ids.h
> @@ -78,6 +78,14 @@
>  
>  #define PCI_VENDOR_ID_XILINX 0x10ee
>  
> +#define PCI_VENDOR_ID_VIA0x1106
> +#define PCI_DEVICE_ID_VIA_ISA_BRIDGE 0x0686
> +#define PCI_DEVICE_ID_VIA_IDE0x0571
> +#define PCI_DEVICE_ID_VIA_UHCI   0x3038
> +#define PCI_DEVICE_ID_VIA_ACPI   0x3057
> +#define PCI_DEVICE_ID_VIA_AC97   0x3058
> +#define PCI_DEVICE_ID_VIA_MC97   0x3068
> +
>  #define PCI_VENDOR_ID_MARVELL0x11ab
>  
>  #define PCI_VENDOR_ID_ENSONIQ0x1274
> diff --git a/hw/vt82c686.c b/hw/vt82c686.c
> new file mode 100644
> index 000..a0c5747
> --- /dev/null
> +++ b/hw/vt82c686.c
> @@ -0,0 +1,597 @@
> +/*
> + * VT82C686B south bridge support
> + *
> + * Copyright (c) 2008 yajin (ya...@vm-kernel.org)
> + * Copyright (c) 2009 chenming (chenm...@rdc.faw.com.cn)
> + * Copyright (c) 2010 Huacai Chen (zltjiang...@gmail.com)
> + * This code is licensed under the GNU GPL v2.
> + */
> +
> +#include "hw.h"
> +#include "pc.h"
> +#include "vt82c686.h"
> +#include "i2c.h"
> +#include "smbus.h"
> +#include "pci.h"
> +#include "isa.h"
> +#include "sysbus.h"
> +#include "mips.h"
> +#include "apm.h"
> +#include "acpi.h"
> +#include "pm_smbus.h"
> +#include "sysemu.h"
> +#include "qemu-timer.h"
> +
> +typedef uint32_t pci_addr_t;
> +#include "pci_host.h"
> +//#define DEBUG_VT82C686B
> +
> +#ifdef DEBUG_VT82C686B
> +#define DPRINTF(fmt, ...) fprintf(stderr, "%s: " fmt, __FUNCTION__, 
> ##__VA_ARGS__)
> +#else
> +#define DPRINTF(fmt, ...)
> +#endif
> +
> +typedef struct SuperIOConfig
> +{
> +uint8_t config[0xff];
> +uint8_t index;
> +uint8_t data;
> +} SuperIOConfig;
> +
> +typedef struct VT82C686BState {
> +PCIDevice dev;
> +SuperIOConfig superio_conf;
> +} VT82C686BState;
> +
> +static void superio_ioport_writeb(void *opaque, uint32_t addr, uint32_t data)
> +{
> +int can_write;
> +SuperIOConfig *superio_conf = opaque;
> +
> +DPRINTF("superio_ioport_writeb  address 0x%x  val 0x%x  \n", addr, data);
> +if (addr == 0x3f0) {
> +superio_conf->index = data & 0xff;
> +} else {
> +/* 0x3f1 */
> +switch (superio_conf->index) {
> +case 0x00 ... 0xdf:
> +case 0xe4:
> +case 0xe5:
> +case 0xe9 ... 0xed:
> +case 0xf3:
> +case 0xf5:
> +case 0xf7:
> +case 0xf9 ... 0xfb:
> +case 0xfd ... 0xff:
> +can_write = 0;
> +break;
> +default:
> +can_write = 1;
> +
> +if (can_write) {
> +switch (superio_conf->index) {
> +case 0xe7:
> +if ((data & 0xff) != 0xfe) {
> +DPRINTF("chage uart 1 base. unsupported yet \n");
> +}
> +break;
> +case 0xe8:
> +if ((data & 0xff) != 0xbe) {
> +DPRINTF("chage uart 2 base. unsupported yet \n");
> +}
> +break;
> +
> +default:
> +superio_conf->config[superio_conf->index] = data & 0xff;
> +}
> +}
> +}
> +superio_conf->config[superio_conf->index] = data & 0xff;
> +}
> +}
> +
> +static uint32_t superio_ioport_readb(void *opaque, uint32_t addr)
> +{
> +SuperIOConfig *superio_conf = opaque;
> +
> +DPRINTF("superio_ioport_readb  address 0x%x   \n", addr);
> +return (superio_conf->config[superio_conf->index]);
> +}
> +
> +static void vt82c686b_reset(void * opaque)
> +{
> +PCIDevice *d = opaque;
> +uint8_t *pci_conf = d->config;
> +VT82C686BState *vt82c = DO_UPCAST(VT82C686BState, dev, d);
> +
> +pci_set_long(pci_conf + PCI_CAPABILITY_LIST, 0x00c0);
> +pci_set_word(pci_conf + PCI_COMMAND, PCI_COMMAND_IO | PCI_COMMAND_MEMORY 
> |
> + PCI_COMMAND_MASTER | PCI_COM

[Qemu-devel] Re: [PATCH] device-assignment: Rework "name" of assigned pci device

2010-06-29 Thread Markus Armbruster
Summary: upstream qemu commit b560a9ab broke -pcidevice and pci_add host
in two ways:

* Use without options id and name is broken when option host contains
  ':'.  That's because id defaults to host.  I recommend to fix it
  incompatibly: don't default id to host.  The alternative is to get
  upstream qemu to accept ':' in qdev IDs again.

* Funny characters in option name no longer work.  Same as funny
  characters in id options all over the place.  Intentional change.  I
  recommend to do nothing.

Details inline.

Hidetoshi Seto  writes:

> Thanks Markus,
>
> (2010/06/29 14:28), Markus Armbruster wrote:
>> Hidetoshi Seto  writes:
>> 
>>> "Hao, Xudong"  writes:
> When assign one PCI device, qemu fail to parse the command line:
> qemu-system_x86 -smp 2 -m 1024 -hda /path/to/img -pcidevice host=00:19.0
> Error:
> qemu-system-x86_64: Parameter 'id' expects an identifier
> Identifiers consist of letters, digits, '-', '.', '_', starting with a 
> letter.
> pcidevice argument parse error; please check the help text for usage
> Could not add assigned device host=00:19.0
>
> https://bugs.launchpad.net/qemu/+bug/597932
>
> This issue caused by qemu-kvm commit 
> b560a9ab9be06afcbb78b3791ab836dad208a239.
>>>
>>> This patch is a response to the above report.
>>>
>>> Thanks,
>>> H.Seto
>>>
>>> =
>>>
>>> Because use of some characters in "id" is restricted recently, assigned
>>> device start to fail having implicit "id" that uses address string of
>>> host device, like "00:19.0" which includes restricted character ':'.
>>>
>>> It seems that this implicit "id" is intended to be run as "name" that
>>> could be passed with option "-pcidevice ... ,name=..." to specify a
>>> string to be used in log outputs.  In other words it seems that
>>> dev->dev.qdev.id of assigned device had been used to have such
>>> "name", that is user-defined string or address string of "host".
>> 
>> As far as I can tell, option "name" is just a leftover from pre-qdev
>> days, kept for compatibility.
>
> Yea, I see.
> I don't know well about the history of such pre-qdev days...

It's often useful to examine history to figure out what a piece of code
attempts to accomplish.  git-blame and git-log are your friends :)

>>> The problem is that "name" for specific use is not equal to "id" for
>>> universal use.  So it is better to remove this tricky mix up here.
>>>
>>> This patch introduces new function assigned_dev_name() that returns
>>> proper name string for the device.
>>> Now property "name" is explicitly defined in struct AssignedDevice.
>>>
>>> When if the device have neither "name" nor "id", address string like
>>> ":00:19.0" will be created and passed instead.  Once created, new
>>> field r_name holds the string to be reused and to be released later.
[...]
>>> @@ -1520,6 +1545,7 @@ static PCIDeviceInfo assign_info = {
>>>  DEFINE_PROP("host", AssignedDevice, host, qdev_prop_hostaddr, 
>>> PCIHostDevice),
>>>  DEFINE_PROP_UINT32("iommu", AssignedDevice, use_iommu, 1),
>>>  DEFINE_PROP_STRING("configfd", AssignedDevice, configfd_name),
>>> +DEFINE_PROP_STRING("name", AssignedDevice, u_name),
>>>  DEFINE_PROP_END_OF_LIST(),
>>>  },
>>>  };
>>> @@ -1545,24 +1571,25 @@ device_init(assign_register_devices)
>>>  QemuOpts *add_assigned_device(const char *arg)
>>>  {
>>>  QemuOpts *opts = NULL;
>>> -char host[64], id[64], dma[8];
>>> +char host[64], buf[64], dma[8];
>>>  int r;
>>>  
>>> +/* "host" must be with -pcidevice */
>>>  r = get_param_value(host, sizeof(host), "host", arg);
>>>  if (!r)
>>>   goto bad;
>>> -r = get_param_value(id, sizeof(id), "id", arg);
>>> -if (!r)
>>> -r = get_param_value(id, sizeof(id), "name", arg);
>>> -if (!r)
>>> -r = get_param_value(id, sizeof(id), "host", arg);
>>>  
>>> -opts = qemu_opts_create(&qemu_device_opts, id, 0);
>>> +opts = qemu_opts_create(&qemu_device_opts, NULL, 0);
>> 
>> I think you break option id here.  Its value must become the qdev ID,
>> visible in info qtree and usable as argument to device_del.  And if
>> option id is missing, option name must become the qdev ID, for backwards
>> compatibility.
>
> Ah, I missed to check hot-add path - I had wonder why id could be here
> since I could not find documents that mentions use of id with -pcidevice.
>
> So, I should use id here if specified. That's right,
>
> But in case if id is missing and name is specified, I think there is no
> reason that the characters in name should be restricted in same manner
> with that in id.
>
> In case that there is neither id nor name but host (in fact it is original
> case) now ID "BB:DD.F" (like 00:19.0) will be rejected (because it starts
> with digit, plus it uses restricted ':').
>
> In other words, your change already breaks backwards compatibility because it
> prevents "-pcidevice host=BB:DD.F" from creating ID "BB:DD.F" that might be
>