Re: [Qemu-devel] [PATCH v7 00/11] target-arm: Parts of the AArch64 EL2/3 exception model

2014-09-29 Thread Jan Kiszka
On 2014-09-29 12:41, Peter Maydell wrote:
 On 29 September 2014 11:31, Jan Kiszka jan.kis...@siemens.com wrote:
 Sorry for hijacking the thread, but it seems related: These bits address
 AArch64, but what is the status of AArch32 /wrt hyp mode emulation?
 After playing with the fast model, I would be glad to find such
 support in QEMU rather sooner than later.
 
 I don't think anybody is currently working on that,
 though it is an obvious gap to fill in at some point.
 
 I would caution against testing hypervisors on QEMU
 rather than against hardware or the Fast Model,
 except for anything other than basic smoke tests,
 though -- there are a lot of complications and corner
 cases in the architecture here and I expect we will
 spend some time flushing out bugs as OSes expose
 them.

No difference with nested virt for KVM: I fixed several corner cases
along the way of getting Jailhouse running on x86. This time we already
have a working baseline, so shaking out bugs might be even easier. What
matters more mid- and long-term is the round-trip time, in fact.

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] [PATCH v2] slirp: udp: fix NULL pointer dereference because of uninitialized socket

2014-09-18 Thread Jan Kiszka
On 2014-09-18 08:35, Petr Matousek wrote:
 When guest sends udp packet with source port and source addr 0,
 uninitialized socket is picked up when looking for matching and already
 created udp sockets, and later passed to sosendto() where NULL pointer
 dereference is hit during so-slirp-vnetwork_mask.s_addr access.
 
 Fix this by checking that the socket is not just a socket stub.
 
 This is CVE-2014-3640.
 
 Signed-off-by: Petr Matousek pmato...@redhat.com
 Reported-by: Xavier Mehrenberger xavier.mehrenber...@airbus.com
 Reported-by: Stephane Duverger stephane.duver...@eads.net
 ---
 v1 - v2
   * change the check so that it's consistent with the rest of the code
 
  slirp/udp.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/slirp/udp.c b/slirp/udp.c
 index 8cc6cb6..f77e00f 100644
 --- a/slirp/udp.c
 +++ b/slirp/udp.c
 @@ -152,7 +152,7 @@ udp_input(register struct mbuf *m, int iphlen)
* Locate pcb for datagram.
*/
   so = slirp-udp_last_so;
 - if (so-so_lport != uh-uh_sport ||
 + if (so == slirp-udb || so-so_lport != uh-uh_sport ||
   so-so_laddr.s_addr != ip-ip_src.s_addr) {
   struct socket *tmp;
  
 

Reviewed-by: Jan Kiszka jan.kis...@siemens.com

Thanks,
Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] [PATCH] kvm: ioapic: conditionally delay irq delivery during eoi broadcast

2014-09-11 Thread Jan Kiszka
On 2014-09-11 07:06, Zhang Haoyu wrote:
 Currently, we call ioapic_service() immediately when we find the irq is still
 active during eoi broadcast. But for real hardware, there's some dealy between
 the EOI writing and irq delivery (system bus latency?). So we need to emulate
 this behavior. Otherwise, for a guest who haven't register a proper irq 
 handler
 , it would stay in the interrupt routine as this irq would be re-injected
 immediately after guest enables interrupt. This would lead guest can't move
 forward and may miss the possibility to get proper irq handler registered (one
 example is windows guest resuming from hibernation).
 
 As there's no way to differ the unhandled irq from new raised ones, this patch
 solve this problems by scheduling a delayed work when the count of irq 
 injected
 during eoi broadcast exceeds a threshold value. After this patch, the guest 
 can
 move a little forward when there's no suitable irq handler in case it may
 register one very soon and for guest who has a bad irq detection routine ( 
 such
 as note_interrupt() in linux ), this bad irq would be recognized soon as in 
 the
 past.
 
 Cc: Michael S. Tsirkin m...@redhat.com
 Signed-off-by: Jason Wang jasow...@redhat.com
 Signed-off-by: Zhang Haoyu zhan...@sangfor.com
 ---
  include/trace/events/kvm.h | 20 ++
  virt/kvm/ioapic.c  | 51 
 --
  virt/kvm/ioapic.h  |  6 ++
  3 files changed, 75 insertions(+), 2 deletions(-)
 
 diff --git a/include/trace/events/kvm.h b/include/trace/events/kvm.h
 index 908925a..b05f688 100644
 --- a/include/trace/events/kvm.h
 +++ b/include/trace/events/kvm.h
 @@ -95,6 +95,26 @@ TRACE_EVENT(kvm_ioapic_set_irq,
 __entry-coalesced ?  (coalesced) : )
  );
  
 +TRACE_EVENT(kvm_ioapic_delayed_eoi_inj,
 + TP_PROTO(__u64 e),
 + TP_ARGS(e),
 +
 + TP_STRUCT__entry(
 + __field(__u64,  e   )
 + ),
 +
 + TP_fast_assign(
 + __entry-e  = e;
 + ),
 +
 + TP_printk(dst %x vec=%u (%s|%s|%s%s),
 +   (u8)(__entry-e  56), (u8)__entry-e,
 +   __print_symbolic((__entry-e  8  0x7), kvm_deliver_mode),
 +   (__entry-e  (111)) ? logical : physical,
 +   (__entry-e  (115)) ? level : edge,
 +   (__entry-e  (116)) ? |masked : )
 +);
 +
  TRACE_EVENT(kvm_msi_set_irq,
   TP_PROTO(__u64 address, __u64 data),
   TP_ARGS(address, data),
 diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
 index e8ce34c..a36c4c4 100644
 --- a/virt/kvm/ioapic.c
 +++ b/virt/kvm/ioapic.c
 @@ -405,6 +405,24 @@ void kvm_ioapic_clear_all(struct kvm_ioapic *ioapic, int 
 irq_source_id)
   spin_unlock(ioapic-lock);
  }
  
 +static void kvm_ioapic_eoi_inject_work(struct work_struct *work)
 +{
 + int i;
 + struct kvm_ioapic *ioapic = container_of(work, struct kvm_ioapic,
 +  eoi_inject.work);
 + spin_lock(ioapic-lock);
 + for (i = 0; i  IOAPIC_NUM_PINS; i++) {
 + union kvm_ioapic_redirect_entry *ent = ioapic-redirtbl[i];
 +
 + if (ent-fields.trig_mode != IOAPIC_LEVEL_TRIG)
 + continue;
 +
 + if (ioapic-irr  (1  i)  !ent-fields.remote_irr)
 + ioapic_service(ioapic, i, false);
 + }
 + spin_unlock(ioapic-lock);
 +}
 +
  static void __kvm_ioapic_update_eoi(struct kvm_vcpu *vcpu,
   struct kvm_ioapic *ioapic, int vector, int trigger_mode)
  {
 @@ -435,8 +453,33 @@ static void __kvm_ioapic_update_eoi(struct kvm_vcpu 
 *vcpu,
  
   ASSERT(ent-fields.trig_mode == IOAPIC_LEVEL_TRIG);
   ent-fields.remote_irr = 0;
 - if (ioapic-irr  (1  i))
 - ioapic_service(ioapic, i, false);
 + if (!ent-fields.mask  (ioapic-irr  (1  i))) {

The mask check is new - why now? You don't check it in the work handler
as well.

 + ++ioapic-irq_eoi[i];
 + if (ioapic-irq_eoi[i] == 
 IOAPIC_SUCCESSIVE_IRQ_MAX_COUNT) {
 + /*
 +  * Real hardware does not deliver the irq so
 +  * immediately during eoi broadcast, so we need
 +  * to emulate this behavior. Otherwise, for
 +  * guests who has not registered handler of a
 +  * level irq, this irq would be injected
 +  * immediately after guest enables interrupt
 +  * (which happens usually at the end of the
 +  * common interrupt routine). This would lead
 +  * guest can't move forward and may miss the
 +  * possibility to get proper irq handler
 +  * registered. So we 

Re: [Qemu-devel] [PATCH] virtio-pci: enable bus master for old guests

2014-09-08 Thread Jan Kiszka
On 2014-09-08 18:05, Michael S. Tsirkin wrote:
 commit cc943c36faa192cd4b32af8fe5edb31894017d35
 pci: Use bus master address space for delivering MSI/MSI-X messages
 breaks virtio-net for rhel6.[56] x86 guests because they don't
 enable bus mastering for virtio PCI devices
 
 Old guests forgot to enable bus mastering, enable it
 automatically on DRIVER_OK.
 
 Note: we should either back out the original patch from
 stable or apply this one on top.
 
 Cc: qemu-sta...@nongnu.org
 Reported-by: Greg Kurz gk...@linux.vnet.ibm.com
 Signed-off-by: Jan Kiszka jan.kis...@siemens.com

I didn't signed off as I didn't write this patch ;). But you can add my
reviewed-by if you like.

Jan

 Signed-off-by: Michael S. Tsirkin m...@redhat.com
 ---
  hw/virtio/virtio-pci.c | 2 ++
  1 file changed, 2 insertions(+)
 
 diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
 index ddb5da1..af937d2 100644
 --- a/hw/virtio/virtio-pci.c
 +++ b/hw/virtio/virtio-pci.c
 @@ -320,6 +320,8 @@ static void virtio_ioport_write(void *opaque, uint32_t 
 addr, uint32_t val)
  if ((val  VIRTIO_CONFIG_S_DRIVER_OK) 
  !(proxy-pci_dev.config[PCI_COMMAND]  PCI_COMMAND_MASTER)) {
  proxy-flags |= VIRTIO_PCI_FLAG_BUS_MASTER_BUG;
 +
 memory_region_set_enabled(proxy-pci_dev.bus_master_enable_region,
 +  true);
  }
  break;
  case VIRTIO_MSI_CONFIG_VECTOR:
 

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] [PATCH] slirp: Honour vlan/stack in hostfwd_remove commands

2014-09-02 Thread Jan Kiszka
On 2014-09-02 12:33, Peter Maydell wrote:
 On 8 July 2014 12:40, Peter Maydell peter.mayd...@linaro.org wrote:
 On 26 June 2014 13:35, Peter Maydell peter.mayd...@linaro.org wrote:
 On 16 June 2014 16:47, Peter Maydell peter.mayd...@linaro.org wrote:
 The hostfwd_add and hostfwd_remove monitor commands allow the user
 to optionally specify a vlan/stack tuple. hostfwd_add honours this,
 but hostfwd_remove does not (it looks up the tuple but then ignores
 the SlirpState it has looked up and always uses the first stack
 in the list anyway). Correct this to honour what the user requested.

 Signed-off-by: Peter Maydell peter.mayd...@linaro.org
 ---
  net/slirp.c | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)

 diff --git a/net/slirp.c b/net/slirp.c
 index 647039e..c171119 100644
 --- a/net/slirp.c
 +++ b/net/slirp.c
 @@ -345,8 +345,7 @@ void net_slirp_hostfwd_remove(Monitor *mon, const 
 QDict *qdict)

  host_port = atoi(p);

 -err = slirp_remove_hostfwd(QTAILQ_FIRST(slirp_stacks)-slirp, is_udp,
 -   host_addr, host_port);
 +err = slirp_remove_hostfwd(s-slirp, is_udp, host_addr, host_port);

  monitor_printf(mon, host forwarding rule for %s %s\n, src_str,
 err ? not found : removed);
 --
 1.9.2

 Ping! (and cc trivial).

 Ping^2.
 
 Ping^3 :-(

Sorry, you will probably need someone else for these topics. I've
difficulties scheduling these into my todo list. :(

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] [PATCH v3 0/5] intel-iommu: introduce Intel IOMMU (VT-d) emulation to q35 chipset

2014-08-18 Thread Jan Kiszka
On 2014-08-18 18:34, Knut Omang wrote:
 On Sat, 2014-08-16 at 10:47 +0200, Jan Kiszka wrote:
 On 2014-08-16 10:45, Jan Kiszka wrote:
 On 2014-08-16 09:54, Knut Omang wrote:
 On Fri, 2014-08-15 at 19:37 +0800, Le Tan wrote:
 Hi Knut,

 2014-08-15 19:15 GMT+08:00 Knut Omang knut.om...@oracle.com:
 On Fri, 2014-08-15 at 06:42 +0200, Knut Omang wrote:
 On Thu, 2014-08-14 at 14:10 +0200, Jan Kiszka wrote:
 On 2014-08-14 13:15, Michael S. Tsirkin wrote:
 On Mon, Aug 11, 2014 at 03:04:57PM +0800, Le Tan wrote:
 Hi,

 These patches are intended to introduce Intel IOMMU (VT-d) emulation 
 to q35
 chipset. The major job in these patches is to add support for 
 emulating Intel
 IOMMU according to the VT-d specification, including basic responses 
 to CSRs
 accesses, the logics of DMAR (DMA remapping) and DMA memory address
 translations.

 Thanks!
 Looks very good overall, I noted some coding style issues - I didn't
 bother reporting each issue in every place where it appears - reported
 each issue once only, so please find and fix all instances of each
 issue.

 BTW, because I was in urgent need for virtual test environment for
 Jailhouse, I hacked interrupt remapping on top of Le's patches:

 http://git.kiszka.org/?p=qemu.git;a=shortlog;h=refs/heads/queues/vtd-intremap

 The approach likely needs further discussions and refinements but it
 already allows me to work on top with our hypervisor, and also Linux.
 You can see from the last commit that Le's work made it pretty easy to
 build this on top.

 Le,

 I have tried Jan's branch with my device setup which consists of a
 minimal q35 setup, an ioh3420 root port (specified as -device
 ioh3420,slot=0 ) and a pcie device plugged into that root port, which
 gives the following lscpi -t:

 -[:00]-+-00.0
+-01.0
+-02.0
+-03.0-[01]00.0
+-04.0
+-1f.0
+-1f.2
\-1f.3

 All seems to work beautifully (I see the ISA bridge happily receive
 translations) until the first DMA from my device model (at 1:00.0)
 arrives, at which point I get:

 [ 1663.732413] dmar: DMAR:[DMA Write] Request device [00:03.0] fault 
 addr fffa
 [ 1663.732413] DMAR:[fault reason 02] Present bit in context entry is 
 clear

 I would have expected request device 01:00.0 for this.
 It is not clear to me yet if this is a weakness of the implementation of
 ioh3420 or the iommu. Just wanted to let you know right away in case you
 can shed some light to it or it is an easy fix,

 The device uses pci_dma_rw with itself as device pointer.

 To verify my hypothesis: with this rude hack my device now works much
 better:

 @@ -774,6 +780,8 @@ static void iommu_translate(VTDAddressSpace *vtd_as,
 int bus_num, int devfn,
  is_fpd_set = ce.lo  VTD_CONTEXT_ENTRY_FPD;
  } else {
  ret_fr = dev_to_context_entry(s, bus_num, devfn, ce);
 +if (ret_fr)
 +ret_fr = dev_to_context_entry(s, 1, 0, ce);
  is_fpd_set = ce.lo  VTD_CONTEXT_ENTRY_FPD;
  if (ret_fr) {
  ret_fr = -ret_fr;

 Looking at how things look on hardware, multiple devices often receive
 overlapping DMA address ranges for different physical addresses.

 So if I understand the way this works, every requester ID would also
 need to have it's own unique VTDAddressSpace, as each pci
 device/function sees a unique DMA address space..

 ioh3420 is a pcie-to-pcie bridge, right? 

 Yes.

 In my opinion, each pci-e
 device behind the pcie-to-pcie bridge can be assigned individually.
 For now I added the VT-d to q35 by just adding it to the root pci bus.
 You can see here in q35.c:
 pci_setup_iommu(pci_bus, q35_host_dma_iommu, mch-iommu);
 So if we add a pcie-to-pcie bridge, we may have to call the
 pci_setup_iommu() for that new bus. I don't know where to hook into
 this now. :) If you know the mechanism behind that, you can try to add
 that for the new bus. (I will dive into this after the clean up.)
 What do you think?

 Thanks for the quick answer, that helped a lot!

 Looking into the details here I realize it is slightly more complicated:
 secondary buses are enumerated after device instantiation, as part of
 the host PCI enumeration, so if I add a similar setup call in the bridge
 setup, it will be called for a new device long before it has received
 it's bus number from the OS (via config[PCI_SECONDARY_BUS] )

 I agree that the lookup function for contexts needs to be as efficient
 as possible so the simple busno,defvn lookup key may be the best
 solution but then the address_spaces table cannot be populated with the
 secondary bus entries before it receives a nonzero != 255 bus number,
 eg. along the lines of this: 

 diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c
 index 4becdc1..d9a8c23 100644
 --- a/hw/pci/pci_bridge.c
 +++ b/hw/pci/pci_bridge.c
 @@ -265,6 +265,12 @@ void pci_bridge_write_config(PCIDevice *d,
  pci_bridge_update_mappings(s);
  }
  
 +if (ranges_overlap(address

Re: [Qemu-devel] [PATCH v3 0/5] intel-iommu: introduce Intel IOMMU (VT-d) emulation to q35 chipset

2014-08-18 Thread Jan Kiszka
On 2014-08-19 06:08, Knut Omang wrote:
 Are you depending on interrupt remapping? If not, my patches are a bit
 hacky and may cause their own issues if you are unlucky.
  
 It does not depend directly but interprets a NULL PciDevice pointer as
 the special bus number (0xff) for non-pci devices, so I have tried to

Yeah, this won't map nicely on future chipsets - unless we redefine
Q35_PSEUDO_XXX as VTD_PSEUDO_XXX, or so, and use the same values for all
of them. Or create an unconnected pseudo PCI bus for the chipset devices
that carry the pseudo bus number.

 take heights for that - I can rebase if so desired, but I would like to
 see the interrupt remapping emerge as well ;-)

Good, then we are already two ;). Let's get the baseline ready, then we
can discuss extensions like interrupt remapping.

Jan




signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH v3 0/5] intel-iommu: introduce Intel IOMMU (VT-d) emulation to q35 chipset

2014-08-16 Thread Jan Kiszka
On 2014-08-16 09:54, Knut Omang wrote:
 On Fri, 2014-08-15 at 19:37 +0800, Le Tan wrote:
 Hi Knut,

 2014-08-15 19:15 GMT+08:00 Knut Omang knut.om...@oracle.com:
 On Fri, 2014-08-15 at 06:42 +0200, Knut Omang wrote:
 On Thu, 2014-08-14 at 14:10 +0200, Jan Kiszka wrote:
 On 2014-08-14 13:15, Michael S. Tsirkin wrote:
 On Mon, Aug 11, 2014 at 03:04:57PM +0800, Le Tan wrote:
 Hi,

 These patches are intended to introduce Intel IOMMU (VT-d) emulation to 
 q35
 chipset. The major job in these patches is to add support for emulating 
 Intel
 IOMMU according to the VT-d specification, including basic responses to 
 CSRs
 accesses, the logics of DMAR (DMA remapping) and DMA memory address
 translations.

 Thanks!
 Looks very good overall, I noted some coding style issues - I didn't
 bother reporting each issue in every place where it appears - reported
 each issue once only, so please find and fix all instances of each
 issue.

 BTW, because I was in urgent need for virtual test environment for
 Jailhouse, I hacked interrupt remapping on top of Le's patches:

 http://git.kiszka.org/?p=qemu.git;a=shortlog;h=refs/heads/queues/vtd-intremap

 The approach likely needs further discussions and refinements but it
 already allows me to work on top with our hypervisor, and also Linux.
 You can see from the last commit that Le's work made it pretty easy to
 build this on top.

 Le,

 I have tried Jan's branch with my device setup which consists of a
 minimal q35 setup, an ioh3420 root port (specified as -device
 ioh3420,slot=0 ) and a pcie device plugged into that root port, which
 gives the following lscpi -t:

 -[:00]-+-00.0
+-01.0
+-02.0
+-03.0-[01]00.0
+-04.0
+-1f.0
+-1f.2
\-1f.3

 All seems to work beautifully (I see the ISA bridge happily receive
 translations) until the first DMA from my device model (at 1:00.0)
 arrives, at which point I get:

 [ 1663.732413] dmar: DMAR:[DMA Write] Request device [00:03.0] fault addr 
 fffa
 [ 1663.732413] DMAR:[fault reason 02] Present bit in context entry is clear

 I would have expected request device 01:00.0 for this.
 It is not clear to me yet if this is a weakness of the implementation of
 ioh3420 or the iommu. Just wanted to let you know right away in case you
 can shed some light to it or it is an easy fix,

 The device uses pci_dma_rw with itself as device pointer.

 To verify my hypothesis: with this rude hack my device now works much
 better:

 @@ -774,6 +780,8 @@ static void iommu_translate(VTDAddressSpace *vtd_as,
 int bus_num, int devfn,
  is_fpd_set = ce.lo  VTD_CONTEXT_ENTRY_FPD;
  } else {
  ret_fr = dev_to_context_entry(s, bus_num, devfn, ce);
 +if (ret_fr)
 +ret_fr = dev_to_context_entry(s, 1, 0, ce);
  is_fpd_set = ce.lo  VTD_CONTEXT_ENTRY_FPD;
  if (ret_fr) {
  ret_fr = -ret_fr;

 Looking at how things look on hardware, multiple devices often receive
 overlapping DMA address ranges for different physical addresses.

 So if I understand the way this works, every requester ID would also
 need to have it's own unique VTDAddressSpace, as each pci
 device/function sees a unique DMA address space..

 ioh3420 is a pcie-to-pcie bridge, right? 
 
 Yes.
 
 In my opinion, each pci-e
 device behind the pcie-to-pcie bridge can be assigned individually.
 For now I added the VT-d to q35 by just adding it to the root pci bus.
 You can see here in q35.c:
 pci_setup_iommu(pci_bus, q35_host_dma_iommu, mch-iommu);
 So if we add a pcie-to-pcie bridge, we may have to call the
 pci_setup_iommu() for that new bus. I don't know where to hook into
 this now. :) If you know the mechanism behind that, you can try to add
 that for the new bus. (I will dive into this after the clean up.)
 What do you think?
 
 Thanks for the quick answer, that helped a lot!
 
 Looking into the details here I realize it is slightly more complicated:
 secondary buses are enumerated after device instantiation, as part of
 the host PCI enumeration, so if I add a similar setup call in the bridge
 setup, it will be called for a new device long before it has received
 it's bus number from the OS (via config[PCI_SECONDARY_BUS] )
 
 I agree that the lookup function for contexts needs to be as efficient
 as possible so the simple busno,defvn lookup key may be the best
 solution but then the address_spaces table cannot be populated with the
 secondary bus entries before it receives a nonzero != 255 bus number,
 eg. along the lines of this: 
 
 diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c
 index 4becdc1..d9a8c23 100644
 --- a/hw/pci/pci_bridge.c
 +++ b/hw/pci/pci_bridge.c
 @@ -265,6 +265,12 @@ void pci_bridge_write_config(PCIDevice *d,
  pci_bridge_update_mappings(s);
  }
  
 +if (ranges_overlap(address, len, PCI_SECONDARY_BUS, 1)) {
 +int bus_num = pci_bus_num(s-sec_bus);
 +if (bus_num != 0xff  bus_num != 0x00

Re: [Qemu-devel] [PATCH v3 0/5] intel-iommu: introduce Intel IOMMU (VT-d) emulation to q35 chipset

2014-08-16 Thread Jan Kiszka
On 2014-08-16 10:45, Jan Kiszka wrote:
 On 2014-08-16 09:54, Knut Omang wrote:
 On Fri, 2014-08-15 at 19:37 +0800, Le Tan wrote:
 Hi Knut,

 2014-08-15 19:15 GMT+08:00 Knut Omang knut.om...@oracle.com:
 On Fri, 2014-08-15 at 06:42 +0200, Knut Omang wrote:
 On Thu, 2014-08-14 at 14:10 +0200, Jan Kiszka wrote:
 On 2014-08-14 13:15, Michael S. Tsirkin wrote:
 On Mon, Aug 11, 2014 at 03:04:57PM +0800, Le Tan wrote:
 Hi,

 These patches are intended to introduce Intel IOMMU (VT-d) emulation 
 to q35
 chipset. The major job in these patches is to add support for 
 emulating Intel
 IOMMU according to the VT-d specification, including basic responses 
 to CSRs
 accesses, the logics of DMAR (DMA remapping) and DMA memory address
 translations.

 Thanks!
 Looks very good overall, I noted some coding style issues - I didn't
 bother reporting each issue in every place where it appears - reported
 each issue once only, so please find and fix all instances of each
 issue.

 BTW, because I was in urgent need for virtual test environment for
 Jailhouse, I hacked interrupt remapping on top of Le's patches:

 http://git.kiszka.org/?p=qemu.git;a=shortlog;h=refs/heads/queues/vtd-intremap

 The approach likely needs further discussions and refinements but it
 already allows me to work on top with our hypervisor, and also Linux.
 You can see from the last commit that Le's work made it pretty easy to
 build this on top.

 Le,

 I have tried Jan's branch with my device setup which consists of a
 minimal q35 setup, an ioh3420 root port (specified as -device
 ioh3420,slot=0 ) and a pcie device plugged into that root port, which
 gives the following lscpi -t:

 -[:00]-+-00.0
+-01.0
+-02.0
+-03.0-[01]00.0
+-04.0
+-1f.0
+-1f.2
\-1f.3

 All seems to work beautifully (I see the ISA bridge happily receive
 translations) until the first DMA from my device model (at 1:00.0)
 arrives, at which point I get:

 [ 1663.732413] dmar: DMAR:[DMA Write] Request device [00:03.0] fault addr 
 fffa
 [ 1663.732413] DMAR:[fault reason 02] Present bit in context entry is 
 clear

 I would have expected request device 01:00.0 for this.
 It is not clear to me yet if this is a weakness of the implementation of
 ioh3420 or the iommu. Just wanted to let you know right away in case you
 can shed some light to it or it is an easy fix,

 The device uses pci_dma_rw with itself as device pointer.

 To verify my hypothesis: with this rude hack my device now works much
 better:

 @@ -774,6 +780,8 @@ static void iommu_translate(VTDAddressSpace *vtd_as,
 int bus_num, int devfn,
  is_fpd_set = ce.lo  VTD_CONTEXT_ENTRY_FPD;
  } else {
  ret_fr = dev_to_context_entry(s, bus_num, devfn, ce);
 +if (ret_fr)
 +ret_fr = dev_to_context_entry(s, 1, 0, ce);
  is_fpd_set = ce.lo  VTD_CONTEXT_ENTRY_FPD;
  if (ret_fr) {
  ret_fr = -ret_fr;

 Looking at how things look on hardware, multiple devices often receive
 overlapping DMA address ranges for different physical addresses.

 So if I understand the way this works, every requester ID would also
 need to have it's own unique VTDAddressSpace, as each pci
 device/function sees a unique DMA address space..

 ioh3420 is a pcie-to-pcie bridge, right? 

 Yes.

 In my opinion, each pci-e
 device behind the pcie-to-pcie bridge can be assigned individually.
 For now I added the VT-d to q35 by just adding it to the root pci bus.
 You can see here in q35.c:
 pci_setup_iommu(pci_bus, q35_host_dma_iommu, mch-iommu);
 So if we add a pcie-to-pcie bridge, we may have to call the
 pci_setup_iommu() for that new bus. I don't know where to hook into
 this now. :) If you know the mechanism behind that, you can try to add
 that for the new bus. (I will dive into this after the clean up.)
 What do you think?

 Thanks for the quick answer, that helped a lot!

 Looking into the details here I realize it is slightly more complicated:
 secondary buses are enumerated after device instantiation, as part of
 the host PCI enumeration, so if I add a similar setup call in the bridge
 setup, it will be called for a new device long before it has received
 it's bus number from the OS (via config[PCI_SECONDARY_BUS] )

 I agree that the lookup function for contexts needs to be as efficient
 as possible so the simple busno,defvn lookup key may be the best
 solution but then the address_spaces table cannot be populated with the
 secondary bus entries before it receives a nonzero != 255 bus number,
 eg. along the lines of this: 

 diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c
 index 4becdc1..d9a8c23 100644
 --- a/hw/pci/pci_bridge.c
 +++ b/hw/pci/pci_bridge.c
 @@ -265,6 +265,12 @@ void pci_bridge_write_config(PCIDevice *d,
  pci_bridge_update_mappings(s);
  }
  
 +if (ranges_overlap(address, len, PCI_SECONDARY_BUS, 1)) {
 +int bus_num = pci_bus_num(s-sec_bus

Re: [Qemu-devel] [PATCH v3 3/5] intel-iommu: add DMAR table to ACPI tables

2014-08-14 Thread Jan Kiszka
On 2014-08-14 13:06, Michael S. Tsirkin wrote:
 On Mon, Aug 11, 2014 at 03:05:00PM +0800, Le Tan wrote:
 Expose Intel IOMMU to the BIOS. If object of TYPE_INTEL_IOMMU_DEVICE exists,
 add DMAR table to ACPI RSDT table. For now the DMAR table indicates that 
 there
 is only one hardware unit without INTR_REMAP capability on the platform.

 Signed-off-by: Le Tan tamlokv...@gmail.com
 
 Could you add a unit test please?

While unit tests would really be helpful, I'm afraid that's not in reach
(GSoC is almost over). The good news is that we have pretty broad test
coverage with both Linux and also Jailhouse already.

Do you see unit tests as precondition for merging the series?

Jan




signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH v3 3/5] intel-iommu: add DMAR table to ACPI tables

2014-08-14 Thread Jan Kiszka
On 2014-08-14 13:43, Michael S. Tsirkin wrote:
 On Thu, Aug 14, 2014 at 01:36:49PM +0200, Jan Kiszka wrote:
 On 2014-08-14 13:06, Michael S. Tsirkin wrote:
 On Mon, Aug 11, 2014 at 03:05:00PM +0800, Le Tan wrote:
 Expose Intel IOMMU to the BIOS. If object of TYPE_INTEL_IOMMU_DEVICE 
 exists,
 add DMAR table to ACPI RSDT table. For now the DMAR table indicates that 
 there
 is only one hardware unit without INTR_REMAP capability on the platform.

 Signed-off-by: Le Tan tamlokv...@gmail.com

 Could you add a unit test please?

 While unit tests would really be helpful, I'm afraid that's not in reach
 (GSoC is almost over). The good news is that we have pretty broad test
 coverage with both Linux and also Jailhouse already.

 Do you see unit tests as precondition for merging the series?

 Jan

 
 Not a pre-requisite - it's very easy to add a unit test,
 just add a case in test_acpi_tcg. So I can do it myself afterwards.
 

Ah, ok, that's tests/bios-tables-test.c. Maybe Le can have a look if
time is left. But hard pecils-down is already on the 18th.

Jan




signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH v3 0/5] intel-iommu: introduce Intel IOMMU (VT-d) emulation to q35 chipset

2014-08-14 Thread Jan Kiszka
On 2014-08-14 13:15, Michael S. Tsirkin wrote:
 On Mon, Aug 11, 2014 at 03:04:57PM +0800, Le Tan wrote:
 Hi,

 These patches are intended to introduce Intel IOMMU (VT-d) emulation to q35
 chipset. The major job in these patches is to add support for emulating Intel
 IOMMU according to the VT-d specification, including basic responses to CSRs
 accesses, the logics of DMAR (DMA remapping) and DMA memory address
 translations.
 
 Thanks!
 Looks very good overall, I noted some coding style issues - I didn't
 bother reporting each issue in every place where it appears - reported
 each issue once only, so please find and fix all instances of each
 issue.

BTW, because I was in urgent need for virtual test environment for
Jailhouse, I hacked interrupt remapping on top of Le's patches:

http://git.kiszka.org/?p=qemu.git;a=shortlog;h=refs/heads/queues/vtd-intremap

The approach likely needs further discussions and refinements but it
already allows me to work on top with our hypervisor, and also Linux.
You can see from the last commit that Le's work made it pretty easy to
build this on top.

Jan




signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] MSI-X interrupt emulation

2014-08-13 Thread Jan Kiszka
On 2014-08-01 08:22, Danzer, Uwe wrote:
 Hi there,
 
 I'm implementing an emulated PCIe Memory class device, but can't get MSI-X 
 interrupt emulation working.
 
 So far, the card appears in the guest system and the driver for the card 
 recognises it and the emulation of 1MB of accessible r/w registers works as 
 desired.
 
 As the real card is connected to the outside world, it can signal events from 
 there to the software via 5 MSI-X interrupts. Though I do not manage do get 
 MSI-X emulation working in my implementation.
 
 The guest OS is QNX and works just fine on the real hardware. Running QNX as 
 guest inside QEMU, the command pci -vvv (QNX equivalent of lspci on Linux) 
 shows my card and that it says it's able to do the desired 5 MSI-X 
 interrupts, but the QNX driver doesn't activate MSI-X for the card.
 

I suppose the config space layouts are identical between emulated and
real card? Just in case the QNX driver has hard-coded capability offsets
(it shouldn't, but who knows).

Do you have the source code of the driver?

 In my init function of the PCIe card, I try to make MSI-X available with this 
 code:
 
 ret = msix_init_exclusive_bar (dev, 5, 1);
 if (ret) {
 printf(msix_init() failed\n);
 } else {
 int i;
 
 for (i = 0; i  5; i++) {
 msix_vector_use (dev, i);
 }
 
 msix = 1;
 }
 
 Can someone tell me, what is wrong or missing in my code or does somebody 
 have a minimal example for a (pseudo)device with MSI-X?

If the problem still persists and you can't share your code here, drop
me an email directly, and I can have a look or suggest further
instrumentations.

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux



[Qemu-devel] [PATCH][RESEND] pci: Use bus master address space for delivering MSI/MSI-X, messages

2014-08-13 Thread Jan Kiszka
The spec says (and real HW confirms this) that, if the bus master bit
is 0, the device will not generate any PCI accesses. MSI and MSI-X
messages fall among these, so we should use the corresponding address
space to deliver them. This will prevent delivery if bus master support
is disabled.

Signed-off-by: Jan Kiszka jan.kis...@siemens.com
Reviewed-by: Paolo Bonzini pbonz...@redhat.com
Reviewed-by: Michael S. Tsirkin m...@redhat.com

---
 hw/pci/msi.c  | 2 +-
 hw/pci/msix.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/pci/msi.c b/hw/pci/msi.c
index a4a3040..52d2313 100644
--- a/hw/pci/msi.c
+++ b/hw/pci/msi.c
@@ -291,7 +291,7 @@ void msi_notify(PCIDevice *dev, unsigned int vector)
notify vector 0x%x
 address: 0x%PRIx64 data: 0x%PRIx32\n,
vector, msg.address, msg.data);
-stl_le_phys(address_space_memory, msg.address, msg.data);
+stl_le_phys(dev-bus_master_as, msg.address, msg.data);
 }
 
 /* Normally called by pci_default_write_config(). */
diff --git a/hw/pci/msix.c b/hw/pci/msix.c
index 5c49bfc..20ae476 100644
--- a/hw/pci/msix.c
+++ b/hw/pci/msix.c
@@ -439,7 +439,7 @@ void msix_notify(PCIDevice *dev, unsigned vector)
 
 msg = msix_get_message(dev, vector);
 
-stl_le_phys(address_space_memory, msg.address, msg.data);
+stl_le_phys(dev-bus_master_as, msg.address, msg.data);
 }
 
 void msix_reset(PCIDevice *dev)
-- 
1.8.1.1.298.ge7eed54



Re: [Qemu-devel] [Qemu-trivial] [PATCH] apic: Fix reported DFR content

2014-08-13 Thread Jan Kiszka
On 2014-08-13 08:14, Michael Tokarev wrote:
 09.08.2014 18:05, Jan Kiszka wrote:
 From: Jan Kiszka jan.kis...@siemens.com

 IA-32 SDM, Figure 10-14: Bits 27:0 are reserved as 1.

 Fixes Jailhouse hypervisor start with in-kernel irqchips off.
 
 Applied to -trivial, thank you!
 
 Are the other similar cases in there okay?
 Say, 0x0d or 0x02 which also uses shifts like this one?

Nope, didn't find any.

Jan




signature.asc
Description: OpenPGP digital signature


[Qemu-devel] [PATCH] apic: Fix reported DFR content

2014-08-09 Thread Jan Kiszka
From: Jan Kiszka jan.kis...@siemens.com

IA-32 SDM, Figure 10-14: Bits 27:0 are reserved as 1.

Fixes Jailhouse hypervisor start with in-kernel irqchips off.

Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---
 hw/intc/apic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/intc/apic.c b/hw/intc/apic.c
index ef19e55..03ff9e9 100644
--- a/hw/intc/apic.c
+++ b/hw/intc/apic.c
@@ -698,7 +698,7 @@ static uint32_t apic_mem_readl(void *opaque, hwaddr addr)
 val = s-log_dest  24;
 break;
 case 0x0e:
-val = s-dest_mode  28;
+val = (s-dest_mode  28) | 0xfff;
 break;
 case 0x0f:
 val = s-spurious_vec;
-- 
1.8.1.1.298.ge7eed54



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] About the VT-d features that q35 chipset supports

2014-08-05 Thread Jan Kiszka
On 2014-08-05 14:55, Le Tan wrote:
 Hi,
 I am now adding features to the VT-d emulation for q35 chipset. I
 think it is good to know what features q35 chipset supports exactly.
 However I can't find materials about this. Does anyone know this? Or
 it will be fine if someone can tell me the value of the Capability
 Register and Extended Capability Register of the VT-d on q35.

The kernel reports those registers during boot-up, so we would just need
dmesg | grep dmar from a real 82Q35 box (Intel 3 Series).

Jan

 Any help is appreciated!
 Thanks very much!
 
 Regards,
 Le
 




signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] vfio in the guest: no available reset mechanism

2014-08-01 Thread Jan Kiszka
On 2014-08-01 18:35, Alex Williamson wrote:
 On Fri, 2014-08-01 at 09:25 -0600, Alex Williamson wrote:
 On Fri, 2014-08-01 at 09:35 +0800, Le Tan wrote:
 Hi Alex,

 2014-07-30 22:46 GMT+08:00 Alex Williamson alex.william...@redhat.com:
 On Wed, 2014-07-30 at 22:16 +0800, Le Tan wrote:
 Hi Michael,

 2014-07-30 21:16 GMT+08:00 Michael S. Tsirkin m...@redhat.com:
 On Wed, Jul 30, 2014 at 08:24:04PM +0800, Le Tan wrote:
 Hi,
 I am testing vfio in L1 with my VT-d emulation project. I assigned one
 of the two AHCI controllers in L1 to L2 via vfio. After I ran the QEMU
 in L1, it complains that:
 qemu-system-x86_64: vfio: Cannot reset device :00:03.0, no
 available reset mechanism.
 qemu-system-x86_64: vfio: Cannot reset device :00:03.0, no
 available reset mechanism.

 Then L2 paused when the SeaBIOS executed in ahci_controller_setup(). I
 look into this and found that:
 val = ahci_ctrl_readl(ctrl, HOST_CTL);
 ahci_ctrl_writel(ctrl, HOST_CTL, val | HOST_CTL_AHCI_EN);
 When the BIOS tried to read the HOST_CTL, it returns 0x8002, which
 bit 2 (Interrupt Enable) is 1. The AHCI manual says that this bit
 should be cleared by default. So maybe L1 didn't reset the device
 before assigning it to L2?
 Then the BIOS tried to write back to HOST_CTL and it was stuck here. :(

 So can anyone give me some advice? About the state of PCI device or
 bus-level reset?

 Here is the detail of the environment and the way I did the vfio.
 1. lspci in L1 said:
 00:03.0 SATA controller [0106]: Intel Corporation 82801IR/IO/IH
 (ICH9R/DO/DH) 6 port SATA Controller [AHCI mode] [8086:2922] (rev 02)
 00:1f.0 ISA bridge [0601]: Intel Corporation 82801IB (ICH9) LPC
 Interface Controller [8086:2918] (rev 02)
 00:1f.2 SATA controller [0106]: Intel Corporation 82801IR/IO/IH
 (ICH9R/DO/DH) 6 port SATA Controller [AHCI mode] [8086:2922] (rev 02)
 00:1f.3 SMBus [0c05]: Intel Corporation 82801I (ICH9 Family) SMBus
 Controller [8086:2930] (rev 02)
 2. Unbind 00:03.0 and do vfio:
 modprobe -r vfio_iommu_type1
 modprobe vfio_iommu_type1 allow_unsafe_interrupts=1
 modprobe vfio-pci
 echo :00:03.0  /sys/bus/pci/devices/\:00\:03.0/driver/unbind
 echo 8086 2922  /sys/bus/pci/drivers/vfio-pci/new_id
 3. run L2 with -device vfio-pci,host=00:03.0

 Any help is appreciated! Thanks very much!

 Regards,
 Le

 Clearly, bus level reset can't work for the root bus :)

 Thanks very much!
 I test the vfio with a second-bus ahci controller and it didn't
 complain about the lack of reset mechanism. :) And the return val of
 HOST_CTL is normal now (the same as emulated ahci controller).
 However, it still paused when the BIOS tried to write to the HOST_CTL.
 Do you have any idea?
 And we should just test vfio and legacy pci-assignment with second bus
 devices, not considering the root-bus devices?

 AHCI seems like a poor choice of device for this work, they typically
 don't support any kind of reset and they can be troublesome even for the
 L1 assignment.  You really want something with FLR support so that both
 the host and L1 guest can potentially reset the device.  That said, you
 may still run into bugs with a L1 guest directed FLR.  Thanks,

 Alex


 So what device do you think is suitable for the pci-assign test? e1000?
 I just tested it with sound card ac97 and USB controller. But I don't
 know how to attach them to a pcie-to-pci bridge, so maybe they weren't
 reset before being assigned to L2. But it seems that they can work.
 1. With the sound card, I assigned it to L2 via both vfio and legacy
 pci-assign and I could hear the music played in L2 from host's
 speakers. Of course, the vfio also complained about the lack of reset
 mechanism.

 First off, maybe I'm a little confused, are you trying to assigned
 emulated devices for the L1 guest to the L2 guest or are these assigned
 devices to the L1 guest that you want to re-assign to L2?  AFAIK, we
 don't have any emulated devices that support reset, but it wouldn't be
 that hard to add a PM capability to one that advertises a soft reset on
 D3hot-D0 transition and calls the QEMU driver reset function when that
 occurs.  This would provide the most flexibility.
 
 Or even better might be to add an Advanced Features capability to each
 device so it can do an explicit FLR.  We don't tend to trust PM reset as
 being reliably on all devices.

But FLR is PCIe-only, isn't it? Also, it may let some of our device
models deviate from their real versions (I suppose, e.g., none of the
e1000 devices we currently emulate exposed FLR).

In any case, I suggested Le to focus on his GSoC task again (VT-d
emulation) and postpone this topic to later point (likely outside GSoC).
Only few weeks left...

Jan




signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] vfio in the guest: no available reset mechanism

2014-08-01 Thread Jan Kiszka
On 2014-08-01 19:16, Alex Williamson wrote:
  Also, it may let some of our device
 models deviate from their real versions (I suppose, e.g., none of the
 e1000 devices we currently emulate exposed FLR).
 
 Of course, but what are the chances that the driver will care?

No drivers of GPOSes, but special or legacy OSes may do so. Keep in mind
that Intel e.g. is documenting their PCI devices with fixed config space
addresses for their capability.

Also, we completely lack PM caps so far. Adding them would already have
the value of increasing emulation accuracy. And here I think we are free
to always implement reset behavior behind D3-D0 transitions. So my
believe is that this will be the better path.

Jan




signature.asc
Description: OpenPGP digital signature


[Qemu-devel] [PATCH 2/3] pc: Create 2.2 machine type

2014-07-30 Thread Jan Kiszka
Yet identical to 2.1.

Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---
 hw/i386/pc_piix.c| 19 ---
 hw/i386/pc_q35.c | 17 +++--
 include/hw/i386/pc.h |  3 +++
 3 files changed, 34 insertions(+), 5 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 73ba77d..5edf42a 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -450,16 +450,28 @@ static void pc_xen_hvm_init(MachineState *machine)
 .desc = Standard PC (i440FX + PIIX, 1996), \
 .hot_add_cpu = pc_hot_add_cpu
 
-#define PC_I440FX_2_1_MACHINE_OPTIONS   \
+#define PC_I440FX_2_2_MACHINE_OPTIONS   \
 PC_I440FX_MACHINE_OPTIONS,  \
 .default_machine_opts = firmware=bios-256k.bin
 
+static QEMUMachine pc_i440fx_machine_v2_2 = {
+PC_I440FX_2_2_MACHINE_OPTIONS,
+.name = pc-i440fx-2.2,
+.alias = pc,
+.init = pc_init_pci,
+.is_default = 1,
+};
+
+#define PC_I440FX_2_1_MACHINE_OPTIONS PC_I440FX_2_2_MACHINE_OPTIONS
+
 static QEMUMachine pc_i440fx_machine_v2_1 = {
 PC_I440FX_2_1_MACHINE_OPTIONS,
 .name = pc-i440fx-2.1,
-.alias = pc,
 .init = pc_init_pci,
-.is_default = 1,
+.compat_props = (GlobalProperty[]) {
+PC_COMPAT_2_1,
+{ /* end of list */ }
+},
 };
 
 #define PC_I440FX_2_0_MACHINE_OPTIONS PC_I440FX_2_1_MACHINE_OPTIONS
@@ -896,6 +908,7 @@ static QEMUMachine xenfv_machine = {
 
 static void pc_machine_init(void)
 {
+qemu_register_pc_machine(pc_i440fx_machine_v2_2);
 qemu_register_pc_machine(pc_i440fx_machine_v2_1);
 qemu_register_pc_machine(pc_i440fx_machine_v2_0);
 qemu_register_pc_machine(pc_i440fx_machine_v1_7);
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index c39ee98..fcc3ad7 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -348,15 +348,27 @@ static void pc_q35_init_1_4(MachineState *machine)
 .desc = Standard PC (Q35 + ICH9, 2009), \
 .hot_add_cpu = pc_hot_add_cpu
 
-#define PC_Q35_2_1_MACHINE_OPTIONS  \
+#define PC_Q35_2_2_MACHINE_OPTIONS  \
 PC_Q35_MACHINE_OPTIONS, \
 .default_machine_opts = firmware=bios-256k.bin
 
+static QEMUMachine pc_q35_machine_v2_2 = {
+PC_Q35_2_2_MACHINE_OPTIONS,
+.name = pc-q35-2.2,
+.alias = q35,
+.init = pc_q35_init,
+};
+
+#define PC_Q35_2_1_MACHINE_OPTIONS PC_Q35_2_2_MACHINE_OPTIONS
+
 static QEMUMachine pc_q35_machine_v2_1 = {
 PC_Q35_2_1_MACHINE_OPTIONS,
 .name = pc-q35-2.1,
-.alias = q35,
 .init = pc_q35_init,
+.compat_props = (GlobalProperty[]) {
+PC_COMPAT_2_1,
+{ /* end of list */ }
+},
 };
 
 #define PC_Q35_2_0_MACHINE_OPTIONS PC_Q35_2_1_MACHINE_OPTIONS
@@ -421,6 +433,7 @@ static QEMUMachine pc_q35_machine_v1_4 = {
 
 static void pc_q35_machine_init(void)
 {
+qemu_register_pc_machine(pc_q35_machine_v2_2);
 qemu_register_pc_machine(pc_q35_machine_v2_1);
 qemu_register_pc_machine(pc_q35_machine_v2_0);
 qemu_register_pc_machine(pc_q35_machine_v1_7);
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index f4b9b2b..0fb4200 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -295,7 +295,10 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t);
 int e820_get_num_entries(void);
 bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
 
+#define PC_COMPAT_2_1
+
 #define PC_COMPAT_2_0 \
+PC_COMPAT_2_1, \
 {\
 .driver   = virtio-scsi-pci,\
 .property = any_layout,\
-- 
1.8.1.1.298.ge7eed54




[Qemu-devel] [PATCH 3/3] hw/audio/intel-hda: Fix MSI capability address

2014-07-30 Thread Jan Kiszka
According to ICH9 spec, the MSI capability is located at 0x60. This is
important for guest drivers that do not parse the capability chain and
use absolute addresses instead.

CC: Gerd Hoffmann kra...@redhat.com
Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---
 hw/audio/intel-hda.c | 4 +++-
 include/hw/i386/pc.h | 7 ++-
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c
index aa49b47..0ac911e 100644
--- a/hw/audio/intel-hda.c
+++ b/hw/audio/intel-hda.c
@@ -187,6 +187,7 @@ struct IntelHDAState {
 /* properties */
 uint32_t debug;
 uint32_t msi;
+bool old_msi_addr;
 };
 
 #define TYPE_INTEL_HDA_GENERIC intel-hda-generic
@@ -1141,7 +1142,7 @@ static int intel_hda_init(PCIDevice *pci)
   intel-hda, 0x4000);
 pci_register_bar(d-pci, 0, 0, d-mmio);
 if (d-msi) {
-msi_init(d-pci, 0x50, 1, true, false);
+msi_init(d-pci, d-old_msi_addr ? 0x50 : 0x60, 1, true, false);
 }
 
 hda_codec_bus_init(DEVICE(pci), d-codecs, sizeof(d-codecs),
@@ -1236,6 +1237,7 @@ static const VMStateDescription vmstate_intel_hda = {
 static Property intel_hda_properties[] = {
 DEFINE_PROP_UINT32(debug, IntelHDAState, debug, 0),
 DEFINE_PROP_UINT32(msi, IntelHDAState, msi, 1),
+DEFINE_PROP_BOOL(old_msi_addr, IntelHDAState, old_msi_addr, false),
 DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 0fb4200..be8fdfe 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -295,7 +295,12 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t);
 int e820_get_num_entries(void);
 bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
 
-#define PC_COMPAT_2_1
+#define PC_COMPAT_2_1 \
+{\
+.driver   = intel-hda,\
+.property = old_msi_addr,\
+.value= on,\
+}
 
 #define PC_COMPAT_2_0 \
 PC_COMPAT_2_1, \
-- 
1.8.1.1.298.ge7eed54




[Qemu-devel] [PATCH 0/3] pc: Small fixes for Intel HDA config space and PC_COMPAT_1_0

2014-07-30 Thread Jan Kiszka
Patch 1 is a repost, likely a harmless issue as no one should have used
QEMU 1.0 for such purposes that require compat migration (rather
qemu-kvm at that time).

Patch 2 prepares compat support for 2.1 machines and the last one fixes
the MSI capability address of intel-hda in backward-compatible way.


CC: Gerd Hoffmann kra...@redhat.com

Jan Kiszka (3):
  pc: Fix disabling of vapic for compat PC models
  pc: Create 2.2 machine type
  hw/audio/intel-hda: Fix MSI capability address

 hw/audio/intel-hda.c |  4 +++-
 hw/i386/pc_piix.c| 21 +
 hw/i386/pc_q35.c | 17 +++--
 include/hw/i386/pc.h |  8 
 4 files changed, 43 insertions(+), 7 deletions(-)

-- 
1.8.1.1.298.ge7eed54




[Qemu-devel] [PATCH 1/3] pc: Fix disabling of vapic for compat PC models

2014-07-30 Thread Jan Kiszka
We used to be able to address both the QEMU and the KVM APIC via apic.
This doesn't work anymore. So we need to use their parent class to turn
off the vapic on machines that should not expose them.

Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---
 hw/i386/pc_piix.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 9694f88..73ba77d 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -645,7 +645,7 @@ static QEMUMachine pc_machine_v1_1 = {
 .property = class,\
 .value= stringify(PCI_CLASS_MEMORY_RAM),\
 },{\
-.driver   = apic,\
+.driver   = apic-common,\
 .property = vapic,\
 .value= off,\
 },{\
-- 
1.8.1.1.298.ge7eed54




Re: [Qemu-devel] [PATCH 2/3] pc: Create 2.2 machine type

2014-07-30 Thread Jan Kiszka
On 2014-07-30 10:41, Alex Bligh wrote:
 On 30/07/2014 08:02, Jan Kiszka wrote:
 Yet identical to 2.1.
 
 If it's *really* identical to 2.1, are we going to add the
 2.2 machine type to 2.1 as well, so that things that
 have difficulty using different source and destination
 machine types can migrate from 2.2 to 2.1?
 
 Or am I missing the point and is the issue that 2.2
 is actually different to 2.1, not in the machine
 type definition, but in what is sent within the
 migrated data? If so, a commit message to this
 effect might be helpful.

See patch 3 for the (current) motivation of this patch. Maybe more will
come as 2.2 evolves.

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] [PATCH] hw/audio/intel-hda: Fix MSI capability address

2014-07-28 Thread Jan Kiszka
On 2014-07-28 10:11, Paolo Bonzini wrote:
 Il 27/07/2014 08:57, Jan Kiszka ha scritto:
 From: Jan Kiszka jan.kis...@siemens.com

 According to ICH9 spec, the MSI capability is located at 0x60. This is
 important for guest drivers that do not parse the capability chain and
 use absolute addresses instead.

 Signed-off-by: Jan Kiszka jan.kis...@siemens.com
 ---
  hw/audio/intel-hda.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c
 index aa49b47..09c4118 100644
 --- a/hw/audio/intel-hda.c
 +++ b/hw/audio/intel-hda.c
 @@ -1141,7 +1141,7 @@ static int intel_hda_init(PCIDevice *pci)
intel-hda, 0x4000);
  pci_register_bar(d-pci, 0, 0, d-mmio);
  if (d-msi) {
 -msi_init(d-pci, 0x50, 1, true, false);
 +msi_init(d-pci, 0x60, 1, true, false);
  }

  hda_codec_bus_init(DEVICE(pci), d-codecs, sizeof(d-codecs),

 
 Does this need a compat property?

Sigh, right, it's guest visible.

Jan




signature.asc
Description: OpenPGP digital signature


[Qemu-devel] [PATCH] hw/audio/intel-hda: Fix MSI capability address

2014-07-27 Thread Jan Kiszka
From: Jan Kiszka jan.kis...@siemens.com

According to ICH9 spec, the MSI capability is located at 0x60. This is
important for guest drivers that do not parse the capability chain and
use absolute addresses instead.

Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---
 hw/audio/intel-hda.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c
index aa49b47..09c4118 100644
--- a/hw/audio/intel-hda.c
+++ b/hw/audio/intel-hda.c
@@ -1141,7 +1141,7 @@ static int intel_hda_init(PCIDevice *pci)
   intel-hda, 0x4000);
 pci_register_bar(d-pci, 0, 0, d-mmio);
 if (d-msi) {
-msi_init(d-pci, 0x50, 1, true, false);
+msi_init(d-pci, 0x60, 1, true, false);
 }

 hda_codec_bus_init(DEVICE(pci), d-codecs, sizeof(d-codecs),
-- 
1.8.1.1.298.ge7eed54



signature.asc
Description: OpenPGP digital signature


[Qemu-devel] [PATCH] pci: Use bus master address space for delivering MSI/MSI-X messages

2014-07-27 Thread Jan Kiszka
From: Jan Kiszka jan.kis...@siemens.com

The spec says (and real HW confirms this) that, if the bus master bit
is 0, the device will not generate any PCI accesses. MSI and MSI-X
messages fall among these, so we should use the corresponding address
space to deliver them. This will prevent delivery if bus master support
is disabled.

Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---
 hw/pci/msi.c  | 2 +-
 hw/pci/msix.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/pci/msi.c b/hw/pci/msi.c
index a4a3040..52d2313 100644
--- a/hw/pci/msi.c
+++ b/hw/pci/msi.c
@@ -291,7 +291,7 @@ void msi_notify(PCIDevice *dev, unsigned int vector)
notify vector 0x%x
 address: 0x%PRIx64 data: 0x%PRIx32\n,
vector, msg.address, msg.data);
-stl_le_phys(address_space_memory, msg.address, msg.data);
+stl_le_phys(dev-bus_master_as, msg.address, msg.data);
 }
 
 /* Normally called by pci_default_write_config(). */
diff --git a/hw/pci/msix.c b/hw/pci/msix.c
index 5c49bfc..20ae476 100644
--- a/hw/pci/msix.c
+++ b/hw/pci/msix.c
@@ -439,7 +439,7 @@ void msix_notify(PCIDevice *dev, unsigned vector)
 
 msg = msix_get_message(dev, vector);
 
-stl_le_phys(address_space_memory, msg.address, msg.data);
+stl_le_phys(dev-bus_master_as, msg.address, msg.data);
 }
 
 void msix_reset(PCIDevice *dev)
-- 
1.8.1.1.298.ge7eed54



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH 3/3] intel-iommu: add Intel IOMMU emulation to q35 and add a machine option vtd as a switch

2014-07-26 Thread Jan Kiszka
On 2014-07-22 17:47, Le Tan wrote:
 Add Intel IOMMU emulation to q35 chipset and expose it to the guest.
 1. Add a machine option. Users can use -machine vtd=on|off in the command
 line to enable/disable Intel IOMMU. The default is off.

Better call it iommu. We could once reuse the switch when someone adds
an AMD IOMMU emulation (and a corresponding chipset as well, I suppose).
And maybe other target archs will find it useful as well.

Jan




signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH] pci: Don't deliver MSI/MSI-X messages if bus master support is off

2014-07-23 Thread Jan Kiszka
On 2014-07-22 21:06, Michael S. Tsirkin wrote:
 On Mon, Jul 21, 2014 at 12:04:22AM +0200, Jan Kiszka wrote:
 On 2014-07-20 23:03, Michael S. Tsirkin wrote:
 On Sun, Jul 20, 2014 at 11:45:10PM +0200, Jan Kiszka wrote:
 On 2014-07-20 21:48, Michael S. Tsirkin wrote:
 On Sat, Jul 19, 2014 at 06:55:48PM +0200, Jan Kiszka wrote:
 From: Jan Kiszka jan.kis...@siemens.com

 The spec says (and real HW confirms this) that, if the bus master bit
 is 0, the device will not generate any PCI accesses. MSI and MSI-X
 messages fall among these.

 Signed-off-by: Jan Kiszka jan.kis...@siemens.com

 I guess an alternative is for callers to check before
 invoking msi_notify. Please note is this is only option
 when using e.g. irqfd, so this has some advantages.
 Is there a specific device that is affected by this?
 I would expect drivers to disable msi before clearing
 bus master bit ...

 This is about emulating conforming behaviour without touching each and
 every device. I stumbled over this while playing with emulated vs. real
 Intel HDA.

 Right so that's my question.
 How did you hit it? With a custom driver?

 So to say: with a hand full lines of code to tickle some MSI event out
 of that device for testing purposes.

 Doesn't regulat driver disable MSI?

 Sure. This is not fixing a regular's driver problem. It's a behavioral
 correction for faulty corner cases.

 Jan
 
 OK based on this I think this is not 2.1 material. Agree?

Agree.

I'll look into Paolo's suggestion how to model this asap.

Jan




signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH] kvm-all: Use 'tmpcpu' instead of 'cpu' in sub-looping to avoid 'cpu' be NULL

2014-07-20 Thread Jan Kiszka
On 2014-07-19 03:21, Chen Gang wrote:
 If kvm_arch_remove_sw_breakpoint() in CPU_FOREACH() always be fail, it
 will let 'cpu' NULL. And the next kvm_arch_remove_sw_breakpoint() in
 QTAILQ_FOREACH_SAFE() will get NULL parameter for 'cpu'.
 
 And kvm_arch_remove_sw_breakpoint() can assumes 'cpu' must never be NULL,
 so need define additional temporary variable for 'cpu' to avoid the case.
 
 
 Signed-off-by: Chen Gang gang.chen.5...@gmail.com
 ---
  kvm-all.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)
 
 diff --git a/kvm-all.c b/kvm-all.c
 index 3ae30ee..1402f4f 100644
 --- a/kvm-all.c
 +++ b/kvm-all.c
 @@ -2077,12 +2077,13 @@ void kvm_remove_all_breakpoints(CPUState *cpu)
  {
  struct kvm_sw_breakpoint *bp, *next;
  KVMState *s = cpu-kvm_state;
 +CPUState *tmpcpu;
  
  QTAILQ_FOREACH_SAFE(bp, s-kvm_sw_breakpoints, entry, next) {
  if (kvm_arch_remove_sw_breakpoint(cpu, bp) != 0) {
  /* Try harder to find a CPU that currently sees the breakpoint. 
 */
 -CPU_FOREACH(cpu) {
 -if (kvm_arch_remove_sw_breakpoint(cpu, bp) == 0) {
 +CPU_FOREACH(tmpcpu) {
 +if (kvm_arch_remove_sw_breakpoint(tmpcpu, bp) == 0) {
  break;
  }
  }
 

Good catch. To make it clear in the changelog: The actual issue is that
we misuse cpu as an iteration variable while its original value is
still in use. That cpu can eventually become NULL this way is one result.

Jan



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH] pci: Don't deliver MSI/MSI-X messages if bus master support is off

2014-07-20 Thread Jan Kiszka
On 2014-07-20 21:48, Michael S. Tsirkin wrote:
 On Sat, Jul 19, 2014 at 06:55:48PM +0200, Jan Kiszka wrote:
 From: Jan Kiszka jan.kis...@siemens.com

 The spec says (and real HW confirms this) that, if the bus master bit
 is 0, the device will not generate any PCI accesses. MSI and MSI-X
 messages fall among these.

 Signed-off-by: Jan Kiszka jan.kis...@siemens.com
 
 I guess an alternative is for callers to check before
 invoking msi_notify. Please note is this is only option
 when using e.g. irqfd, so this has some advantages.
 Is there a specific device that is affected by this?
 I would expect drivers to disable msi before clearing
 bus master bit ...

This is about emulating conforming behaviour without touching each and
every device. I stumbled over this while playing with emulated vs. real
Intel HDA.

It may not be complete, but I think it's a step forward. Irqfd users
apparently have to do this themselves then, I didn't look into this. But
all the rest should not open-code this logic.

Jan

 
 ---
  hw/pci/msi.c  | 4 
  hw/pci/msix.c | 4 
  2 files changed, 8 insertions(+)

 diff --git a/hw/pci/msi.c b/hw/pci/msi.c
 index a4a3040..36b651b 100644
 --- a/hw/pci/msi.c
 +++ b/hw/pci/msi.c
 @@ -285,6 +285,10 @@ void msi_notify(PCIDevice *dev, unsigned int vector)
  return;
  }
  
 +if (!(pci_get_word(dev-config + PCI_COMMAND)  PCI_COMMAND_MASTER)) {
 +return;
 +}
 +
  msg = msi_get_message(dev, vector);
  
  MSI_DEV_PRINTF(dev,
 diff --git a/hw/pci/msix.c b/hw/pci/msix.c
 index 5c49bfc..c77ae7d 100644
 --- a/hw/pci/msix.c
 +++ b/hw/pci/msix.c
 @@ -437,6 +437,10 @@ void msix_notify(PCIDevice *dev, unsigned vector)
  return;
  }
  
 +if (!(pci_get_word(dev-config + PCI_COMMAND)  PCI_COMMAND_MASTER)) {
 +return;
 +}
 +
  msg = msix_get_message(dev, vector);
  
  stl_le_phys(address_space_memory, msg.address, msg.data);
 -- 
 1.8.1.1.298.ge7eed54

 
 




signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH] pci: Don't deliver MSI/MSI-X messages if bus master support is off

2014-07-20 Thread Jan Kiszka
On 2014-07-20 23:03, Michael S. Tsirkin wrote:
 On Sun, Jul 20, 2014 at 11:45:10PM +0200, Jan Kiszka wrote:
 On 2014-07-20 21:48, Michael S. Tsirkin wrote:
 On Sat, Jul 19, 2014 at 06:55:48PM +0200, Jan Kiszka wrote:
 From: Jan Kiszka jan.kis...@siemens.com

 The spec says (and real HW confirms this) that, if the bus master bit
 is 0, the device will not generate any PCI accesses. MSI and MSI-X
 messages fall among these.

 Signed-off-by: Jan Kiszka jan.kis...@siemens.com

 I guess an alternative is for callers to check before
 invoking msi_notify. Please note is this is only option
 when using e.g. irqfd, so this has some advantages.
 Is there a specific device that is affected by this?
 I would expect drivers to disable msi before clearing
 bus master bit ...

 This is about emulating conforming behaviour without touching each and
 every device. I stumbled over this while playing with emulated vs. real
 Intel HDA.
 
 Right so that's my question.
 How did you hit it? With a custom driver?

So to say: with a hand full lines of code to tickle some MSI event out
of that device for testing purposes.

 Doesn't regulat driver disable MSI?

Sure. This is not fixing a regular's driver problem. It's a behavioral
correction for faulty corner cases.

Jan

 
 
 It may not be complete, but I think it's a step forward. Irqfd users
 apparently have to do this themselves then, I didn't look into this. But
 all the rest should not open-code this logic.

 Jan


 ---
  hw/pci/msi.c  | 4 
  hw/pci/msix.c | 4 
  2 files changed, 8 insertions(+)

 diff --git a/hw/pci/msi.c b/hw/pci/msi.c
 index a4a3040..36b651b 100644
 --- a/hw/pci/msi.c
 +++ b/hw/pci/msi.c
 @@ -285,6 +285,10 @@ void msi_notify(PCIDevice *dev, unsigned int vector)
  return;
  }
  
 +if (!(pci_get_word(dev-config + PCI_COMMAND)  PCI_COMMAND_MASTER)) {
 +return;
 +}
 +
  msg = msi_get_message(dev, vector);
  
  MSI_DEV_PRINTF(dev,
 diff --git a/hw/pci/msix.c b/hw/pci/msix.c
 index 5c49bfc..c77ae7d 100644
 --- a/hw/pci/msix.c
 +++ b/hw/pci/msix.c
 @@ -437,6 +437,10 @@ void msix_notify(PCIDevice *dev, unsigned vector)
  return;
  }
  
 +if (!(pci_get_word(dev-config + PCI_COMMAND)  PCI_COMMAND_MASTER)) {
 +return;
 +}
 +
  msg = msix_get_message(dev, vector);
  
  stl_le_phys(address_space_memory, msg.address, msg.data);
 -- 
 1.8.1.1.298.ge7eed54





 
 




signature.asc
Description: OpenPGP digital signature


[Qemu-devel] [PATCH] pci: Don't deliver MSI/MSI-X messages if bus master support is off

2014-07-19 Thread Jan Kiszka
From: Jan Kiszka jan.kis...@siemens.com

The spec says (and real HW confirms this) that, if the bus master bit
is 0, the device will not generate any PCI accesses. MSI and MSI-X
messages fall among these.

Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---
 hw/pci/msi.c  | 4 
 hw/pci/msix.c | 4 
 2 files changed, 8 insertions(+)

diff --git a/hw/pci/msi.c b/hw/pci/msi.c
index a4a3040..36b651b 100644
--- a/hw/pci/msi.c
+++ b/hw/pci/msi.c
@@ -285,6 +285,10 @@ void msi_notify(PCIDevice *dev, unsigned int vector)
 return;
 }
 
+if (!(pci_get_word(dev-config + PCI_COMMAND)  PCI_COMMAND_MASTER)) {
+return;
+}
+
 msg = msi_get_message(dev, vector);
 
 MSI_DEV_PRINTF(dev,
diff --git a/hw/pci/msix.c b/hw/pci/msix.c
index 5c49bfc..c77ae7d 100644
--- a/hw/pci/msix.c
+++ b/hw/pci/msix.c
@@ -437,6 +437,10 @@ void msix_notify(PCIDevice *dev, unsigned vector)
 return;
 }
 
+if (!(pci_get_word(dev-config + PCI_COMMAND)  PCI_COMMAND_MASTER)) {
+return;
+}
+
 msg = msix_get_message(dev, vector);
 
 stl_le_phys(address_space_memory, msg.address, msg.data);
-- 
1.8.1.1.298.ge7eed54



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH] ahci: map memory via device's address space instead of address_space_memory

2014-07-03 Thread Jan Kiszka
On 2014-07-03 10:26, Le Tan wrote:
 In map_page() in hw/ide/ahci.c, replace cpu_physical_memory_map() and
 cpu_physical_memory_unmap() with dma_memory_map() and dma_memory_unmap(),
 because ahci devices should not access memory directly but via their address
 space. Add an AddressSpace parameter to map_page(). In order to call
 map_page(), we should pass the AHCIState.as as the AddressSpace argument.

BTW, when doing git grep cpu_physical_memory_map hw, there are some
more cases that should be checked (for x86). I suppose vhost is
incompatible with an IOMMU, but plain virtio should work, same for vmxnet.

Jan

 
 Signed-off-by: Le Tan tamlokv...@gmail.com
 ---
  hw/ide/ahci.c |   21 +++--
  1 file changed, 11 insertions(+), 10 deletions(-)
 
 diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
 index 9bae22e..7bb0a03 100644
 --- a/hw/ide/ahci.c
 +++ b/hw/ide/ahci.c
 @@ -175,17 +175,18 @@ static void ahci_trigger_irq(AHCIState *s, AHCIDevice 
 *d,
  ahci_check_irq(s);
  }
  
 -static void map_page(uint8_t **ptr, uint64_t addr, uint32_t wanted)
 +static void map_page(AddressSpace *as, uint8_t **ptr, uint64_t addr,
 + uint32_t wanted)
  {
  hwaddr len = wanted;
  
  if (*ptr) {
 -cpu_physical_memory_unmap(*ptr, len, 1, len);
 +dma_memory_unmap(as, *ptr, len, DMA_DIRECTION_FROM_DEVICE, len);
  }
  
 -*ptr = cpu_physical_memory_map(addr, len, 1);
 +*ptr = dma_memory_map(as, addr, len, DMA_DIRECTION_FROM_DEVICE);
  if (len  wanted) {
 -cpu_physical_memory_unmap(*ptr, len, 1, len);
 +dma_memory_unmap(as, *ptr, len, DMA_DIRECTION_FROM_DEVICE, len);
  *ptr = NULL;
  }
  }
 @@ -198,24 +199,24 @@ static void  ahci_port_write(AHCIState *s, int port, 
 int offset, uint32_t val)
  switch (offset) {
  case PORT_LST_ADDR:
  pr-lst_addr = val;
 -map_page(s-dev[port].lst,
 +map_page(s-as, s-dev[port].lst,
   ((uint64_t)pr-lst_addr_hi  32) | pr-lst_addr, 1024);
  s-dev[port].cur_cmd = NULL;
  break;
  case PORT_LST_ADDR_HI:
  pr-lst_addr_hi = val;
 -map_page(s-dev[port].lst,
 +map_page(s-as, s-dev[port].lst,
   ((uint64_t)pr-lst_addr_hi  32) | pr-lst_addr, 1024);
  s-dev[port].cur_cmd = NULL;
  break;
  case PORT_FIS_ADDR:
  pr-fis_addr = val;
 -map_page(s-dev[port].res_fis,
 +map_page(s-as, s-dev[port].res_fis,
   ((uint64_t)pr-fis_addr_hi  32) | pr-fis_addr, 256);
  break;
  case PORT_FIS_ADDR_HI:
  pr-fis_addr_hi = val;
 -map_page(s-dev[port].res_fis,
 +map_page(s-as, s-dev[port].res_fis,
   ((uint64_t)pr-fis_addr_hi  32) | pr-fis_addr, 256);
  break;
  case PORT_IRQ_STAT:
 @@ -1260,9 +1261,9 @@ static int ahci_state_post_load(void *opaque, int 
 version_id)
  ad = s-dev[i];
  AHCIPortRegs *pr = ad-port_regs;
  
 -map_page(ad-lst,
 +map_page(s-as, ad-lst,
   ((uint64_t)pr-lst_addr_hi  32) | pr-lst_addr, 1024);
 -map_page(ad-res_fis,
 +map_page(s-as, ad-res_fis,
   ((uint64_t)pr-fis_addr_hi  32) | pr-fis_addr, 256);
  /*
   * All pending i/o should be flushed out on a migrate. However,
 




signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH] ahci: map memory via device's address space instead of address_space_memory

2014-07-03 Thread Jan Kiszka
On 2014-07-03 12:02, Michael S. Tsirkin wrote:
 On Thu, Jul 03, 2014 at 10:43:57AM +0200, Jan Kiszka wrote:
 On 2014-07-03 10:26, Le Tan wrote:
 In map_page() in hw/ide/ahci.c, replace cpu_physical_memory_map() and
 cpu_physical_memory_unmap() with dma_memory_map() and dma_memory_unmap(),
 because ahci devices should not access memory directly but via their address
 space. Add an AddressSpace parameter to map_page(). In order to call
 map_page(), we should pass the AHCIState.as as the AddressSpace argument.

 BTW, when doing git grep cpu_physical_memory_map hw, there are some
 more cases that should be checked (for x86). I suppose vhost is
 incompatible with an IOMMU,
 
 vhost can be made to work: you just need to
 update its memory tables as appropriate.
 But see below
 
 but plain virtio should work,
 
 It doesn't: all guests pass in physical addresses at the moment.

You mean they do not put virtio devices into IOMMU domains, or they do
put them but ignore any translation rules that are not 1:1?

 We discussed requiring this for virtio 1.0, but in the end,
 most people thought that passing through virtio devices
 isn't worthwhile.

It should be consistent at least. If virtio is not translated, we have
to exclude such devices via ACPI tables from the scope of our IOMMUs.

 We can certainly add that as an option, with a feature bit.
 
 If you feel otherwise, you can comment on the latest spec draft.

Does the spec at least state that virtio devices are not subject to any
guest configured IOMMU translation? Is is this left undefined?

Jan




signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH] ahci: map memory via device's address space instead of address_space_memory

2014-07-03 Thread Jan Kiszka
On 2014-07-03 22:30, Michael S. Tsirkin wrote:
 On Thu, Jul 03, 2014 at 06:45:03PM +0200, Jan Kiszka wrote:
 On 2014-07-03 12:02, Michael S. Tsirkin wrote:
 On Thu, Jul 03, 2014 at 10:43:57AM +0200, Jan Kiszka wrote:
 On 2014-07-03 10:26, Le Tan wrote:
 In map_page() in hw/ide/ahci.c, replace cpu_physical_memory_map() and
 cpu_physical_memory_unmap() with dma_memory_map() and dma_memory_unmap(),
 because ahci devices should not access memory directly but via their 
 address
 space. Add an AddressSpace parameter to map_page(). In order to call
 map_page(), we should pass the AHCIState.as as the AddressSpace argument.

 BTW, when doing git grep cpu_physical_memory_map hw, there are some
 more cases that should be checked (for x86). I suppose vhost is
 incompatible with an IOMMU,

 vhost can be made to work: you just need to
 update its memory tables as appropriate.
 But see below

 but plain virtio should work,

 It doesn't: all guests pass in physical addresses at the moment.

 You mean they do not put virtio devices into IOMMU domains, or they do
 put them but ignore any translation rules that are not 1:1?
 
 Look at the code. We just pass in physical addresses
 ignoring which iommu domain device ended up with.

Should probably be fixed, at least in Linux. I suppose there is always a
1:1 domain where virtio devices can be put in so that they are not
involved in any translation if this is not desired (PPC?).

 
 We discussed requiring this for virtio 1.0, but in the end,
 most people thought that passing through virtio devices
 isn't worthwhile.

 It should be consistent at least. If virtio is not translated, we have
 to exclude such devices via ACPI tables from the scope of our IOMMUs.
 
 I didn't know this is possible. How does one do this?

Both the DMAR (Intel) and the IVRS (AMD) ACPI tables allow to report the
scope of an IOMMU unit. There you list sets of devices on specific buses
or a complete segment that the unit virtualizes. Le currently reports
that there is a single DMAR unit for q35, and that one controls the
complete segment 0. The structure could be adapted if a virtio device is
present so that this device is not included. That should prevent that
the guest tries to control virtio devices via IOMMUs or even assign them
to some L2 guests.

We could then provide a machine property that controls virtualization of
virtio devices, default off, but you can enable it when running properly
adjusted guest drivers.

 
 We can certainly add that as an option, with a feature bit.

 If you feel otherwise, you can comment on the latest spec draft.

 Does the spec at least state that virtio devices are not subject to any
 guest configured IOMMU translation? Is is this left undefined?

 Jan


 
 I don't think we have anything like this.
 

And I'm not even sure it necessarily belongs to the spec. I think it's
just a guest driver deficit to ignore IOMMU settings that the guest OS
may apply.

Jan



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] Why devfn will be -1

2014-07-01 Thread Jan Kiszka
Hi Le,

On 2014-07-01 04:34, Le Tan wrote:
 Hi Jan,
 I use pci_setup_iommu() to setup a PCIIOMMUFunc for the q35 pci bus.
 In the iommu_fn, I print out the devfn parameter and find out that it
 sometimes will be -1. So what does it mean?
 The detail code is here:
 
 In mch_init() function, I write like this:
 PCIBus *pci_bus = PCI_BUS(qdev_get_parent_bus(DEVICE(mch)));
 pci_setup_iommu(pci_bus, q35_host_dma_iommu, mch-iommu);
 
 And in q35_host_dma_iommu(PCIBus *bus, void *opaque, int devfn), I
 print out the devfn parameter, sometimes it will be -1.

Hmm, I have no idea about the reason and would suggest to set a
conditional breakpoint on this function, then print the backtrace to see
where this comes from and analyze the device structure from where that
-1 was most probably taken.

Jan




signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PULL 2/4] net: L2TPv3 transport

2014-07-01 Thread Jan Kiszka
On 2014-06-27 16:24, Stefan Hajnoczi wrote:
 From: Anton Ivanov antiv...@cisco.com
 
 This transport allows to connect a QEMU nic to a static Ethernet
 over L2TPv3 tunnel. The transport supports all options present
 in the Linux kernel implementation. It allows QEMU to connect
 to any Linux host running kernel 3.3+, most routers and network
 devices as well as other QEMU instances.
 
 [Fixed up net_client_init1() switch statement to support -netdev
 --Stefan]
 
 Signed-off-by: Anton Ivanov antiv...@cisco.com
 Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
 ---
  net/Makefile.objs |   1 +
  net/clients.h |   2 +
  net/l2tpv3.c  | 757 
 ++
  net/net.c |   6 +
  qapi-schema.json  |  60 +
  qemu-options.hx   |  82 ++
  6 files changed, 908 insertions(+)
  create mode 100644 net/l2tpv3.c
 
 diff --git a/net/Makefile.objs b/net/Makefile.objs
 index 301f6b6..a06ba59 100644
 --- a/net/Makefile.objs
 +++ b/net/Makefile.objs
 @@ -2,6 +2,7 @@ common-obj-y = net.o queue.o checksum.o util.o hub.o
  common-obj-y += socket.o
  common-obj-y += dump.o
  common-obj-y += eth.o
 +common-obj-$(CONFIG_LINUX) += l2tpv3.o
  common-obj-$(CONFIG_POSIX) += tap.o vhost-user.o
  common-obj-$(CONFIG_LINUX) += tap-linux.o
  common-obj-$(CONFIG_WIN32) += tap-win32.o
 diff --git a/net/clients.h b/net/clients.h
 index 7f3d4ae..2e8feda 100644
 --- a/net/clients.h
 +++ b/net/clients.h
 @@ -47,6 +47,8 @@ int net_init_tap(const NetClientOptions *opts, const char 
 *name,
  int net_init_bridge(const NetClientOptions *opts, const char *name,
  NetClientState *peer);
  
 +int net_init_l2tpv3(const NetClientOptions *opts, const char *name,
 +NetClientState *peer);
  #ifdef CONFIG_VDE
  int net_init_vde(const NetClientOptions *opts, const char *name,
   NetClientState *peer);
 diff --git a/net/l2tpv3.c b/net/l2tpv3.c
 new file mode 100644
 index 000..528d95b
 --- /dev/null
 +++ b/net/l2tpv3.c
 @@ -0,0 +1,757 @@
 +/*
 + * QEMU System Emulator
 + *
 + * Copyright (c) 2003-2008 Fabrice Bellard
 + * Copyright (c) 2012-2014 Cisco Systems
 + *
 + * 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 linux/ip.h
 +#include netdb.h
 +#include config-host.h
 +#include net/net.h
 +#include clients.h
 +#include monitor/monitor.h
 +#include qemu-common.h
 +#include qemu/error-report.h
 +#include qemu/option.h
 +#include qemu/sockets.h
 +#include qemu/iov.h
 +#include qemu/main-loop.h
 +
 +
 +/* The buffer size needs to be investigated for optimum numbers and
 + * optimum means of paging in on different systems. This size is
 + * chosen to be sufficient to accommodate one packet with some headers
 + */
 +
 +#define BUFFER_ALIGN sysconf(_SC_PAGESIZE)
 +#define BUFFER_SIZE 2048
 +#define IOVSIZE 2
 +#define MAX_L2TPV3_MSGCNT 64
 +#define MAX_L2TPV3_IOVCNT (MAX_L2TPV3_MSGCNT * IOVSIZE)
 +
 +/* Header set to 0x3 signifies a data packet */
 +
 +#define L2TPV3_DATA_PACKET 0x3
 +
 +/* IANA-assigned IP protocol ID for L2TPv3 */
 +
 +#ifndef IPPROTO_L2TP
 +#define IPPROTO_L2TP 0x73
 +#endif
 +
 +typedef struct NetL2TPV3State {
 +NetClientState nc;
 +int fd;
 +
 +/*
 + * these are used for xmit - that happens packet a time
 + * and for first sign of life packet (easier to parse that once)
 + */
 +
 +uint8_t *header_buf;
 +struct iovec *vec;
 +
 +/*
 + * these are used for receive - try to eat up to 32 packets at a time
 + */
 +
 +struct mmsghdr *msgvec;

struct mmsghdr is only available with recent distro kernel headers.
Please check if it's there and otherwise disable L2TPv3 (or provide the
struct yourself).

Thanks,
Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] Why devfn will be -1

2014-07-01 Thread Jan Kiszka
On 2014-07-01 14:55, Le Tan wrote:
 2014-07-01 20:52 GMT+08:00 Le Tan tamlokv...@gmail.com:
 Hi Jan,

 2014-07-01 15:34 GMT+08:00 Jan Kiszka jan.kis...@web.de:
 Hi Le,

 On 2014-07-01 04:34, Le Tan wrote:
 Hi Jan,
 I use pci_setup_iommu() to setup a PCIIOMMUFunc for the q35 pci bus.
 In the iommu_fn, I print out the devfn parameter and find out that it
 sometimes will be -1. So what does it mean?
 The detail code is here:

 In mch_init() function, I write like this:
 PCIBus *pci_bus = PCI_BUS(qdev_get_parent_bus(DEVICE(mch)));
 pci_setup_iommu(pci_bus, q35_host_dma_iommu, mch-iommu);

 And in q35_host_dma_iommu(PCIBus *bus, void *opaque, int devfn), I
 print out the devfn parameter, sometimes it will be -1.

 Hmm, I have no idea about the reason and would suggest to set a
 conditional breakpoint on this function, then print the backtrace to see
 where this comes from and analyze the device structure from where that
 -1 was most probably taken.
 
 I think maybe this is a bug? In the function do_pci_register_device(),
 maybe these two sentence should be reorder?
 dma_as = pci_device_iommu_address_space(pci_dev);
 pci_dev-devfn = devfn;

Looks like. Give it a try, then possibly send a patch :)

Jan



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] Why devfn will be -1

2014-07-01 Thread Jan Kiszka
On 2014-07-01 15:02, Le Tan wrote:
 2014-07-01 20:56 GMT+08:00 Jan Kiszka jan.kis...@web.de:
 On 2014-07-01 14:55, Le Tan wrote:
 2014-07-01 20:52 GMT+08:00 Le Tan tamlokv...@gmail.com:
 Hi Jan,

 2014-07-01 15:34 GMT+08:00 Jan Kiszka jan.kis...@web.de:
 Hi Le,

 On 2014-07-01 04:34, Le Tan wrote:
 Hi Jan,
 I use pci_setup_iommu() to setup a PCIIOMMUFunc for the q35 pci bus.
 In the iommu_fn, I print out the devfn parameter and find out that it
 sometimes will be -1. So what does it mean?
 The detail code is here:

 In mch_init() function, I write like this:
 PCIBus *pci_bus = PCI_BUS(qdev_get_parent_bus(DEVICE(mch)));
 pci_setup_iommu(pci_bus, q35_host_dma_iommu, mch-iommu);

 And in q35_host_dma_iommu(PCIBus *bus, void *opaque, int devfn), I
 print out the devfn parameter, sometimes it will be -1.

 Hmm, I have no idea about the reason and would suggest to set a
 conditional breakpoint on this function, then print the backtrace to see
 where this comes from and analyze the device structure from where that
 -1 was most probably taken.

 I think maybe this is a bug? In the function do_pci_register_device(),
 maybe these two sentence should be reorder?
 dma_as = pci_device_iommu_address_space(pci_dev);
 pci_dev-devfn = devfn;

 Looks like. Give it a try, then possibly send a patch :)
 I reorder these two sentences and get the print log like this:
 vtd bus 0 slot 31 func 0 devfn 248
 vtd bus 0 slot 31 func 2 devfn 250
 vtd bus 0 slot 31 func 3 devfn 251
 vtd bus 0 slot 1 func 0 devfn 8
 vtd bus 0 slot 2 func 0 devfn 16
 
 The info pci output is here:
 (qemu) info pci
   Bus  0, device   0, function 0:
 Host bridge: PCI device 8086:29c0
   id 
   Bus  0, device   1, function 0:
 VGA controller: PCI device 1013:00b8
   BAR0: 32 bit prefetchable memory at 0xfc00 [0xfdff].
   BAR1: 32 bit memory at 0xfebf [0xfebf0fff].
   BAR6: 32 bit memory at 0x [0xfffe].
   id 
   Bus  0, device   2, function 0:
 Ethernet controller: PCI device 8086:100e
   IRQ 11.
   BAR0: 32 bit memory at 0xfebc [0xfebd].
   BAR1: I/O at 0xc000 [0xc03f].
   BAR6: 32 bit memory at 0x [0x0003fffe].
   id 
   Bus  0, device  31, function 0:
 ISA bridge: PCI device 8086:2918
   id 
   Bus  0, device  31, function 2:
 SATA controller: PCI device 8086:2922
   IRQ 10.
   BAR4: I/O at 0xc080 [0xc09f].
   BAR5: 32 bit memory at 0xfebf1000 [0xfebf1fff].
   id 
   Bus  0, device  31, function 3:
 SMBus: PCI device 8086:2930
   IRQ 10.
   BAR4: I/O at 0x0700 [0x073f].
   id 
 
 So maybe it is all right now? And I will go on the vtd emulation.:)
 Thanks very much!

Looks better :). Don't forget to send the patch against pci.c!

Jan




signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] About AddressSpace in intel-iommu emulation

2014-06-27 Thread Jan Kiszka
On 2014-06-27 07:46, Le Tan wrote:
 2014-06-27 12:55 GMT+08:00 Paolo Bonzini pbonz...@redhat.com:
 Il 27/06/2014 04:08, Le Tan ha scritto:

 1. In struct IOMMUTLBEntry, I think the addr_mask field should be the
 mask of the page offset, right? But I see different usages of this
 field. In spapr_tce_translate_iommu(), the addr_mask field is assigned
 with the mask of the page offset. However, in pbm_translate_iommu(),
 in the passthrough case, the addr_mask field seems to be assigned the
 mask of the page number. Is there any problem here?


 The intended usage is the one of spapr_tce_translate_iommu().  In practice
 it doesn't matter, both work.


 2. For q35, how to identify origination of DMA requests? The VT-d
 manual says we should use source-id(for PCI-Express devices, it is
 requester identifier) to map devices to domains. What is the related
 part in QEMU? Where can I get the source-id of a DMA request?


 You need to create a different AddressSpace for each PCI bus or device.
 
 How to create a different AddressSpace for each device? I thought a
 AddressSpace just belongs to a PCI bus before. The paging structures
 for different functions of the same device can also be different, too.
 So maybe we should create a different AddressSpace for each function?
 How to achieve it? Could you give me some more hints or is there any
 existing example in QEMU?

I would suggest to study the apb IOMMU implementation Paolo referenced
and the PCI layer functions used by that code. Specifically,
pci_setup_iommu takes a callback that is supposed to return an address
space to be used for a particular device. For apb, it's the same for all
devices on a bus, but that's not required...

Jan




signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] Using virtio for inter-VM communication

2014-06-17 Thread Jan Kiszka
On 2014-06-17 07:24, Paolo Bonzini wrote:
 Il 15/06/2014 08:20, Jan Kiszka ha scritto:
  I think implementing Xen hypercalls in jailhouse for grant table and
  event channels would actually make a lot of sense.  The Xen
  implementation is 2.5kLOC and I think it should be possible to compact
  it noticeably, especially if you limit yourself to 64-bit guests.
 At least the grant table model seems unsuited for Jailhouse. It allows a
 guest to influence the mapping of another guest during runtime. This we
 want (or even have) to avoid in Jailhouse.
 
 IIRC implementing the grant table hypercalls with copies is inefficient
 but valid.

Back to #1: This is what Rusty is suggesting for virtio. Nothing to win
with grant tables then. And if we really have to copy, I would prefer to
use a standard.

I guess we need to play with prototypes to assess feasibility and impact
on existing code.

Jan




signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] Using virtio for inter-VM communication

2014-06-15 Thread Jan Kiszka
On 2014-06-13 10:45, Paolo Bonzini wrote:
 Il 13/06/2014 08:23, Jan Kiszka ha scritto:
 That would preserve zero-copy capabilities (as long as you can work
 against the shared mem directly, e.g. doing DMA from a physical NIC or
 storage device into it) and keep the hypervisor out of the loop.
 
  This seems ill thought out.  How will you program a NIC via the virtio
  protocol without a hypervisor?  And how will you make it safe?  You'll
  need an IOMMU.  But if you have an IOMMU you don't need shared memory.

 Scenarios behind this are things like driver VMs: You pass through the
 physical hardware to a driver guest that talks to the hardware and
 relays data via one or more virtual channels to other VMs. This confines
 a certain set of security and stability risks to the driver VM.
 
 I think implementing Xen hypercalls in jailhouse for grant table and
 event channels would actually make a lot of sense.  The Xen
 implementation is 2.5kLOC and I think it should be possible to compact
 it noticeably, especially if you limit yourself to 64-bit guests.

At least the grant table model seems unsuited for Jailhouse. It allows a
guest to influence the mapping of another guest during runtime. This we
want (or even have) to avoid in Jailhouse.

I'm therefore more in favor of a model where the shared memory region is
defined on cell (guest) creation by adding a virtual device that comes
with such a region.

Jan

 
 It should also be almost enough to run Xen PVH guests as jailhouse
 partitions.
 
 If later Xen starts to support virtio, you will get that for free.
 
 Paolo




signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] Using virtio for inter-VM communication

2014-06-13 Thread Jan Kiszka
On 2014-06-13 02:47, Rusty Russell wrote:
 Jan Kiszka jan.kis...@siemens.com writes:
 On 2014-06-12 04:27, Rusty Russell wrote:
 Henning Schild henning.sch...@siemens.com writes:
 It was also never implemented, and remains a thought experiment.
 However, implementing it in lguest should be fairly easy.

 The reason why a trusted helper, i.e. additional logic in the
 hypervisor, is not our favorite solution is that we'd like to keep the
 hypervisor as small as possible. I wouldn't exclude such an approach
 categorically, but we have to weigh the costs (lines of code, additional
 hypervisor interface) carefully against the gain (existing
 specifications and guest driver infrastructure).
 
 Reasonable, but I think you'll find it is about the minimal
 implementation in practice.  Unfortunately, I don't have time during the
 next 6 months to implement it myself :(
 
 Back to VIRTIO_F_RING_SHMEM_ADDR (which you once brought up in an MCA
 working group discussion): What speaks against introducing an
 alternative encoding of addresses inside virtio data structures? The
 idea of this flag was to replace guest-physical addresses with offsets
 into a shared memory region associated with or part of a virtio
 device.
 
 We would also need a way of defining the shared memory region.  But
 that's not the problem.  If such a feature is not accepted by the guest?
 How to you fall back?

Depends on the hypervisor and its scope, but it should be quite
straightforward: full-featured ones like KVM could fall back to slow
copying, specialized ones like Jailhouse would clear FEATURES_OK if the
guest driver does not accept it (because there would be no ring walking
or copying code in Jailhouse), thus refuse the activate the device. That
would be absolutely fine for application domains of specialized
hypervisors (often embedded, customized guests etc.).

The shared memory regions could be exposed as a BARs (PCI) or additional
address ranges (device tree) and addressed in the redefined guest
address fields via some region index and offset.

 
 We don't add features which unmake the standard.
 
 That would preserve zero-copy capabilities (as long as you can work
 against the shared mem directly, e.g. doing DMA from a physical NIC or
 storage device into it) and keep the hypervisor out of the loop.
 
 This seems ill thought out.  How will you program a NIC via the virtio
 protocol without a hypervisor?  And how will you make it safe?  You'll
 need an IOMMU.  But if you have an IOMMU you don't need shared memory.

Scenarios behind this are things like driver VMs: You pass through the
physical hardware to a driver guest that talks to the hardware and
relays data via one or more virtual channels to other VMs. This confines
a certain set of security and stability risks to the driver VM.

 
 Is it
 too invasive to existing infrastructure or does it have some other pitfalls?
 
 You'll have to convince every vendor to implement your addition to the
 standard.  Which is easier than inventing a completely new system, but
 it's not quite virtio.

It would be an optional addition, a feature all three sides (host and
the communicating guests) would have to agree on. I think we would only
have to agree on extending the spec to enable this - after demonstrating
it via an implementation, of course.

Thanks,
Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] Using virtio for inter-VM communication

2014-06-11 Thread Jan Kiszka
On 2014-06-12 04:27, Rusty Russell wrote:
 Henning Schild henning.sch...@siemens.com writes:
 Hi,

 i am working on the jailhouse[1] project and am currently looking at
 inter-VM communication. We want to connect guests directly with virtual
 consoles based on shared memory. The code complexity in the hypervisor
 should be minimal, it should just make the shared memory discoverable
 and provide a signaling mechanism.
 
 Hi Henning,
 
 The virtio assumption was that the host can see all of guest
 memory.  This simplifies things significantly, and makes it efficient.
 
 If you don't have this, *someone* needs to do a copy.  Usually the guest
 OS does a bounce buffer into your shared region.  Goodbye performance.
 Or you can play remapping tricks.  Goodbye performance again.
 
 My preferred model is to have a trusted helper (ie. host) which
 understands how to copy between virtio rings.  The backend guest (to
 steal Xen vocab) R/O maps the descriptor, avail ring and used rings in
 the guest.  It then asks the trusted helper to do various operation
 (copy into writable descriptor, copy out of readable descriptor, mark
 used).  The virtio ring itself acts as a grant table.
 
 Note: that helper mechanism is completely protocol agnostic.  It was
 also explicitly designed into the virtio mechanism (with its 4k
 boundaries for data structures and its 'len' field to indicate how much
 was written into the descriptor). 
 
 It was also never implemented, and remains a thought experiment.
 However, implementing it in lguest should be fairly easy.

The reason why a trusted helper, i.e. additional logic in the
hypervisor, is not our favorite solution is that we'd like to keep the
hypervisor as small as possible. I wouldn't exclude such an approach
categorically, but we have to weigh the costs (lines of code, additional
hypervisor interface) carefully against the gain (existing
specifications and guest driver infrastructure).

Back to VIRTIO_F_RING_SHMEM_ADDR (which you once brought up in an MCA
working group discussion): What speaks against introducing an
alternative encoding of addresses inside virtio data structures? The
idea of this flag was to replace guest-physical addresses with offsets
into a shared memory region associated with or part of a virtio device.
That would preserve zero-copy capabilities (as long as you can work
against the shared mem directly, e.g. doing DMA from a physical NIC or
storage device into it) and keep the hypervisor out of the loop. Is it
too invasive to existing infrastructure or does it have some other pitfalls?

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] [PATCH] slirp: goto bad in udp_input if sosendto fails

2014-06-11 Thread Jan Kiszka
On 2014-06-11 10:55, Samuel Thibault wrote:
 Before this patch, if sosendto fails, udp_input is executed as if the
 packet was sent. This could cause memory leak.

Cannot follow yet how this could leak (not saying I fully got what it
should NOT leak - nasty code). Can you elaborate on the before/after?

Jan

 This patch adds a goto bad to cut the execution of this function.
 
 Signed-off-by: Guillaume Subiron maet...@subiron.org
 ---
  slirp/udp.c | 1 +
  1 file changed, 1 insertion(+)
 
 diff --git a/slirp/udp.c b/slirp/udp.c
 index 8cc6cb6..fd2446a 100644
 --- a/slirp/udp.c
 +++ b/slirp/udp.c
 @@ -218,6 +218,7 @@ udp_input(register struct mbuf *m, int iphlen)
 *ip=save_ip;
 DEBUG_MISC((dfd,udp tx errno = %d-%s\n,errno,strerror(errno)));
 icmp_error(m, ICMP_UNREACH,ICMP_UNREACH_NET, 0,strerror(errno));
 +   goto bad;
   }
  
   m_free(so-so_m);   /* used for ICMP if error on sorecvfrom */
 

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] [PATCHv2, DoS] slirp (arp): do not special-case bogus IP addresses

2014-06-11 Thread Jan Kiszka
On 2014-05-28 01:10, Edgar E. Iglesias wrote:
 On Wed, May 14, 2014 at 01:22:25AM +, Edgar E. Iglesias wrote:
 On Wed, May 14, 2014 at 03:13:09AM +0200, Samuel Thibault wrote:
 Do not special-case addresses with zero host part, as we do not
 necessarily know how big it is, and the guest can fake them anyway.
 Silently avoid having 0.0.0.0 as a destination, however.

 Signed-off-by: Samuel Thibault samuel.thiba...@ens-lyon.org

 Reviewed-by: Edgar E. Iglesias edgar.igles...@xilinx.com
 
 Ping

Thanks for merging!

Jan

 
 
 

 ---

 This is particularly bad actually, one can for instance simply do this
 inside a Linux guest

 ip addr add 192.0.0.0/1 dev eth0

 and crash qemu (thus a DoS) by just emitting a packet (thus from
 192.0.0.0), getting:

 qemu-system-x86_64: /usr/src/qemu/slirp/arp_table.c:77: arp_table_search: 
 Assertion `(ip_addr  __bswap_32 (~(0xfU  28))) != 0' failed.

 so it should probably go to all stable maintained versions.

 diff --git a/slirp/arp_table.c b/slirp/arp_table.c
 index ecdb0ba..bcaeb44 100644
 --- a/slirp/arp_table.c
 +++ b/slirp/arp_table.c
 @@ -37,12 +37,7 @@ void arp_table_add(Slirp *slirp, uint32_t ip_addr, 
 uint8_t ethaddr[ETH_ALEN])
  ethaddr[0], ethaddr[1], ethaddr[2],
  ethaddr[3], ethaddr[4], ethaddr[5]));
  
 -/* Check 0.0.0.0/8 invalid source-only addresses */
 -if ((ip_addr  htonl(~(0xfU  28))) == 0) {
 -return;
 -}
 -
 -if (ip_addr == 0x || ip_addr == broadcast_addr) {
 +if (ip_addr == 0 || ip_addr == 0x || ip_addr == 
 broadcast_addr) {
  /* Do not register broadcast addresses */
  return;
  }
 @@ -73,9 +68,6 @@ bool arp_table_search(Slirp *slirp, uint32_t ip_addr,
  DEBUG_CALL(arp_table_search);
  DEBUG_ARG(ip = 0x%x, ip_addr);
  
 -/* Check 0.0.0.0/8 invalid source-only addresses */
 -assert((ip_addr  htonl(~(0xfU  28))) != 0);
 -
  /* If broadcast address */
  if (ip_addr == 0x || ip_addr == broadcast_addr) {
  /* return Ethernet broadcast address */
 diff --git a/slirp/slirp.c b/slirp/slirp.c
 index 3fb48a4..00f4eb5 100644
 --- a/slirp/slirp.c
 +++ b/slirp/slirp.c
 @@ -778,6 +778,11 @@ int if_encap(Slirp *slirp, struct mbuf *ifm)
  return 1;
  }
  
 +if (iph-ip_dst.s_addr == 0) {
 +/* 0.0.0.0 can not be a destination address, something went wrong,
 + * avoid making it worse */
 +return 1;
 +}
  if (!arp_table_search(slirp, iph-ip_dst.s_addr, ethaddr)) {
  uint8_t arp_req[ETH_HLEN + sizeof(struct arphdr)];
  struct ethhdr *reh = (struct ethhdr *)arp_req;


-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] [PATCH v3] block: replace fprintf(stderr, ...) with error_report()

2014-05-25 Thread Jan Kiszka
On 2014-05-25 10:44, Le Tan wrote:
 Replace fprintf(stderr,...) with error_report() in files block/*, block.c,
 block-migration.c and blockdev.c. The trailing \ns of the @fmt argument
 have been removed because @fmt of error_report() should not contain newline.
 Also fix some coding style issues.

Let's do the also part in a separate patch. It's more than one or two
trivial pass-by style fixes, and some people may like the fprintf
conversion while having different views on the other changes.

Jan




signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH 0/4] Support more than 255 cpus

2014-05-15 Thread Jan Kiszka
On 2014-05-15 09:16, Li, ZhenHua wrote:
 Kernel's kvm support is not here.
 x2APIC is needed, I will try to do that later.

x2APIC emulation can wait if KVM support is there. But we need at least
one of them before starting to think about raising the limit.

Jan

PS: Please don't top-post.

 
 On 05/13/2014 06:53 PM, Jan Kiszka wrote:
 On 2014-05-13 09:09, Li, Zhen-Hua wrote:
 From: Li, ZhenHua zhen-h...@hp.com

 These series patches are trying to make Qemu support more than 255 CPUs.
 The max cpu number changed to 4096.

   Support more than 255 cpus: ACPI and APIC defines
   Support more than 255 cpus: max_cpus to 4096
   Support more than 255 cpus: max cpumask bit to 4096
   Support more than 255 cpus: runtime chec

   include/hw/acpi/cpu_hotplug_defs.h | 4 ++--
   include/hw/i386/apic_internal.h| 2 +-
   include/hw/i386/pc.h | 2 +-
   include/sysemu/sysemu.h | 2 +-
   hw/i386/acpi-build.c | 8 
 Don't we need x2APIC support to provide 255 CPUs? Where so you enforce
 this, i.e. keep the restriction to 255 CPUs when we are not in KVM mode
 with in-kernel APIC (the emulate APIC lacks x2APIC mode, unfortunately)?
 But, wait, KVM only supports up to 255 VCPUs. So what are you
 targeting at?

 Jan

 

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] [PATCH 0/4] Support more than 255 cpus

2014-05-13 Thread Jan Kiszka
On 2014-05-13 09:09, Li, Zhen-Hua wrote:
 From: Li, ZhenHua zhen-h...@hp.com
 
 These series patches are trying to make Qemu support more than 255 CPUs. 
 The max cpu number changed to 4096.
 
  Support more than 255 cpus: ACPI and APIC defines
  Support more than 255 cpus: max_cpus to 4096
  Support more than 255 cpus: max cpumask bit to 4096
  Support more than 255 cpus: runtime chec
 
  include/hw/acpi/cpu_hotplug_defs.h | 4 ++--
  include/hw/i386/apic_internal.h| 2 +-
  include/hw/i386/pc.h | 2 +-
  include/sysemu/sysemu.h | 2 +-
  hw/i386/acpi-build.c | 8 

Don't we need x2APIC support to provide 255 CPUs? Where so you enforce
this, i.e. keep the restriction to 255 CPUs when we are not in KVM mode
with in-kernel APIC (the emulate APIC lacks x2APIC mode, unfortunately)?
But, wait, KVM only supports up to 255 VCPUs. So what are you targeting at?

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] [PULL 7/8] gtk: Fix -serial vc

2014-05-06 Thread Jan Kiszka
On 2014-05-05 16:46, Gerd Hoffmann wrote:
 On Mo, 2014-05-05 at 13:55 +0200, Gerd Hoffmann wrote:
   Hi,

 Looks better thanks to gtk: zap scrolled_window (monitor is properly
 formatted again). But the whole queue spits this out on the terminal:

 (unknown:13169): Gtk-CRITICAL **: IA__gtk_window_resize: assertion `width 
  0' failed

 (unknown:13169): Gdk-CRITICAL **: IA__gdk_window_set_cursor: assertion 
 `GDK_IS_WINDOW (window)' failed

 Yes, there is some work-in-progress stuff in that branch causing this.
 
 Feel free to check out the branch again.  Warnings are fixed, resizing
 is fixed, and you can move tabs to windows (so you can see vga + monitor
 at the same time).

Still issues remaining. Scenario: Fired up a Linux guest up to graphical
mode, issued hard reset from monitor console, switched to guest display
- window is larger than boot loader screen.

Jan




signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH] block: Fix open flags with BDRV_O_SNAPSHOT

2014-05-06 Thread Jan Kiszka
On 2014-05-06 12:19, Kevin Wolf wrote:
 The immediately visible effect of this patch is that it fixes committing
 a temporary snapshot to its backing file. Previously, it would fail with
 a permission denied error because bdrv_inherited_flags() forced the
 backing file to be read-only, ignoring the r/w reopen of bdrv_commit().
 
 The bigger problem this releaved is that the original open flags must
 actually only be applied to the temporary snapshot, and the original
 image file must be treated as a backing file of the temporary snapshot
 and get the right flags for that.
 
 Reported-by: Jan Kiszka jan.kis...@web.de
 Signed-off-by: Kevin Wolf kw...@redhat.com
 ---
  block.c| 34 +++---
  include/block/block.h  |  2 +-
  tests/qemu-iotests/051 |  4 
  tests/qemu-iotests/051.out | 10 ++
  4 files changed, 34 insertions(+), 16 deletions(-)
 
 diff --git a/block.c b/block.c
 index b749d31..c90c71a 100644
 --- a/block.c
 +++ b/block.c
 @@ -775,6 +775,16 @@ void bdrv_disable_copy_on_read(BlockDriverState *bs)
  }
  
  /*
 + * Returns the flags that a temporary snapshot should get, based on the
 + * originally requested flags (the originally requested image will have flags
 + * like a backing file)
 + */
 +static int bdrv_temp_snapshot_flags(int flags)
 +{
 +return (flags  ~BDRV_O_SNAPSHOT) | BDRV_O_TEMPORARY;
 +}
 +
 +/*
   * Returns the flags that bs-file should get, based on the given flags for
   * the parent BDS
   */
 @@ -787,11 +797,6 @@ static int bdrv_inherited_flags(int flags)
   * so we can enable both unconditionally on lower layers. */
  flags |= BDRV_O_CACHE_WB | BDRV_O_UNMAP;
  
 -/* The backing file of a temporary snapshot is read-only */
 -if (flags  BDRV_O_SNAPSHOT) {
 -flags = ~BDRV_O_RDWR;
 -}
 -
  /* Clear flags that only apply to the top layer */
  flags = ~(BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING | BDRV_O_COPY_ON_READ);
  
 @@ -817,11 +822,6 @@ static int bdrv_open_flags(BlockDriverState *bs, int 
 flags)
  {
  int open_flags = flags | BDRV_O_CACHE_WB;
  
 -/* The backing file of a temporary snapshot is read-only */
 -if (flags  BDRV_O_SNAPSHOT) {
 -open_flags = ~BDRV_O_RDWR;
 -}
 -
  /*
   * Clear flags that are internal to the block layer before opening the
   * image.
 @@ -1206,7 +1206,7 @@ done:
  return ret;
  }
  
 -void bdrv_append_temp_snapshot(BlockDriverState *bs, Error **errp)
 +void bdrv_append_temp_snapshot(BlockDriverState *bs, int flags, Error **errp)
  {
  /* TODO: extra byte is a hack to ensure MAX_PATH space on Windows. */
  char *tmp_filename = g_malloc0(PATH_MAX + 1);
 @@ -1262,8 +1262,7 @@ void bdrv_append_temp_snapshot(BlockDriverState *bs, 
 Error **errp)
  bs_snapshot = bdrv_new(, error_abort);
  
  ret = bdrv_open(bs_snapshot, NULL, NULL, snapshot_options,
 -(bs-open_flags  ~BDRV_O_SNAPSHOT) | BDRV_O_TEMPORARY,
 -bdrv_qcow2, local_err);
 +flags, bdrv_qcow2, local_err);
  if (ret  0) {
  error_propagate(errp, local_err);
  goto out;
 @@ -1298,6 +1297,7 @@ int bdrv_open(BlockDriverState **pbs, const char 
 *filename,
  BlockDriverState *file = NULL, *bs;
  const char *drvname;
  Error *local_err = NULL;
 +int snapshot_flags = 0;
  
  assert(pbs);
  
 @@ -1358,6 +1358,10 @@ int bdrv_open(BlockDriverState **pbs, const char 
 *filename,
  if (flags  BDRV_O_RDWR) {
  flags |= BDRV_O_ALLOW_RDWR;
  }
 +if (flags  BDRV_O_SNAPSHOT) {
 +snapshot_flags = bdrv_temp_snapshot_flags(flags);
 +flags = bdrv_backing_flags(flags);
 +}
  
  assert(file == NULL);
  ret = bdrv_open_image(file, filename, options, file,
 @@ -1417,8 +1421,8 @@ int bdrv_open(BlockDriverState **pbs, const char 
 *filename,
  
  /* For snapshot=on, create a temporary qcow2 overlay. bs points to the
   * temporary snapshot afterwards. */
 -if (flags  BDRV_O_SNAPSHOT) {
 -bdrv_append_temp_snapshot(bs, local_err);
 +if (snapshot_flags) {
 +bdrv_append_temp_snapshot(bs, snapshot_flags, local_err);
  if (local_err) {
  error_propagate(errp, local_err);
  goto close_and_fail;
 diff --git a/include/block/block.h b/include/block/block.h
 index 467fb2b..2fda81c 100644
 --- a/include/block/block.h
 +++ b/include/block/block.h
 @@ -191,7 +191,7 @@ int bdrv_open_image(BlockDriverState **pbs, const char 
 *filename,
  QDict *options, const char *bdref_key, int flags,
  bool allow_none, Error **errp);
  int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error 
 **errp);
 -void bdrv_append_temp_snapshot(BlockDriverState *bs, Error **errp);
 +void bdrv_append_temp_snapshot(BlockDriverState *bs, int flags, Error 
 **errp);
  int bdrv_open(BlockDriverState **pbs, const char *filename,
const char

Re: [Qemu-devel] [PULL 7/8] gtk: Fix -serial vc

2014-05-04 Thread Jan Kiszka
On 2014-04-29 11:46, Gerd Hoffmann wrote:
 From: Cole Robinson crobi...@redhat.com
 
 Try kicking off a rhel5 text install over serial, the text menu navigation
 is all messed up, and some of the kernel boot messages are randomly
 corrupted.
 
 Drop use of a pty and just use vte infrastructure for reading and writing.
 This fixes the above corruption, and is simpler to boot.
 
 (I don't know what was wrong with the original code though. FWIW this is
 what virt-manager has done for years).
 
 Signed-off-by: Cole Robinson crobi...@redhat.com
 Signed-off-by: Gerd Hoffmann kra...@redhat.com
 ---
  ui/gtk.c | 41 +
  1 file changed, 9 insertions(+), 32 deletions(-)
 
 diff --git a/ui/gtk.c b/ui/gtk.c
 index c85aea3..1465a38 100644
 --- a/ui/gtk.c
 +++ b/ui/gtk.c
 @@ -115,7 +115,6 @@ typedef struct VirtualConsole
  GtkWidget *scrolled_window;
  CharDriverState *chr;
  #endif
 -int fd;
  } VirtualConsole;
  
  typedef struct GtkDisplayState
 @@ -1162,9 +1161,12 @@ static gboolean gd_focus_out_event(GtkWidget *widget,
  
  static int gd_vc_chr_write(CharDriverState *chr, const uint8_t *buf, int len)
  {
 +#if defined(CONFIG_VTE)
  VirtualConsole *vc = chr-opaque;
  
 -return vc ? write(vc-fd, buf, len) : len;
 +vte_terminal_feed(VTE_TERMINAL(vc-terminal), (const char *)buf, len);
 +#endif
 +return len;
  }
  
  static int nb_vcs;
 @@ -1190,19 +1192,12 @@ void early_gtk_display_init(void)
  }
  
  #if defined(CONFIG_VTE)
 -static gboolean gd_vc_in(GIOChannel *chan, GIOCondition cond, void *opaque)
 +static gboolean gd_vc_in(VteTerminal *terminal, gchar *text, guint size,
 + gpointer user_data)
  {
 -VirtualConsole *vc = opaque;
 -uint8_t buffer[1024];
 -ssize_t len;
 -
 -len = read(vc-fd, buffer, sizeof(buffer));
 -if (len = 0) {
 -return FALSE;
 -}
 -
 -qemu_chr_be_write(vc-chr, buffer, len);
 +VirtualConsole *vc = user_data;
  
 +qemu_chr_be_write(vc-chr, (uint8_t  *)text, (unsigned int)size);
  return TRUE;
  }
  #endif
 @@ -1214,13 +1209,8 @@ static GSList *gd_vc_init(GtkDisplayState *s, 
 VirtualConsole *vc, int index, GSL
  const char *label;
  char buffer[32];
  char path[32];
 -#if VTE_CHECK_VERSION(0, 26, 0)
 -VtePty *pty;
 -#endif
 -GIOChannel *chan;
  GtkWidget *scrolled_window;
  GtkAdjustment *vadjustment;
 -int master_fd, slave_fd;
  
  snprintf(buffer, sizeof(buffer), vc%d, index);
  snprintf(path, sizeof(path), QEMU/View/VC%d, index);
 @@ -1239,16 +1229,7 @@ static GSList *gd_vc_init(GtkDisplayState *s, 
 VirtualConsole *vc, int index, GSL
  gtk_accel_map_add_entry(path, GDK_KEY_2 + index, HOTKEY_MODIFIERS);
  
  vc-terminal = vte_terminal_new();
 -
 -master_fd = qemu_openpty_raw(slave_fd, NULL);
 -g_assert(master_fd != -1);
 -
 -#if VTE_CHECK_VERSION(0, 26, 0)
 -pty = vte_pty_new_foreign(master_fd, NULL);
 -vte_terminal_set_pty_object(VTE_TERMINAL(vc-terminal), pty);
 -#else
 -vte_terminal_set_pty(VTE_TERMINAL(vc-terminal), master_fd);
 -#endif
 +g_signal_connect(vc-terminal, commit, G_CALLBACK(gd_vc_in), vc);
  
  vte_terminal_set_scrollback_lines(VTE_TERMINAL(vc-terminal), -1);
  
 @@ -1263,7 +1244,6 @@ static GSList *gd_vc_init(GtkDisplayState *s, 
 VirtualConsole *vc, int index, GSL
  
  vte_terminal_set_size(VTE_TERMINAL(vc-terminal), 80, 25);
  
 -vc-fd = slave_fd;
  vc-chr-opaque = vc;
  vc-scrolled_window = scrolled_window;
  
 @@ -1281,9 +1261,6 @@ static GSList *gd_vc_init(GtkDisplayState *s, 
 VirtualConsole *vc, int index, GSL
  vc-chr-init(vc-chr);
  }
  
 -chan = g_io_channel_unix_new(vc-fd);
 -g_io_add_watch(chan, G_IO_IN, gd_vc_in, vc);
 -
  #endif /* CONFIG_VTE */
  return group;
  }
 

This commit somehow messes up the monitor vc: Fire up qemu-system-x86_64
and switch to console 2 (monitor). You'll find it formatted as if the
console was only ~10 chars wide during printout of the monitor
greetings. When typing, everything is fine again. Maybe an ordering
issue that was only revealed by this commit, dunno yet.

Jan



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PULL 16/31] block: Fix open_flags in bdrv_reopen()

2014-05-04 Thread Jan Kiszka
On 2014-04-30 20:23, Kevin Wolf wrote:
 Use the same function as bdrv_open() for determining what the right
 flags for bs-file are. Without doing this, a reopen means that
 bs-file loses BDRV_O_CACHE_WB or BDRV_O_UNMAP if bs doesn't have it as
 well.
 
 Signed-off-by: Kevin Wolf kw...@redhat.com
 Reviewed-by: Max Reitz mre...@redhat.com
 ---
  block.c | 5 -
  1 file changed, 4 insertions(+), 1 deletion(-)
 
 diff --git a/block.c b/block.c
 index 9f6f07e..b749d31 100644
 --- a/block.c
 +++ b/block.c
 @@ -1525,8 +1525,11 @@ BlockReopenQueue *bdrv_reopen_queue(BlockReopenQueue 
 *bs_queue,
  QSIMPLEQ_INIT(bs_queue);
  }
  
 +/* bdrv_open() masks this flag out */
 +flags = ~BDRV_O_PROTOCOL;
 +
  if (bs-file) {
 -bdrv_reopen_queue(bs_queue, bs-file, flags);
 +bdrv_reopen_queue(bs_queue, bs-file, bdrv_inherited_flags(flags));
  }
  
  bs_entry = g_new0(BlockReopenQueueEntry, 1);
 

This breaks the commit monitor command for disks in snapshot mode.
Returned error is no permission (I assume -EPERM).

Jan




signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PULL 7/8] gtk: Fix -serial vc

2014-05-04 Thread Jan Kiszka
On 2014-05-04 19:07, Cole Robinson wrote:
 On 05/04/2014 04:35 AM, Jan Kiszka wrote:
 On 2014-04-29 11:46, Gerd Hoffmann wrote:
 From: Cole Robinson crobi...@redhat.com

 Try kicking off a rhel5 text install over serial, the text menu navigation
 is all messed up, and some of the kernel boot messages are randomly
 corrupted.

 Drop use of a pty and just use vte infrastructure for reading and writing.
 This fixes the above corruption, and is simpler to boot.

 (I don't know what was wrong with the original code though. FWIW this is
 what virt-manager has done for years).

 Signed-off-by: Cole Robinson crobi...@redhat.com
 Signed-off-by: Gerd Hoffmann kra...@redhat.com
 ---
  ui/gtk.c | 41 +
  1 file changed, 9 insertions(+), 32 deletions(-)

 diff --git a/ui/gtk.c b/ui/gtk.c
 index c85aea3..1465a38 100644
 --- a/ui/gtk.c
 +++ b/ui/gtk.c
 @@ -115,7 +115,6 @@ typedef struct VirtualConsole
  GtkWidget *scrolled_window;
  CharDriverState *chr;
  #endif
 -int fd;
  } VirtualConsole;
  
  typedef struct GtkDisplayState
 @@ -1162,9 +1161,12 @@ static gboolean gd_focus_out_event(GtkWidget *widget,
  
  static int gd_vc_chr_write(CharDriverState *chr, const uint8_t *buf, int 
 len)
  {
 +#if defined(CONFIG_VTE)
  VirtualConsole *vc = chr-opaque;
  
 -return vc ? write(vc-fd, buf, len) : len;
 +vte_terminal_feed(VTE_TERMINAL(vc-terminal), (const char *)buf, len);
 +#endif
 +return len;
  }
  
  static int nb_vcs;
 @@ -1190,19 +1192,12 @@ void early_gtk_display_init(void)
  }
  
  #if defined(CONFIG_VTE)
 -static gboolean gd_vc_in(GIOChannel *chan, GIOCondition cond, void *opaque)
 +static gboolean gd_vc_in(VteTerminal *terminal, gchar *text, guint size,
 + gpointer user_data)
  {
 -VirtualConsole *vc = opaque;
 -uint8_t buffer[1024];
 -ssize_t len;
 -
 -len = read(vc-fd, buffer, sizeof(buffer));
 -if (len = 0) {
 -return FALSE;
 -}
 -
 -qemu_chr_be_write(vc-chr, buffer, len);
 +VirtualConsole *vc = user_data;
  
 +qemu_chr_be_write(vc-chr, (uint8_t  *)text, (unsigned int)size);
  return TRUE;
  }
  #endif
 @@ -1214,13 +1209,8 @@ static GSList *gd_vc_init(GtkDisplayState *s, 
 VirtualConsole *vc, int index, GSL
  const char *label;
  char buffer[32];
  char path[32];
 -#if VTE_CHECK_VERSION(0, 26, 0)
 -VtePty *pty;
 -#endif
 -GIOChannel *chan;
  GtkWidget *scrolled_window;
  GtkAdjustment *vadjustment;
 -int master_fd, slave_fd;
  
  snprintf(buffer, sizeof(buffer), vc%d, index);
  snprintf(path, sizeof(path), QEMU/View/VC%d, index);
 @@ -1239,16 +1229,7 @@ static GSList *gd_vc_init(GtkDisplayState *s, 
 VirtualConsole *vc, int index, GSL
  gtk_accel_map_add_entry(path, GDK_KEY_2 + index, HOTKEY_MODIFIERS);
  
  vc-terminal = vte_terminal_new();
 -
 -master_fd = qemu_openpty_raw(slave_fd, NULL);
 -g_assert(master_fd != -1);
 -
 -#if VTE_CHECK_VERSION(0, 26, 0)
 -pty = vte_pty_new_foreign(master_fd, NULL);
 -vte_terminal_set_pty_object(VTE_TERMINAL(vc-terminal), pty);
 -#else
 -vte_terminal_set_pty(VTE_TERMINAL(vc-terminal), master_fd);
 -#endif
 +g_signal_connect(vc-terminal, commit, G_CALLBACK(gd_vc_in), vc);
  
  vte_terminal_set_scrollback_lines(VTE_TERMINAL(vc-terminal), -1);
  
 @@ -1263,7 +1244,6 @@ static GSList *gd_vc_init(GtkDisplayState *s, 
 VirtualConsole *vc, int index, GSL
  
  vte_terminal_set_size(VTE_TERMINAL(vc-terminal), 80, 25);
  
 -vc-fd = slave_fd;
  vc-chr-opaque = vc;
  vc-scrolled_window = scrolled_window;
  
 @@ -1281,9 +1261,6 @@ static GSList *gd_vc_init(GtkDisplayState *s, 
 VirtualConsole *vc, int index, GSL
  vc-chr-init(vc-chr);
  }
  
 -chan = g_io_channel_unix_new(vc-fd);
 -g_io_add_watch(chan, G_IO_IN, gd_vc_in, vc);
 -
  #endif /* CONFIG_VTE */
  return group;
  }


 This commit somehow messes up the monitor vc: Fire up qemu-system-x86_64
 and switch to console 2 (monitor). You'll find it formatted as if the
 console was only ~10 chars wide during printout of the monitor
 greetings. When typing, everything is fine again. Maybe an ordering
 issue that was only revealed by this commit, dunno yet.

 
 Check out gerd's ui-gtk-next branch, there's a few extra patches related to
 vte sizing that might fix it.

Looks better thanks to gtk: zap scrolled_window (monitor is properly
formatted again). But the whole queue spits this out on the terminal:

(unknown:13169): Gtk-CRITICAL **: IA__gtk_window_resize: assertion `width  
0' failed

(unknown:13169): Gdk-CRITICAL **: IA__gdk_window_set_cursor: assertion 
`GDK_IS_WINDOW (window)' failed

The last two patches seem to be responsible for this.

Jan




signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PULL 7/8] gtk: Fix -serial vc

2014-05-04 Thread Jan Kiszka
On 2014-05-04 20:25, Jan Kiszka wrote:
 On 2014-05-04 19:07, Cole Robinson wrote:
 On 05/04/2014 04:35 AM, Jan Kiszka wrote:
 On 2014-04-29 11:46, Gerd Hoffmann wrote:
 From: Cole Robinson crobi...@redhat.com

 Try kicking off a rhel5 text install over serial, the text menu navigation
 is all messed up, and some of the kernel boot messages are randomly
 corrupted.

 Drop use of a pty and just use vte infrastructure for reading and writing.
 This fixes the above corruption, and is simpler to boot.

 (I don't know what was wrong with the original code though. FWIW this is
 what virt-manager has done for years).

 Signed-off-by: Cole Robinson crobi...@redhat.com
 Signed-off-by: Gerd Hoffmann kra...@redhat.com
 ---
  ui/gtk.c | 41 +
  1 file changed, 9 insertions(+), 32 deletions(-)

 diff --git a/ui/gtk.c b/ui/gtk.c
 index c85aea3..1465a38 100644
 --- a/ui/gtk.c
 +++ b/ui/gtk.c
 @@ -115,7 +115,6 @@ typedef struct VirtualConsole
  GtkWidget *scrolled_window;
  CharDriverState *chr;
  #endif
 -int fd;
  } VirtualConsole;
  
  typedef struct GtkDisplayState
 @@ -1162,9 +1161,12 @@ static gboolean gd_focus_out_event(GtkWidget 
 *widget,
  
  static int gd_vc_chr_write(CharDriverState *chr, const uint8_t *buf, int 
 len)
  {
 +#if defined(CONFIG_VTE)
  VirtualConsole *vc = chr-opaque;
  
 -return vc ? write(vc-fd, buf, len) : len;
 +vte_terminal_feed(VTE_TERMINAL(vc-terminal), (const char *)buf, len);
 +#endif
 +return len;
  }
  
  static int nb_vcs;
 @@ -1190,19 +1192,12 @@ void early_gtk_display_init(void)
  }
  
  #if defined(CONFIG_VTE)
 -static gboolean gd_vc_in(GIOChannel *chan, GIOCondition cond, void 
 *opaque)
 +static gboolean gd_vc_in(VteTerminal *terminal, gchar *text, guint size,
 + gpointer user_data)
  {
 -VirtualConsole *vc = opaque;
 -uint8_t buffer[1024];
 -ssize_t len;
 -
 -len = read(vc-fd, buffer, sizeof(buffer));
 -if (len = 0) {
 -return FALSE;
 -}
 -
 -qemu_chr_be_write(vc-chr, buffer, len);
 +VirtualConsole *vc = user_data;
  
 +qemu_chr_be_write(vc-chr, (uint8_t  *)text, (unsigned int)size);
  return TRUE;
  }
  #endif
 @@ -1214,13 +1209,8 @@ static GSList *gd_vc_init(GtkDisplayState *s, 
 VirtualConsole *vc, int index, GSL
  const char *label;
  char buffer[32];
  char path[32];
 -#if VTE_CHECK_VERSION(0, 26, 0)
 -VtePty *pty;
 -#endif
 -GIOChannel *chan;
  GtkWidget *scrolled_window;
  GtkAdjustment *vadjustment;
 -int master_fd, slave_fd;
  
  snprintf(buffer, sizeof(buffer), vc%d, index);
  snprintf(path, sizeof(path), QEMU/View/VC%d, index);
 @@ -1239,16 +1229,7 @@ static GSList *gd_vc_init(GtkDisplayState *s, 
 VirtualConsole *vc, int index, GSL
  gtk_accel_map_add_entry(path, GDK_KEY_2 + index, HOTKEY_MODIFIERS);
  
  vc-terminal = vte_terminal_new();
 -
 -master_fd = qemu_openpty_raw(slave_fd, NULL);
 -g_assert(master_fd != -1);
 -
 -#if VTE_CHECK_VERSION(0, 26, 0)
 -pty = vte_pty_new_foreign(master_fd, NULL);
 -vte_terminal_set_pty_object(VTE_TERMINAL(vc-terminal), pty);
 -#else
 -vte_terminal_set_pty(VTE_TERMINAL(vc-terminal), master_fd);
 -#endif
 +g_signal_connect(vc-terminal, commit, G_CALLBACK(gd_vc_in), vc);
  
  vte_terminal_set_scrollback_lines(VTE_TERMINAL(vc-terminal), -1);
  
 @@ -1263,7 +1244,6 @@ static GSList *gd_vc_init(GtkDisplayState *s, 
 VirtualConsole *vc, int index, GSL
  
  vte_terminal_set_size(VTE_TERMINAL(vc-terminal), 80, 25);
  
 -vc-fd = slave_fd;
  vc-chr-opaque = vc;
  vc-scrolled_window = scrolled_window;
  
 @@ -1281,9 +1261,6 @@ static GSList *gd_vc_init(GtkDisplayState *s, 
 VirtualConsole *vc, int index, GSL
  vc-chr-init(vc-chr);
  }
  
 -chan = g_io_channel_unix_new(vc-fd);
 -g_io_add_watch(chan, G_IO_IN, gd_vc_in, vc);
 -
  #endif /* CONFIG_VTE */
  return group;
  }


 This commit somehow messes up the monitor vc: Fire up qemu-system-x86_64
 and switch to console 2 (monitor). You'll find it formatted as if the
 console was only ~10 chars wide during printout of the monitor
 greetings. When typing, everything is fine again. Maybe an ordering
 issue that was only revealed by this commit, dunno yet.


 Check out gerd's ui-gtk-next branch, there's a few extra patches related to
 vte sizing that might fix it.
 
 Looks better thanks to gtk: zap scrolled_window (monitor is properly
 formatted again).

...err - no. The price of this patch is that the window is no longer
properly resized on all guest mode changes. There is still more broken.

Jan




signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [GSoC] Wanted: small warmup tasks

2014-04-25 Thread Jan Kiszka
On 2014-04-24 08:19, Jan Kiszka wrote:
 On 2014-04-23 11:25, Stefan Hajnoczi wrote:
 Dear QEMU, Libvirt, and KVM communities,
 We are participating in Google Summer of Code 2014
 (http://google-melange.com/) and Outreach Program for Women
 (http://opw.gnome.org/).  Both programs fund candidates to work on our
 open source projects for 12 weeks this summer.
 
 To follow up on this: I'm currently looking for optional tiny warmup
 tasks for our QEMU students during the bonding period (till May 18). If
 you have any trivial issues or extensions in mind that someone could
 address within a few days or even hours, that would be perfect. It could
 even be something like reformat the printing of these messages or so.
 
 We used this mechanism last year with the KVM student quite
 successfully. The idea is to give the student very early a chance to get
 in contact with the community and with the patch submission  review
 procedure. So the focus is more on dealing with patches than on solving
 a technical problem in QEMU. If all works fine, this should encourage
 her/him to work with the community right from the beginning, ask
 question, post things early etc.

Thanks for all these suggestion so far! I personally wasn't able to look
into them in details yet.

A wish to the mentors and students: if someone picks up a task, please
drop a note here so that we can avoid duplicate work!

Jan



signature.asc
Description: OpenPGP digital signature


[Qemu-devel] [GSoC] Wanted: small warmup tasks

2014-04-24 Thread Jan Kiszka
On 2014-04-23 11:25, Stefan Hajnoczi wrote:
 Dear QEMU, Libvirt, and KVM communities,
 We are participating in Google Summer of Code 2014
 (http://google-melange.com/) and Outreach Program for Women
 (http://opw.gnome.org/).  Both programs fund candidates to work on our
 open source projects for 12 weeks this summer.

To follow up on this: I'm currently looking for optional tiny warmup
tasks for our QEMU students during the bonding period (till May 18). If
you have any trivial issues or extensions in mind that someone could
address within a few days or even hours, that would be perfect. It could
even be something like reformat the printing of these messages or so.

We used this mechanism last year with the KVM student quite
successfully. The idea is to give the student very early a chance to get
in contact with the community and with the patch submission  review
procedure. So the focus is more on dealing with patches than on solving
a technical problem in QEMU. If all works fine, this should encourage
her/him to work with the community right from the beginning, ask
question, post things early etc.

Jan




signature.asc
Description: OpenPGP digital signature


[Qemu-devel] [PATCH] pc: Fix disabling of vapic for compat PC models

2014-04-20 Thread Jan Kiszka
From: Jan Kiszka jan.kis...@siemens.com

We used to be able to address both the QEMU and the KVM APIC via apic.
This doesn't work anymore. So we need to use their parent class to turn
off the vapic on machines that should not expose them.

Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---

I'm pretty sure it was like this, that -global apic.vapic=off also
worked for -enable-kvm, ie. the KVM APIC. If someone could explain what
changes, probably around QOM - maybe we can restore the original
behavior and skip this patch.

 hw/i386/pc_piix.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 7930a26..12ee716 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -566,7 +566,7 @@ static QEMUMachine pc_machine_v1_1 = {
 .property = class,\
 .value= stringify(PCI_CLASS_MEMORY_RAM),\
 },{\
-.driver   = apic,\
+.driver   = apic-common,\
 .property = vapic,\
 .value= off,\
 },{\
-- 
1.8.1.1.298.ge7eed54



Re: [Qemu-devel] About SIG_IPI handler

2014-04-16 Thread Jan Kiszka
On 2014-04-17 07:46, Shiru Ren wrote:
 Hi, all
 
 I’m trying to figure out how do_savevm works in QEMU. But there is one
 thing has bothered me quite a lot. I found that vm_stop invoke
 qemu_cpu_kick_thread to send SIG_IPI to a vcpu thread, and I have
 understand that in TCG mode, the cpu_signal() function will be invoked as
 the SIG_IPI handler. But I don’t know what happens in KVM mode. Actually I
 can’t find the signal handler function. I only find a function named
 dummy_signal, and it doesn't do anything.

This signal is handled synchronously in KVM mode, see
qemu_kvm_eat_signals in cpus.c.

Jan




signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH v2 for-2.0] target-i386: x86_cpu_get_phys_page_debug(): support 1GB page translation

2014-03-20 Thread Jan Kiszka
On 2014-03-19 22:03, Luiz Capitulino wrote:
 Linux guests, when using more than 4GB of RAM, may end up using 1GB pages
 to store (kernel) data. When this happens, we're unable to debug a running
 Linux kernel with GDB:
 
 (gdb) p node_data[0]-node_id
 Cannot access memory at address 0x88013fffd3a0
 (gdb)
 
 GDB returns this error because x86_cpu_get_phys_page_debug() doesn't support
 translating 1GB pages in IA-32e paging mode and returns an error to GDB.
 
 This commit adds support for 1GB page translation for IA32e paging.
 
 Signed-off-by: Luiz capitulino lcapitul...@redhat.com
 ---
 
 - I'm proposing this patch for 2.0 because GDB debugging of large Linux
   guests is kind of broken
 
 - Changelog v2:
   - Move PS bit handling to if (env-hflags  HF_LMA_MASK) block
   - Update changelog
 
  target-i386/helper.c | 9 +
  1 file changed, 9 insertions(+)
 
 diff --git a/target-i386/helper.c b/target-i386/helper.c
 index 4f447b8..7cee501 100644
 --- a/target-i386/helper.c
 +++ b/target-i386/helper.c
 @@ -941,6 +941,14 @@ hwaddr x86_cpu_get_phys_page_debug(CPUState *cs, vaddr 
 addr)
  pdpe = ldq_phys(cs-as, pdpe_addr);
  if (!(pdpe  PG_PRESENT_MASK))
  return -1;
 +
 +if (pdpe  PG_PSE_MASK) {
 +page_size = 1024 * 1024 * 1024;
 +pte = pdpe  ~( (page_size - 1)  ~0xfff);
 +pte = ~(PG_NX_MASK | PG_HI_USER_MASK);
 +goto out;
 +}
 +
  } else
  #endif
  {
 @@ -993,6 +1001,7 @@ hwaddr x86_cpu_get_phys_page_debug(CPUState *cs, vaddr 
 addr)
  pte = pte  env-a20_mask;
  }
  
 +out:
  page_offset = (addr  TARGET_PAGE_MASK)  (page_size - 1);
  paddr = (pte  TARGET_PAGE_MASK) + page_offset;
  return paddr;
 

Reviewed-by: Jan Kiszka jan.kis...@siemens.com

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] [PATCH for-2.0?] target-i386: fix gdb debugging with large memory guests

2014-03-18 Thread Jan Kiszka
On 2014-03-18 02:54, Luiz Capitulino wrote:
 If you start a Linux guest with more than 4GB of memory and try to look at a
 memory address, you will get an error from gdb:
 
 (gdb) p node_data[0]-node_id
 Cannot access memory at address 0x88013fffd3a0
 (gdb)

I suppose this is x86-64, not 32-bit with PTE, right?

 
 I debugged this down to x86_cpu_get_phys_page_debug(), it doesn't handle the
 case where the PDPTE has the PS bit set (although I didn't check where Linux
 sets that bit). This commit adds the PS bit handling, which fixes the problem
 for me.
 
 Signed-off-by: Luiz capitulino lcapitul...@redhat.com
 ---
 
 Two observations:
 
  1. This bug has always existed, so it's not a regression, so I'm not sure
 it's worth it to fix for 2.0
 
  2. I'm not familiar with every detail of x86_cpu_get_phys_page_debug(),
 so I'm not completely sure this is the right thing to do
 
  target-i386/helper.c | 8 
  1 file changed, 8 insertions(+)
 
 diff --git a/target-i386/helper.c b/target-i386/helper.c
 index 4f447b8..9b7803f 100644
 --- a/target-i386/helper.c
 +++ b/target-i386/helper.c
 @@ -951,6 +951,13 @@ hwaddr x86_cpu_get_phys_page_debug(CPUState *cs, vaddr 
 addr)
  return -1;
  }
  
 +if (pdpe  PG_PSE_MASK) {
 +page_size = 1024 * 1024 * 1024;
 +pte = pdpe  ~( (page_size - 1)  ~0xfff);
 +pte = ~(PG_NX_MASK | PG_HI_USER_MASK);
 +goto out;
 +}

Does this also apply if we are not in long mode?

I'll check this more carefully later.

Jan

 +
  pde_addr = ((pdpe  ~0xfff  ~(PG_NX_MASK | PG_HI_USER_MASK)) +
  (((addr  21)  0x1ff)  3))  env-a20_mask;
  pde = ldq_phys(cs-as, pde_addr);
 @@ -993,6 +1000,7 @@ hwaddr x86_cpu_get_phys_page_debug(CPUState *cs, vaddr 
 addr)
  pte = pte  env-a20_mask;
  }
  
 +out:
  page_offset = (addr  TARGET_PAGE_MASK)  (page_size - 1);
  paddr = (pte  TARGET_PAGE_MASK) + page_offset;
  return paddr;
 

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] [PATCH for-2.0?] target-i386: fix gdb debugging with large memory guests

2014-03-18 Thread Jan Kiszka
On 2014-03-18 08:36, Paolo Bonzini wrote:
 Il 18/03/2014 08:19, Jan Kiszka ha scritto:
 On 2014-03-18 02:54, Luiz Capitulino wrote:
 If you start a Linux guest with more than 4GB of memory and try to
 look at a
 memory address, you will get an error from gdb:

 (gdb) p node_data[0]-node_id
 Cannot access memory at address 0x88013fffd3a0
 (gdb)

 I suppose this is x86-64, not 32-bit with PTE, right?


 I debugged this down to x86_cpu_get_phys_page_debug(), it doesn't
 handle the
 case where the PDPTE has the PS bit set (although I didn't check
 where Linux
 sets that bit). This commit adds the PS bit handling, which fixes the
 problem
 for me.

 Signed-off-by: Luiz capitulino lcapitul...@redhat.com
 ---

 Two observations:

  1. This bug has always existed, so it's not a regression, so I'm not
 sure
 it's worth it to fix for 2.0
 
 Sure, why not?
 
  2. I'm not familiar with every detail of x86_cpu_get_phys_page_debug(),
 so I'm not completely sure this is the right thing to do

  target-i386/helper.c | 8 
  1 file changed, 8 insertions(+)

 diff --git a/target-i386/helper.c b/target-i386/helper.c
 index 4f447b8..9b7803f 100644
 --- a/target-i386/helper.c
 +++ b/target-i386/helper.c
 @@ -951,6 +951,13 @@ hwaddr x86_cpu_get_phys_page_debug(CPUState *cs,
 vaddr addr)
  return -1;
  }

 +if (pdpe  PG_PSE_MASK) {
 +page_size = 1024 * 1024 * 1024;
 +pte = pdpe  ~( (page_size - 1)  ~0xfff);
 +pte = ~(PG_NX_MASK | PG_HI_USER_MASK);
 +goto out;
 +}

 Does this also apply if we are not in long mode?
 
 No, it doesn't.  The only valid bits in a PAE PDPTE are P, PWT and PCD.
  Bit 7 (PS) is reserved.

Right, this belongs in the if (env-hflags  HF_LMA_MASK) block.

And the subject or description should mention that
x86_cpu_get_phys_page_debug was lacking support for 1G hugepages.

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] [PATCH for-2.0?] target-i386: fix gdb debugging with large memory guests

2014-03-18 Thread Jan Kiszka
On 2014-03-18 14:00, Luiz Capitulino wrote:
 On Tue, 18 Mar 2014 11:30:42 +0100
 Jan Kiszka jan.kis...@siemens.com wrote:
 
 On 2014-03-18 08:36, Paolo Bonzini wrote:
 Il 18/03/2014 08:19, Jan Kiszka ha scritto:
 On 2014-03-18 02:54, Luiz Capitulino wrote:
 If you start a Linux guest with more than 4GB of memory and try to
 look at a
 memory address, you will get an error from gdb:

 (gdb) p node_data[0]-node_id
 Cannot access memory at address 0x88013fffd3a0
 (gdb)

 I suppose this is x86-64, not 32-bit with PTE, right?


 I debugged this down to x86_cpu_get_phys_page_debug(), it doesn't
 handle the
 case where the PDPTE has the PS bit set (although I didn't check
 where Linux
 sets that bit). This commit adds the PS bit handling, which fixes the
 problem
 for me.

 Signed-off-by: Luiz capitulino lcapitul...@redhat.com
 ---

 Two observations:

  1. This bug has always existed, so it's not a regression, so I'm not
 sure
 it's worth it to fix for 2.0

 Sure, why not?

  2. I'm not familiar with every detail of x86_cpu_get_phys_page_debug(),
 so I'm not completely sure this is the right thing to do

  target-i386/helper.c | 8 
  1 file changed, 8 insertions(+)

 diff --git a/target-i386/helper.c b/target-i386/helper.c
 index 4f447b8..9b7803f 100644
 --- a/target-i386/helper.c
 +++ b/target-i386/helper.c
 @@ -951,6 +951,13 @@ hwaddr x86_cpu_get_phys_page_debug(CPUState *cs,
 vaddr addr)
  return -1;
  }

 +if (pdpe  PG_PSE_MASK) {
 +page_size = 1024 * 1024 * 1024;
 +pte = pdpe  ~( (page_size - 1)  ~0xfff);
 +pte = ~(PG_NX_MASK | PG_HI_USER_MASK);
 +goto out;
 +}

 Does this also apply if we are not in long mode?

 No, it doesn't.  The only valid bits in a PAE PDPTE are P, PWT and PCD.
  Bit 7 (PS) is reserved.

 Right, this belongs in the if (env-hflags  HF_LMA_MASK) block.

 And the subject or description should mention that
 x86_cpu_get_phys_page_debug was lacking support for 1G hugepages.
 
 To be honest, although the PS bit is set and that indicates a 1GB page,
 I didn't know Linux does that. I thought Linux would use 4KB pages for
 everything unless it's explicitly asked to use bigger pages. Also, note that
 I was using gdb to debug really early kernel boot code (start_kernel()).

I could imagine that Linux initially creates a giant identity mapping
page table for the startup process and only later on switches to
fine-grained tables of 4K and 2M pages. Giant pages still require
hughtlbfs, IIRC.

 
 I'd feel more confident to have such a changelog after I find out where
 exactly Linux sets that bit, but I won't have time in the next days. On the
 other hand, the patch does fix the problem to me.

Don't worry about Linux (the code should work with any OS anyway), just
believe your reviewers. ;) Alternatively, check Intel IA32 SDM on page
table structures.

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] [PATCH for-2.0?] target-i386: fix gdb debugging with large memory guests

2014-03-18 Thread Jan Kiszka
On 2014-03-18 17:37, Paolo Bonzini wrote:
 Il 18/03/2014 17:23, Luiz Capitulino ha scritto:
 On Tue, 18 Mar 2014 15:36:45 +0100
 Jan Kiszka jan.kis...@siemens.com wrote:

 Right, this belongs in the if (env-hflags  HF_LMA_MASK) block.

 And the subject or description should mention that
 x86_cpu_get_phys_page_debug was lacking support for 1G hugepages.

 To be honest, although the PS bit is set and that indicates a 1GB page,
 I didn't know Linux does that. I thought Linux would use 4KB pages for
 everything unless it's explicitly asked to use bigger pages. Also,
 note that
 I was using gdb to debug really early kernel boot code
 (start_kernel()).

 I could imagine that Linux initially creates a giant identity mapping
 page table for the startup process and only later on switches to
 fine-grained tables of 4K and 2M pages. Giant pages still require
 hughtlbfs, IIRC.


 I'd feel more confident to have such a changelog after I find out where
 exactly Linux sets that bit, but I won't have time in the next days.
 On the
 other hand, the patch does fix the problem to me.

 Don't worry about Linux (the code should work with any OS anyway), just
 believe your reviewers. ;) Alternatively, check Intel IA32 SDM on page
 table structures.

 OK, so you want me to change the subject? Anything else for v2?
 
 You only need to move the new code into the if (env-hflags 
 HF_LMA_MASK), I think.  The subject is ok.

Yes. Subject is fine, a reference to GB pages in the description would
be nice-to-have.

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] [PATCH 1/6] slirp: Remove default_mon usage

2014-03-12 Thread Jan Kiszka
On 2014-03-12 00:15, Cole Robinson wrote:
 These errors don't seem user initiated, so forcibly printing to the
 monitor doesn't seem right. Just print to stderr.
 
 Drop lprint since it's now unused.
 
 Cc: Jan Kiszka jan.kis...@siemens.com
 Signed-off-by: Cole Robinson crobi...@redhat.com
 ---
 checkpatch flags some pre-existing tab issues, but I didn't retab. Should I?
 
  slirp/misc.c  | 13 ++---
  slirp/slirp.c |  8 
  slirp/slirp.h |  2 --
  3 files changed, 6 insertions(+), 17 deletions(-)
 
 diff --git a/slirp/misc.c b/slirp/misc.c
 index 6c1636f..662fb1d 100644
 --- a/slirp/misc.c
 +++ b/slirp/misc.c
 @@ -136,7 +136,7 @@ fork_exec(struct socket *so, const char *ex, int do_pty)
   if ((s = qemu_socket(AF_INET, SOCK_STREAM, 0))  0 ||
   bind(s, (struct sockaddr *)addr, addrlen)  0 ||
   listen(s, 1)  0) {
 - lprint(Error: inet socket: %s\n, strerror(errno));
 + fprintf(stderr, Error: inet socket: %s\n, 
 strerror(errno));
   closesocket(s);
  
   return 0;
 @@ -146,7 +146,7 @@ fork_exec(struct socket *so, const char *ex, int do_pty)
   pid = fork();
   switch(pid) {
case -1:
 - lprint(Error: fork failed: %s\n, strerror(errno));
 + fprintf(stderr, Error: fork failed: %s\n, strerror(errno));
   close(s);
   return 0;
  
 @@ -242,15 +242,6 @@ strdup(str)
  }
  #endif
  
 -void lprint(const char *format, ...)
 -{
 -va_list args;
 -
 -va_start(args, format);
 -monitor_vprintf(default_mon, format, args);
 -va_end(args);
 -}
 -
  void slirp_connection_info(Slirp *slirp, Monitor *mon)
  {
  const char * const tcpstates[] = {
 diff --git a/slirp/slirp.c b/slirp/slirp.c
 index bad8dad..3fb48a4 100644
 --- a/slirp/slirp.c
 +++ b/slirp/slirp.c
 @@ -139,7 +139,7 @@ int get_dns_addr(struct in_addr *pdns_addr)
  return -1;
  
  #ifdef DEBUG
 -lprint(IP address of your DNS(s): );
 +fprintf(stderr, IP address of your DNS(s): );
  #endif
  while (fgets(buff, 512, f) != NULL) {
  if (sscanf(buff, nameserver%*[ \t]%256s, buff2) == 1) {
 @@ -153,17 +153,17 @@ int get_dns_addr(struct in_addr *pdns_addr)
  }
  #ifdef DEBUG
  else
 -lprint(, );
 +fprintf(stderr, , );
  #endif
  if (++found  3) {
  #ifdef DEBUG
 -lprint((more));
 +fprintf(stderr, (more));
  #endif
  break;
  }
  #ifdef DEBUG
  else
 -lprint(%s, inet_ntoa(tmp_addr));
 +fprintf(stderr, %s, inet_ntoa(tmp_addr));
  #endif
  }
  }
 diff --git a/slirp/slirp.h b/slirp/slirp.h
 index e4a1bd4..6589d7e 100644
 --- a/slirp/slirp.h
 +++ b/slirp/slirp.h
 @@ -287,8 +287,6 @@ void if_start(struct ttys *);
   long gethostid(void);
  #endif
  
 -void lprint(const char *, ...) GCC_FMT_ATTR(1, 2);
 -
  #ifndef _WIN32
  #include netdb.h
  #endif
 

Reviewed-by: Jan Kiszka jan.kis...@siemens.com

I suppose this goes through Luiz' queue?

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] slirp smb with modern win guests when samba is also running on host

2014-03-12 Thread Jan Kiszka
On 2013-11-28 20:32, Michael Tokarev wrote:
 After numerous reports that -smb (or -netdev user,smb=foo) not working
 with modern windows (win7 and vista are reported as non-working), I
 started digging myself.  And found that indeed it doesn't work, and
 why.
 
 The thing is that modern win tries to connect to port 445 (microsoft-ds)
 first, and if that fails, it falls back to old port 139 (netbios-ssn).
 
 slirp code in qemu only redirects port 139, it does not touch port 445.
 
 So the prob is that if samba is also running on the host, guest will try
 to communicate using port 445, and that will succed, but ofcourse guest
 will not talk with our samba but with samba running on the host.
 
 If samba is not running on the host, guest will fall back to port 139,
 and will reach the redirecting rule and qemu will spawn smbd correctly.
 
 The solution is to redirect both ports (139 and 445), and the fix is
 a one-liner, adding second call to slirp_add_exec() at the end of
 net/slirp.c:slirp_smb() function (provided below).
 
 But it looks like that is not a proper fix really, since in theory
 we should redirect both ports to the SAME, single samba instance,
 but I'm not sure this is possible with slirp.  Well, even if two
 smbd processes will be run on the same config dir, it should not
 be a problem.

I don't see either that this is expressible with the current exec
feature of slirp.

 
 The one-liner (not exactly 1 since it touches previous line too) is like
 this:
 
 Signed-off-By: Michael Tokarev m...@tls.msk.ru
 
 diff --git a/net/slirp.c b/net/slirp.c
 index 124e953..a22e976 100644
 --- a/net/slirp.c
 +++ b/net/slirp.c
 @@ -549,7 +549,8 @@ static int slirp_smb(SlirpState* s, const char 
 *exported_dir
  snprintf(smb_cmdline, sizeof(smb_cmdline), %s -s %s,
   CONFIG_SMBD_COMMAND, smb_conf);
 
 -if (slirp_add_exec(s-slirp, 0, smb_cmdline, vserver_addr, 139)  0) {
 +if (slirp_add_exec(s-slirp, 0, smb_cmdline, vserver_addr, 139)  0 ||
 +slirp_add_exec(s-slirp, 0, smb_cmdline, vserver_addr, 445)  0) {
  slirp_smb_cleanup(s);
  error_report(conflicting/invalid smbserver address);
  return -1;
 

Thanks, merged to my slirp queue. Will send this out ASAP, sorry for the
excessive delay.

JAn

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] [PATCHv3 00/16] slirp: Adding IPv6 support to Qemu -net user mode

2014-03-12 Thread Jan Kiszka
On 2014-02-11 14:08, Samuel Thibault wrote:
 Hello,
 
 This is a respin of IPv6 in Qemu -net user mode.
 
 These patches add ICMPv6, NDP, and make UDP and TCP compatible with
 IPv6. We have made some refactoring to make current code compatible with
 IPv6.
 
 Some patches, like 2 and 13, can be reviewed using
 interdiff -w /dev/null patchfile
 to get rid of the indentation.

The alternative is to split reindention from refactoring patches.

 
 Difference with version 2 is:
 - The default IP range has been made fec0::/64 instead of fc00::/64
 

This is very valuable work. Unfortunately, I'm lacking time to review or
even test this ATM. If someone else could, that would be great.

Thanks,
Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux



[Qemu-devel] [pull] slirp queue

2014-03-12 Thread Jan Kiszka
The following changes since commit 239618707637ec87eba8c452d2b2f75dc5ca20c7:

  Merge remote-tracking branch 'remotes/kvm/uq/master' into staging (2014-03-11 
19:39:17 +)

are available in the git repository at:


  git://git.kiszka.org/qemu.git queues/slirp

for you to fetch changes up to 5c1e1890bfa1f6b4bc3f51e368bfd47af1b60db0:

  slirp smb with modern win guests when samba is also running on host 
(2014-03-12 08:13:24 +0100)


Michael Buesch (1):
  qemu/slirp: Fix SMB security configuration on newer samba versions

Michael Tokarev (1):
  slirp smb with modern win guests when samba is also running on host

 net/slirp.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)



[Qemu-devel] [PATCH][RESEND] gtk: Allow to activate grab-on-hover from the command line

2014-03-12 Thread Jan Kiszka
As long as we have no persistent GTK configuration, this allows to
enable the useful grab-on-hover feature already when starting the VM.

Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---
 include/ui/console.h |  2 +-
 qemu-options.hx  |  5 +
 ui/gtk.c |  5 -
 vl.c | 22 +-
 4 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/include/ui/console.h b/include/ui/console.h
index 08a38ea..8a86617 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -345,6 +345,6 @@ int index_from_key(const char *key);
 
 /* gtk.c */
 void early_gtk_display_init(void);
-void gtk_display_init(DisplayState *ds, bool full_screen);
+void gtk_display_init(DisplayState *ds, bool full_screen, bool grab_on_hover);
 
 #endif
diff --git a/qemu-options.hx b/qemu-options.hx
index 068da2d..ee5437b 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -810,6 +810,7 @@ ETEXI
 DEF(display, HAS_ARG, QEMU_OPTION_display,
 -display sdl[,frame=on|off][,alt_grab=on|off][,ctrl_grab=on|off]\n
 [,window_close=on|off]|curses|none|\n
+gtk[,grab_on_hover=on|off]|\n
 vnc=display[,optargs]\n
 select display type\n, QEMU_ARCH_ALL)
 STEXI
@@ -833,6 +834,10 @@ graphics card, but its output will not be displayed to the 
QEMU
 user. This option differs from the -nographic option in that it
 only affects what is done with video output; -nographic also changes
 the destination of the serial and parallel port data.
+@item gtk
+Display video output in a GTK window. This interface provides drop-down
+menus and other UI elements to configure and control the VM during
+runtime.
 @item vnc
 Start a VNC server on display arg
 @end table
diff --git a/ui/gtk.c b/ui/gtk.c
index 1851495..992f7d7 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -1415,7 +1415,7 @@ static const DisplayChangeListenerOps dcl_ops = {
 .dpy_cursor_define = gd_cursor_define,
 };
 
-void gtk_display_init(DisplayState *ds, bool full_screen)
+void gtk_display_init(DisplayState *ds, bool full_screen, bool grab_on_hover)
 {
 GtkDisplayState *s = g_malloc0(sizeof(*s));
 char *filename;
@@ -1494,6 +1494,9 @@ void gtk_display_init(DisplayState *ds, bool full_screen)
 if (full_screen) {
 gtk_menu_item_activate(GTK_MENU_ITEM(s-full_screen_item));
 }
+if (grab_on_hover) {
+gtk_menu_item_activate(GTK_MENU_ITEM(s-grab_on_hover_item));
+}
 
 register_displaychangelistener(s-dcl);
 
diff --git a/vl.c b/vl.c
index bca5c95..b73744d 100644
--- a/vl.c
+++ b/vl.c
@@ -143,6 +143,7 @@ int vga_interface_type = VGA_NONE;
 static int full_screen = 0;
 static int no_frame = 0;
 int no_quit = 0;
+static bool grab_on_hover;
 CharDriverState *serial_hds[MAX_SERIAL_PORTS];
 CharDriverState *parallel_hds[MAX_PARALLEL_PORTS];
 CharDriverState *virtcon_hds[MAX_VIRTIO_CONSOLES];
@@ -2241,6 +2242,25 @@ static DisplayType select_display(const char *p)
 } else if (strstart(p, gtk, opts)) {
 #ifdef CONFIG_GTK
 display = DT_GTK;
+while (*opts) {
+const char *nextopt;
+
+if (strstart(opts, ,grab_on_hover=, nextopt)) {
+opts = nextopt;
+if (strstart(opts, on, nextopt)) {
+grab_on_hover = true;
+} else if (strstart(opts, off, nextopt)) {
+grab_on_hover = false;
+} else {
+goto invalid_gtk_args;
+}
+} else {
+invalid_gtk_args:
+fprintf(stderr, Invalid GTK option string: %s\n, p);
+exit(1);
+}
+opts = nextopt;
+}
 #else
 fprintf(stderr, GTK support is disabled\n);
 exit(1);
@@ -4351,7 +4371,7 @@ int main(int argc, char **argv, char **envp)
 #endif
 #if defined(CONFIG_GTK)
 case DT_GTK:
-gtk_display_init(ds, full_screen);
+gtk_display_init(ds, full_screen, grab_on_hover);
 break;
 #endif
 default:
-- 
1.8.1.1.298.ge7eed54



Re: [Qemu-devel] [PATCH 1/2][RESENT] Add GDB qAttached support

2014-03-12 Thread Jan Kiszka
On 2013-07-17 10:10, Jan Kiszka wrote:
 With this patch QEMU handles qAttached request from gdb. When QEMU
 replies 1, GDB sends a detach command at the end of a debugging
 session otherwise GDB sends kill.
 
 The default value for qAttached is 1 on system emulation and 0 on user
 emulation.
 
 Based on original version by Fabien Chouteau.
 
 Signed-off-by: Jan Kiszka jan.kis...@siemens.com
 ---
  gdbstub.c |   10 ++
  1 files changed, 10 insertions(+), 0 deletions(-)
 
 diff --git a/gdbstub.c b/gdbstub.c
 index 0ee82a9..bc626f5 100644
 --- a/gdbstub.c
 +++ b/gdbstub.c
 @@ -42,6 +42,12 @@
  #include sysemu/kvm.h
  #include qemu/bitops.h
  
 +#ifdef CONFIG_USER_ONLY
 +#define GDB_ATTACHED 0
 +#else
 +#define GDB_ATTACHED 1
 +#endif
 +
  #ifndef TARGET_CPU_MEMORY_RW_DEBUG
  static inline int target_memory_rw_debug(CPUArchState *env, target_ulong 
 addr,
   uint8_t *buf, int len, int is_write)
 @@ -2504,6 +2510,10 @@ static int gdb_handle_packet(GDBState *s, const char 
 *line_buf)
  break;
  }
  #endif
 +if (strncmp(p, Attached, 8) == 0) {
 +put_packet(s, GDB_ATTACHED);
 +break;
 +}
  /* Unrecognised 'q' command.  */
  goto unknown_command;
  
 

Peter, could you pick up these two almost trivial long-pending patches?
They still apply and are still useful. If you prefer that I repost them,
just let me know.

Thanks,
Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] [PATCH v2 1/2] Add GDB qAttached support

2014-03-12 Thread Jan Kiszka
On 2014-03-12 17:58, Andreas Färber wrote:
 Am 26.07.2013 20:26, schrieb Jan Kiszka:
 With this patch QEMU handles qAttached request from gdb. When QEMU
 
 With this patch is always weird to read in Git history, also a
 gdbstub:  prefix would've been nice for consistency.

Yes, will rephrase this.

 
 replies 1, GDB sends a detach command at the end of a debugging
 session otherwise GDB sends kill.

 The default value for qAttached is 1 on system emulation and 0 on user
 emulation.

 Based on original version by Fabien Chouteau.
 
 If this is based on code by Fabien, shouldn't it carry his Signed-off-by
 before yours?

Need to check how similar our versions actually are, if I can reuse his
signed-off or if I changed it (in that case it's not appropriate to keep
the original signed-off - according to my understanding).

 
 Since GDB stub is in Odd Fixes state, maybe just step up as
 maintainer and send a pull like for SLIRP? :)

Oh, I can surely send a pull, but I can't handle another maintainership
properly. Slirp already became a stepchild...

Thanks for looking into this.

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] [PULL v3 08/14] Add a 'name' parameter to qemu_thread_create

2014-03-11 Thread Jan Kiszka
On 2014-03-09 20:20, Michael S. Tsirkin wrote:
 From: Dr. David Alan Gilbert dgilb...@redhat.com
 
 If enabled, set the thread name at creation (on GNU systems with
   pthread_set_np)
 Fix up all the callers with a thread name

Seems like not all older Linux systems come with support for
pthread_setname_np. Just ran into a linker bug on a box with glib 2.11.
Can we discover the availability, if nothing helps during configure?

Thanks,
Jan

 
 Signed-off-by: Dr. David Alan Gilbert dgilb...@redhat.com
 Acked-by: Michael S. Tsirkin m...@redhat.com
 Reviewed-by: Laszlo Ersek ler...@redhat.com
 ---
  include/qemu/thread.h   |  2 +-
  cpus.c  | 25 -
  hw/block/dataplane/virtio-blk.c |  2 +-
  hw/usb/ccid-card-emulated.c |  8 
  libcacard/vscclient.c   |  2 +-
  migration.c |  2 +-
  thread-pool.c   |  2 +-
  ui/vnc-jobs.c   |  3 ++-
  util/compatfd.c |  3 ++-
  util/qemu-thread-posix.c|  9 +++--
  util/qemu-thread-win32.c|  2 +-
  11 files changed, 41 insertions(+), 19 deletions(-)
 
 diff --git a/include/qemu/thread.h b/include/qemu/thread.h
 index bf1e110..f7e3b9b 100644
 --- a/include/qemu/thread.h
 +++ b/include/qemu/thread.h
 @@ -52,7 +52,7 @@ void qemu_event_reset(QemuEvent *ev);
  void qemu_event_wait(QemuEvent *ev);
  void qemu_event_destroy(QemuEvent *ev);
  
 -void qemu_thread_create(QemuThread *thread,
 +void qemu_thread_create(QemuThread *thread, const char *name,
  void *(*start_routine)(void *),
  void *arg, int mode);
  void *qemu_thread_join(QemuThread *thread);
 diff --git a/cpus.c b/cpus.c
 index 945d85b..b6421fd 100644
 --- a/cpus.c
 +++ b/cpus.c
 @@ -1117,8 +1117,13 @@ void resume_all_vcpus(void)
  }
  }
  
 +/* For temporary buffers for forming a name */
 +#define VCPU_THREAD_NAME_SIZE 16
 +
  static void qemu_tcg_init_vcpu(CPUState *cpu)
  {
 +char thread_name[VCPU_THREAD_NAME_SIZE];
 +
  tcg_cpu_address_space_init(cpu, cpu-as);
  
  /* share a single thread for all cpus with TCG */
 @@ -1127,8 +1132,10 @@ static void qemu_tcg_init_vcpu(CPUState *cpu)
  cpu-halt_cond = g_malloc0(sizeof(QemuCond));
  qemu_cond_init(cpu-halt_cond);
  tcg_halt_cond = cpu-halt_cond;
 -qemu_thread_create(cpu-thread, qemu_tcg_cpu_thread_fn, cpu,
 -   QEMU_THREAD_JOINABLE);
 +snprintf(thread_name, VCPU_THREAD_NAME_SIZE, CPU %d/TCG,
 + cpu-cpu_index);
 +qemu_thread_create(cpu-thread, thread_name, qemu_tcg_cpu_thread_fn,
 +   cpu, QEMU_THREAD_JOINABLE);
  #ifdef _WIN32
  cpu-hThread = qemu_thread_get_handle(cpu-thread);
  #endif
 @@ -1144,11 +1151,15 @@ static void qemu_tcg_init_vcpu(CPUState *cpu)
  
  static void qemu_kvm_start_vcpu(CPUState *cpu)
  {
 +char thread_name[VCPU_THREAD_NAME_SIZE];
 +
  cpu-thread = g_malloc0(sizeof(QemuThread));
  cpu-halt_cond = g_malloc0(sizeof(QemuCond));
  qemu_cond_init(cpu-halt_cond);
 -qemu_thread_create(cpu-thread, qemu_kvm_cpu_thread_fn, cpu,
 -   QEMU_THREAD_JOINABLE);
 +snprintf(thread_name, VCPU_THREAD_NAME_SIZE, CPU %d/KVM,
 + cpu-cpu_index);
 +qemu_thread_create(cpu-thread, thread_name, qemu_kvm_cpu_thread_fn,
 +   cpu, QEMU_THREAD_JOINABLE);
  while (!cpu-created) {
  qemu_cond_wait(qemu_cpu_cond, qemu_global_mutex);
  }
 @@ -1156,10 +1167,14 @@ static void qemu_kvm_start_vcpu(CPUState *cpu)
  
  static void qemu_dummy_start_vcpu(CPUState *cpu)
  {
 +char thread_name[VCPU_THREAD_NAME_SIZE];
 +
  cpu-thread = g_malloc0(sizeof(QemuThread));
  cpu-halt_cond = g_malloc0(sizeof(QemuCond));
  qemu_cond_init(cpu-halt_cond);
 -qemu_thread_create(cpu-thread, qemu_dummy_cpu_thread_fn, cpu,
 +snprintf(thread_name, VCPU_THREAD_NAME_SIZE, CPU %d/DUMMY,
 + cpu-cpu_index);
 +qemu_thread_create(cpu-thread, thread_name, qemu_dummy_cpu_thread_fn, 
 cpu,
 QEMU_THREAD_JOINABLE);
  while (!cpu-created) {
  qemu_cond_wait(qemu_cpu_cond, qemu_global_mutex);
 diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
 index 2237edb..d1c7ad4 100644
 --- a/hw/block/dataplane/virtio-blk.c
 +++ b/hw/block/dataplane/virtio-blk.c
 @@ -358,7 +358,7 @@ static void start_data_plane_bh(void *opaque)
  
  qemu_bh_delete(s-start_bh);
  s-start_bh = NULL;
 -qemu_thread_create(s-thread, data_plane_thread,
 +qemu_thread_create(s-thread, data_plane, data_plane_thread,
 s, QEMU_THREAD_JOINABLE);
  }
  
 diff --git a/hw/usb/ccid-card-emulated.c b/hw/usb/ccid-card-emulated.c
 index aa913df..7213c89 100644
 --- a/hw/usb/ccid-card-emulated.c
 +++ b/hw/usb/ccid-card-emulated.c
 @@ -546,10 +546,10 @@ static int emulated_initfn(CCIDCardState 

[Qemu-devel] [PATCH] qemu-thread-posix: Fix build against older glibc version

2014-03-11 Thread Jan Kiszka
pthread_setname_np was introduced with 2.12.

Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---
 util/qemu-thread-posix.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c
index 45113b4..960d7f5 100644
--- a/util/qemu-thread-posix.c
+++ b/util/qemu-thread-posix.c
@@ -420,7 +420,7 @@ void qemu_thread_create(QemuThread *thread, const char 
*name,
 if (err)
 error_exit(err, __func__);
 
-#ifdef _GNU_SOURCE
+#if defined(__GLIBC__)  (__GLIBC__  2 || (__GLIBC__ == 2  __GLIBC_MINOR__ 
= 12))
 if (name_threads) {
 pthread_setname_np(thread-thread, name);
 }
-- 
1.8.1.1.298.ge7eed54



[Qemu-devel] [PATCH] gtk: Add mouse wheel support

2014-03-11 Thread Jan Kiszka
Hook into scroll-event to properly forward mouse wheel movements to the
guest, just like we already do in SDL.

Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---
 ui/gtk.c | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/ui/gtk.c b/ui/gtk.c
index 992f7d7..016804d 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -683,6 +683,27 @@ static gboolean gd_button_event(GtkWidget *widget, 
GdkEventButton *button,
 return TRUE;
 }
 
+static gboolean gd_scroll_event(GtkWidget *widget, GdkEventScroll *scroll,
+void *opaque)
+{
+GtkDisplayState *s = opaque;
+InputButton btn;
+
+if (scroll-direction == GDK_SCROLL_UP) {
+btn = INPUT_BUTTON_WHEEL_UP;
+} else if (scroll-direction == GDK_SCROLL_DOWN) {
+btn = INPUT_BUTTON_WHEEL_DOWN;
+} else {
+return TRUE;
+}
+
+qemu_input_queue_btn(s-dcl.con, btn, true);
+qemu_input_event_sync();
+qemu_input_queue_btn(s-dcl.con, btn, false);
+qemu_input_event_sync();
+return TRUE;
+}
+
 static gboolean gd_key_event(GtkWidget *widget, GdkEventKey *key, void *opaque)
 {
 GtkDisplayState *s = opaque;
@@ -1229,6 +1250,8 @@ static void gd_connect_signals(GtkDisplayState *s)
  G_CALLBACK(gd_button_event), s);
 g_signal_connect(s-drawing_area, button-release-event,
  G_CALLBACK(gd_button_event), s);
+g_signal_connect(s-drawing_area, scroll-event,
+ G_CALLBACK(gd_scroll_event), s);
 g_signal_connect(s-drawing_area, key-press-event,
  G_CALLBACK(gd_key_event), s);
 g_signal_connect(s-drawing_area, key-release-event,
-- 
1.8.1.1.298.ge7eed54



Re: [Qemu-devel] [PATCH] hw/arm/musicpal: Remove nonexistent CDTP2, CDTP3 registers

2014-02-20 Thread Jan Kiszka
On 2014-02-18 16:28, Peter Maydell wrote:
 The ethernet device in the musicpal only has two tx queues,
 but we modelled it with four CTDP registers, presumably a
 cut and paste from the rx queue registers. Since the tx_queue[]
 array is only 2 entries long this allowed a guest to overrun
 this buffer. Remove the nonexistent registers.
 
 Signed-off-by: Peter Maydell peter.mayd...@linaro.org

Acked-by: Jan Kiszka jan.kis...@web.de

 ---
 There's no readily available documentation for this SoC,
 but I'm told the BSP for it indicates that there are
 indeed only two tx queues.
 
  hw/arm/musicpal.c | 6 ++
  1 file changed, 2 insertions(+), 4 deletions(-)
 
 diff --git a/hw/arm/musicpal.c b/hw/arm/musicpal.c
 index 023e875..a8d0086 100644
 --- a/hw/arm/musicpal.c
 +++ b/hw/arm/musicpal.c
 @@ -92,8 +92,6 @@
  #define MP_ETH_CRDP30x4AC
  #define MP_ETH_CTDP00x4E0
  #define MP_ETH_CTDP10x4E4
 -#define MP_ETH_CTDP20x4E8
 -#define MP_ETH_CTDP30x4EC
  
  /* MII PHY access */
  #define MP_ETH_SMIR_DATA0x
 @@ -308,7 +306,7 @@ static uint64_t mv88w8618_eth_read(void *opaque, hwaddr 
 offset,
  case MP_ETH_CRDP0 ... MP_ETH_CRDP3:
  return s-rx_queue[(offset - MP_ETH_CRDP0)/4];
  
 -case MP_ETH_CTDP0 ... MP_ETH_CTDP3:
 +case MP_ETH_CTDP0 ... MP_ETH_CTDP1:
  return s-tx_queue[(offset - MP_ETH_CTDP0)/4];
  
  default:
 @@ -362,7 +360,7 @@ static void mv88w8618_eth_write(void *opaque, hwaddr 
 offset,
  s-cur_rx[(offset - MP_ETH_CRDP0)/4] = value;
  break;
  
 -case MP_ETH_CTDP0 ... MP_ETH_CTDP3:
 +case MP_ETH_CTDP0 ... MP_ETH_CTDP1:
  s-tx_queue[(offset - MP_ETH_CTDP0)/4] = value;
  break;
  }
 




signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] Looking for project ideas and mentors for Google Summer of Code 2014

2014-02-14 Thread Jan Kiszka
On 2014-02-11 11:17, Stefan Hajnoczi wrote:
 On Mon, Feb 3, 2014 at 8:45 AM, Stefan Hajnoczi stefa...@gmail.com wrote:
 Project ideas
 Please post project ideas on the wiki page below.  Project ideas
 should be suitable as a 12-week project that a student fluent in
 C/Python/etc can complete.  No prior knowledge of QEMU/KVM/libvirt
 internals can be assumed.

 http://qemu-project.org/Google_Summer_of_Code_2014
 
 Please post your project ideas before Friday.
 
 I need to submit our organization application (including our project
 ideas) on Friday.

Hope it's not too late: just added the VT-d emulation proposal.

Jan




signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH 7/8] roms: update vgabios binaries

2014-01-05 Thread Jan Kiszka
On 2013-12-06 10:35, Gerd Hoffmann wrote:
 This also switches from lgplvgabios to seavgabios.
 
 Signed-off-by: Gerd Hoffmann kra...@redhat.com
 ---
  pc-bios/vgabios-cirrus.bin | Bin 35840 - 36864 bytes
  pc-bios/vgabios-qxl.bin| Bin 40448 - 37376 bytes
  pc-bios/vgabios-stdvga.bin | Bin 40448 - 37376 bytes
  pc-bios/vgabios-vmware.bin | Bin 40448 - 37376 bytes
  pc-bios/vgabios.bin| Bin 40448 - 36864 bytes
  5 files changed, 0 insertions(+), 0 deletions(-)
 

This update breaks a 64-bit win7 guest for me. It now spins infinitely
in the Starting Windows screen. I'm using stdvga. Is this a known issue?

Jan



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH 7/8] roms: update vgabios binaries

2014-01-05 Thread Jan Kiszka
On 2014-01-05 12:26, Paolo Bonzini wrote:
 Il 05/01/2014 10:35, Jan Kiszka ha scritto:
 On 2013-12-06 10:35, Gerd Hoffmann wrote:
 This also switches from lgplvgabios to seavgabios.

 Signed-off-by: Gerd Hoffmann kra...@redhat.com --- 
 pc-bios/vgabios-cirrus.bin | Bin 35840 - 36864 bytes 
 pc-bios/vgabios-qxl.bin| Bin 40448 - 37376 bytes 
 pc-bios/vgabios-stdvga.bin | Bin 40448 - 37376 bytes 
 pc-bios/vgabios-vmware.bin | Bin 40448 - 37376 bytes 
 pc-bios/vgabios.bin| Bin 40448 - 36864 bytes 5 files
 changed, 0 insertions(+), 0 deletions(-)

 
 This update breaks a 64-bit win7 guest for me. It now spins
 infinitely in the Starting Windows screen. I'm using stdvga. Is
 this a known issue?
 
 Yes, and IIRC it's fixed in seabios.git.

(Hmm, cd roms; make seavgabios doesn't work with out-of-tree builds)

Confirmed, win7 boots fine again with current seabios.git master. So
please update qemu.

Jan



signature.asc
Description: OpenPGP digital signature


[Qemu-devel] [PATCH uq/master] kvm: x86: Separately write feature control MSR on reset

2013-12-17 Thread Jan Kiszka
If the guest is running in nested mode on system reset, clearing the
feature MSR signals the kernel to leave this mode. Recent kernels
processes this properly, but leave the VCPU state undefined behind. It
is the job of userspace to bring it to a proper shape. Therefore, write
this specific MSR first so that no state transfer gets lost.

This allows to cleanly reset a guest with VMX in use.

Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---
 target-i386/kvm.c | 32 
 1 file changed, 28 insertions(+), 4 deletions(-)

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 1188482..ec51447 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -1104,6 +1104,25 @@ static int kvm_put_tscdeadline_msr(X86CPU *cpu)
 return kvm_vcpu_ioctl(CPU(cpu), KVM_SET_MSRS, msr_data);
 }
 
+/*
+ * Provide a separate write service for the feature control MSR in order to
+ * kick the VCPU out of VMXON or even guest mode on reset. This has to be done
+ * before writing any other state because forcibly leaving nested mode
+ * invalidates the VCPU state.
+ */
+static int kvm_put_msr_feature_control(X86CPU *cpu)
+{
+struct {
+struct kvm_msrs info;
+struct kvm_msr_entry entry;
+} msr_data;
+
+kvm_msr_entry_set(msr_data.entry, MSR_IA32_FEATURE_CONTROL,
+  cpu-env.msr_ia32_feature_control);
+msr_data.info.nmsrs = 1;
+return kvm_vcpu_ioctl(CPU(cpu), KVM_SET_MSRS, msr_data);
+}
+
 static int kvm_put_msrs(X86CPU *cpu, int level)
 {
 CPUX86State *env = cpu-env;
@@ -1204,10 +1223,8 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
 if (cpu-hyperv_vapic) {
 kvm_msr_entry_set(msrs[n++], HV_X64_MSR_APIC_ASSIST_PAGE, 0);
 }
-if (has_msr_feature_control) {
-kvm_msr_entry_set(msrs[n++], MSR_IA32_FEATURE_CONTROL,
-  env-msr_ia32_feature_control);
-}
+/* Note: MSR_IA32_FEATURE_CONTROL is written separately, see
+ *   kvm_put_msr_feature_control. */
 }
 if (env-mcg_cap) {
 int i;
@@ -1801,6 +1818,13 @@ int kvm_arch_put_registers(CPUState *cpu, int level)
 
 assert(cpu_is_stopped(cpu) || qemu_cpu_is_self(cpu));
 
+if (level = KVM_PUT_RESET_STATE  has_msr_feature_control) {
+ret = kvm_put_msr_feature_control(x86_cpu);
+if (ret  0) {
+return ret;
+}
+}
+
 ret = kvm_getput_regs(x86_cpu, 1);
 if (ret  0) {
 return ret;
-- 
1.8.1.1.298.ge7eed54



Re: [Qemu-devel] [PATCH v3 4/6] qemu-option: support +foo/-foo command line agruments

2013-11-11 Thread Jan Kiszka
On 2013-11-11 13:41, Andreas Färber wrote:
 Am 11.11.2013 08:44, schrieb Alexey Kardashevskiy:
 This converts +foo/-foo to foo=on/foo=off respectively when
 QEMU parser is used for the command line options.

 -cpu parsers in x86 and other architectures should be unaffected
 by this change.

 Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru
 ---
  util/qemu-option.c | 6 ++
  1 file changed, 6 insertions(+)

 diff --git a/util/qemu-option.c b/util/qemu-option.c
 index efcb5dc..6c8667c 100644
 --- a/util/qemu-option.c
 +++ b/util/qemu-option.c
 @@ -890,6 +890,12 @@ static int opts_do_parse(QemuOpts *opts, const char 
 *params,
  if (strncmp(option, no, 2) == 0) {
  memmove(option, option+2, strlen(option+2)+1);
  pstrcpy(value, sizeof(value), off);
 +} else if (strncmp(option, -, 1) == 0) {
 +memmove(option, option+1, strlen(option+1)+1);
 +pstrcpy(value, sizeof(value), off);
 +} else if (strncmp(option, +, 1) == 0) {
 +memmove(option, option+1, strlen(option+1)+1);
 +pstrcpy(value, sizeof(value), on);
  } else {
  pstrcpy(value, sizeof(value), on);
  }
 
 This looks like an interesting idea! However this is much too big a
 change to just CC ppc folks on...
 
 Jan, I wonder if this might break slirp's hostfwd option?

hostfwd starts with : in the simplest case - or what pattern do you
have in mind?

Jan

 
 Not sure what other options potentially starting with '-' might be
 affected. Test cases would be a helpful way of demonstrating that this
 change does not have undesired side effects.
 
 Regards,
 Andreas
 

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux



[Qemu-devel] [PATCH] gtk: Allow to activate grab-on-hover from the command line

2013-11-09 Thread Jan Kiszka
From: Jan Kiszka jan.kis...@siemens.com

As long as we have no persistent GTK configuration, this allows to
enable the useful grab-on-hover feature already when starting the VM.

Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---
 include/ui/console.h |  2 +-
 qemu-options.hx  |  5 +
 ui/gtk.c |  5 -
 vl.c | 22 +-
 4 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/include/ui/console.h b/include/ui/console.h
index 98edf41..13420c1 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -339,6 +339,6 @@ int index_from_keycode(int code);
 
 /* gtk.c */
 void early_gtk_display_init(void);
-void gtk_display_init(DisplayState *ds, bool full_screen);
+void gtk_display_init(DisplayState *ds, bool full_screen, bool grab_on_hover);
 
 #endif
diff --git a/qemu-options.hx b/qemu-options.hx
index 5dc8b75..2cb11b5 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -809,6 +809,7 @@ ETEXI
 DEF(display, HAS_ARG, QEMU_OPTION_display,
 -display sdl[,frame=on|off][,alt_grab=on|off][,ctrl_grab=on|off]\n
 [,window_close=on|off]|curses|none|\n
+gtk[,grab_on_hover=on|off]|\n
 vnc=display[,optargs]\n
 select display type\n, QEMU_ARCH_ALL)
 STEXI
@@ -832,6 +833,10 @@ graphics card, but its output will not be displayed to the 
QEMU
 user. This option differs from the -nographic option in that it
 only affects what is done with video output; -nographic also changes
 the destination of the serial and parallel port data.
+@item gtk
+Display video output in a GTK window. This interface provides drop-down
+menus and other UI elements to configure and control the VM during
+runtime.
 @item vnc
 Start a VNC server on display arg
 @end table
diff --git a/ui/gtk.c b/ui/gtk.c
index b5f4f0b..43883b7 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -1443,7 +1443,7 @@ static const DisplayChangeListenerOps dcl_ops = {
 .dpy_cursor_define = gd_cursor_define,
 };
 
-void gtk_display_init(DisplayState *ds, bool full_screen)
+void gtk_display_init(DisplayState *ds, bool full_screen, bool grab_on_hover)
 {
 GtkDisplayState *s = g_malloc0(sizeof(*s));
 char *filename;
@@ -1522,6 +1522,9 @@ void gtk_display_init(DisplayState *ds, bool full_screen)
 if (full_screen) {
 gtk_menu_item_activate(GTK_MENU_ITEM(s-full_screen_item));
 }
+if (grab_on_hover) {
+gtk_menu_item_activate(GTK_MENU_ITEM(s-grab_on_hover_item));
+}
 
 register_displaychangelistener(s-dcl);
 
diff --git a/vl.c b/vl.c
index 4ad15b8..7b1b02b 100644
--- a/vl.c
+++ b/vl.c
@@ -201,6 +201,7 @@ int vga_interface_type = VGA_NONE;
 static int full_screen = 0;
 static int no_frame = 0;
 int no_quit = 0;
+static bool grab_on_hover;
 CharDriverState *serial_hds[MAX_SERIAL_PORTS];
 CharDriverState *parallel_hds[MAX_PARALLEL_PORTS];
 CharDriverState *virtcon_hds[MAX_VIRTIO_CONSOLES];
@@ -,6 +2223,25 @@ static DisplayType select_display(const char *p)
 } else if (strstart(p, gtk, opts)) {
 #ifdef CONFIG_GTK
 display = DT_GTK;
+while (*opts) {
+const char *nextopt;
+
+if (strstart(opts, ,grab_on_hover=, nextopt)) {
+opts = nextopt;
+if (strstart(opts, on, nextopt)) {
+grab_on_hover = true;
+} else if (strstart(opts, off, nextopt)) {
+grab_on_hover = false;
+} else {
+goto invalid_gtk_args;
+}
+} else {
+invalid_gtk_args:
+fprintf(stderr, Invalid GTK option string: %s\n, p);
+exit(1);
+}
+opts = nextopt;
+}
 #else
 fprintf(stderr, GTK support is disabled\n);
 exit(1);
@@ -4284,7 +4304,7 @@ int main(int argc, char **argv, char **envp)
 #endif
 #if defined(CONFIG_GTK)
 case DT_GTK:
-gtk_display_init(ds, full_screen);
+gtk_display_init(ds, full_screen, grab_on_hover);
 break;
 #endif
 default:
-- 
1.8.1.1.298.ge7eed54



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH 1/7] usb: remove old usb-host code

2013-11-08 Thread Jan Kiszka
On 2013-10-09 13:42, Gerd Hoffmann wrote:
   Hi,
 
 Only very recent distros fulfill the need of = 1.0.13, so you naturally
 fall back to this code. I just realized that even the factory build of
 OpenSUSE is still on libusb-1.0.9. Current Ubuntu versions are on 1.0.12
 at best. Didn't check others so far.

 Ouch.  The 1.0.13 release is one year old by now.

 Fedora 19 is at 1.0.16 btw.

 So isn't this step a bit too early?

 There is always the 'git revert' option in case it turns out there are
 too many issues ...

 So what to do? Do you expect all the other distros to catch up regarding
 libusb until QEMU 1.7 is released?
 
 They will update once they figure this is needed for qemu 1.7 :)
 Updates will probably not yet be available at release time though.
 
 If we revert, thereby continue fallback to the old code, chances are
 high that nothing happens and we'll face the same issue for qemu 1.8.
 
 Given that there are some known issues in the host-linux code which are
 fixed in host-libusb I really want get rid of it.

OK, then here is the first issue I ran into while trying libusbx (git
head, i.e. 1.0.17+: The new stack causes significant latency issues that
makes it almost unusable for pass-through of USB audio devices (some
headset in my case). Reverting usb-linux and disabling libusb over QEMU
git head makes things work again. I'll have to stick with this for now
as it is affecting my work environment.

Any spontaneous ideas how to analyse or even resolve this?

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] [PATCH 1/7] usb: remove old usb-host code

2013-11-08 Thread Jan Kiszka
On 2013-11-08 16:39, Gerd Hoffmann wrote:
   Hi,
 
 OK, then here is the first issue I ran into while trying libusbx (git
 head, i.e. 1.0.17+: The new stack causes significant latency issues that
 makes it almost unusable for pass-through of USB audio devices (some
 headset in my case). Reverting usb-linux and disabling libusb over QEMU
 git head makes things work again. I'll have to stick with this for now
 as it is affecting my work environment.

 Any spontaneous ideas how to analyse or even resolve this?
 
 Try setting isobsize property to something smaller than 32 (which is the
 default).

OK, isobsize=2 and isobufs=32 helped, possibly other combinations as
well - but not just reducing isobsize or increasing isobufs. Any theory
about this? How can we find better defaults?

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] [PATCH] kvm: Add a new machine property kvm_type

2013-11-06 Thread Jan Kiszka
On 2013-11-06 16:08, Aneesh Kumar K.V wrote:
 Jan Kiszka jan.kis...@siemens.com writes:
 
 On 2013-10-07 18:53, Aneesh Kumar K.V wrote:
 From: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com

 Targets like ppc64 support different typed of KVM, one which use
 hypervisor mode and the other which doesn't. Add a new machine
 property kvm_type that helps in selecting the respective ones
 We also add a new QEMUMachine callback get_vm_type that helps
 in mapping the string representation of kvm type specified.

 Signed-off-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com

 ...

 diff --git a/vl.c b/vl.c
 index 4e709d5..7ecc581 100644
 --- a/vl.c
 +++ b/vl.c
 @@ -427,7 +427,12 @@ static QemuOptsList qemu_machine_opts = {
  .name = usb,
  .type = QEMU_OPT_BOOL,
  .help = Set on/off to enable/disable usb,
 +},{
 +.name = kvm_type,
 +.type = QEMU_OPT_STRING,
 +.help = Set to kvm type to be used in create vm ioctl,

 This does not sound like an appropriate documentation for an enduser.

 
 Any other suggestion for that ? It is actually a string which will be
 parsed by the machine opt callback, and the return value is used as the
 argument to create vm ioctl. 

Maybe something like this: Specifies the KVM virtualization mode (xxx,
yyy, whatever)

No user is interesting in how this information gets transfered to the
KVM core. That's an irrelavant implementation detail.

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] USB Passthrough not working for Windows 7 guest

2013-11-05 Thread Jan Kiszka
On 2013-11-05 17:01, Frederich, Jens wrote:
 Hi all,
 
 we're currently evaluating different RTOS systems (Windows CE, Intime, RTX, 
 etc.).
 One system is Linux RT + KVM/QEMU with a Windows 7 guest. Up to now all
 works fine, Linux RT has good latency and KVM/Qemu setup was easy. But one 
 QEMU bug
 breaks my measurement setup and evaluation.
 
 I've some usb devices for the Windows 7 guest. I configure them as USB 
 passthrough.
 The devices appears in the device manager of Windows 7, but with
 Error code 10: device cannot start. The Windows driver fails on USB set 
 configuration.
 The driver creates a IRP and send it via IOCTRL to lower layer. The IOCTRL 
 fails with
 invalid parameter.
 
 driver log:
 0009  0.65470564  vnCDrvUsbControlRequestSetConfiguration, 
 WdfUsbTargetDeviceSelectConfig single interface failed 0xc00d  
 0010  0.65472370  vnCDrvUsbIFPrepareHardwareState, 
 vnCDrvUsbControlRequestSetConfiguration failed: 0xc00d 
 0011  0.65473646  vnCDrvDevConPrepareHardware, 
 vnCDrvUsbIFPrepareHardwareState failed 0xc00d  
 0012  0.65474838  vnCDrvEvtDevicePrepareHardware, 
 vnCDrvDevConPrepareHardware failed 0xc001   
 0013  0.6547
 
 This bug breaks my latency measurement setup and Linux RT is out of the 
 evaluationg
 race. Windows CE should not win :-), it there anyway workaround or hack to 
 fix the issue?

Workaround: Pass-through one of the (typically) many USB host
controllers to the Windows guest (vfio or classic pci-assign). I did
this back then when *HCI emulation was still pretty immature.

But USB device pass-through should also work. Do you happen to pass a
USB 2.0 device via an emulated UHCI? Or are you already using the EHCI
emulation? In the latter case, activating USB tracing (see also
qemu/docs/tracing.txt) and posting the results here may help analysing
the issue.

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] USB Passthrough not working for Windows 7 guest

2013-11-05 Thread Jan Kiszka
On 2013-11-05 21:20, Jens Frederich wrote:
 On 2013-11-05 17:01, Frederich, Jens wrote:
 Hi all,

 we're currently evaluating different RTOS systems (Windows CE, Intime, RTX,
 etc.).
 One system is Linux RT + KVM/QEMU with a Windows 7 guest. Up to now all
 works fine, Linux RT has good latency and KVM/Qemu setup was easy. But one
 QEMU bug
 breaks my measurement setup and evaluation.

 I've some usb devices for the Windows 7 guest. I configure them as USB
 passthrough.
 The devices appears in the device manager of Windows 7, but with
 Error code 10: device cannot start. The Windows driver fails on USB set
 configuration.
 The driver creates a IRP and send it via IOCTRL to lower layer. The IOCTRL
 fails with
 invalid parameter.

 driver log:
 0009  0.65470564  vnCDrvUsbControlRequestSetConfiguration,
 WdfUsbTargetDeviceSelectConfig single interface failed 0xc00d
 0010  0.65472370  vnCDrvUsbIFPrepareHardwareState,
 vnCDrvUsbControlRequestSetConfiguration failed: 0xc00d
 0011  0.65473646  vnCDrvDevConPrepareHardware,
 vnCDrvUsbIFPrepareHardwareState failed 0xc00d
 0012  0.65474838  vnCDrvEvtDevicePrepareHardware,
 vnCDrvDevConPrepareHardware failed 0xc001
 0013  0.6547

 This bug breaks my latency measurement setup and Linux RT is out of the
 evaluationg
 race. Windows CE should not win :-), it there anyway workaround or hack to
 fix the issue?

 Workaround: Pass-through one of the (typically) many USB host
 controllers to the Windows guest (vfio or classic pci-assign). I did
 this back then when *HCI emulation was still pretty immature.

 But USB device pass-through should also work. Do you happen to pass a
 USB 2.0 device via an emulated UHCI? Or are you already using the EHCI
 emulation?
 
 I'm not sure which mode it has been. I've used the virt-manager to configure
 the device. A usb controller is already configured in mode 'default'.
 My steps on virt-manager:
 
 1. add hardware
 2. select usb host device
 3. I can see my usb device, I select it
 4. start guest and open Windows device manager

Unfortunately, I do not know what virt-manager is configuring by
default. It likely also depends on its version, so you should share this
information as well. Maybe other folks here can comment on this.

 
 I don't know is this UHCI or EHCI? On the usb host device list are
 some controller listed e.q. xhci, ehci and so on. Should I map these
 controller to Windows 7 as well?

Pick the host controller to which the USB device you want to give to the
guest is attached to (lsusb and the bus number reported via
/sys/bus/pci/devices/id/usbX can tell this - or trial and error). When
doing this, you no longer need to pass through the USB device itself, it
is implicitely passed.

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] [PATCH] kvm: Add a new machine property kvm_type

2013-11-04 Thread Jan Kiszka
On 2013-10-07 18:53, Aneesh Kumar K.V wrote:
 From: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
 
 Targets like ppc64 support different typed of KVM, one which use
 hypervisor mode and the other which doesn't. Add a new machine
 property kvm_type that helps in selecting the respective ones
 We also add a new QEMUMachine callback get_vm_type that helps
 in mapping the string representation of kvm type specified.
 
 Signed-off-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com

...

 diff --git a/vl.c b/vl.c
 index 4e709d5..7ecc581 100644
 --- a/vl.c
 +++ b/vl.c
 @@ -427,7 +427,12 @@ static QemuOptsList qemu_machine_opts = {
  .name = usb,
  .type = QEMU_OPT_BOOL,
  .help = Set on/off to enable/disable usb,
 +},{
 +.name = kvm_type,
 +.type = QEMU_OPT_STRING,
 +.help = Set to kvm type to be used in create vm ioctl,

This does not sound like an appropriate documentation for an enduser.

OTOH, I do not recall right now how these help strings can be obtained
at all. One could intuitively try -machine sometype,?, but that's
not implemented. Anyone?

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] [PATCH] rtc: remove dead SQW IRQ code

2013-11-04 Thread Jan Kiszka
On 2013-08-14 13:29, Jan Kiszka wrote:
 This was once introduced by commit 100d9891d6 but was never used in-tree
 and then got broken by commit 32e0c8260d. Time to clean up.
 
 Signed-off-by: Jan Kiszka jan.kis...@siemens.com
 ---
  hw/timer/mc146818rtc.c |9 +
  1 files changed, 1 insertions(+), 8 deletions(-)
 
 diff --git a/hw/timer/mc146818rtc.c b/hw/timer/mc146818rtc.c
 index 3c3baac..b3f1baa 100644
 --- a/hw/timer/mc146818rtc.c
 +++ b/hw/timer/mc146818rtc.c
 @@ -70,7 +70,6 @@ typedef struct RTCState {
  uint64_t last_update;
  int64_t offset;
  qemu_irq irq;
 -qemu_irq sqw_irq;
  int it_shift;
  /* periodic timer */
  QEMUTimer *periodic_timer;
 @@ -151,8 +150,7 @@ static void periodic_timer_update(RTCState *s, int64_t 
 current_time)
  
  period_code = s-cmos_data[RTC_REG_A]  0x0f;
  if (period_code != 0
 - ((s-cmos_data[RTC_REG_B]  REG_B_PIE)
 -|| ((s-cmos_data[RTC_REG_B]  REG_B_SQWE)  s-sqw_irq))) {
 + (s-cmos_data[RTC_REG_B]  REG_B_PIE)) {
  if (period_code = 2)
  period_code += 7;
  /* period in 32 Khz cycles */
 @@ -202,11 +200,6 @@ static void rtc_periodic_timer(void *opaque)
  #endif
  qemu_irq_raise(s-irq);
  }
 -if (s-cmos_data[RTC_REG_B]  REG_B_SQWE) {
 -/* Not square wave at all but we don't want 2048Hz interrupts!
 -   Must be seen as a pulse.  */
 -qemu_irq_raise(s-sqw_irq);
 -}
  }
  
  /* handle update-ended timer */
 

ping

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] [PATCH] kvm: Add a new machine property kvm_type

2013-11-04 Thread Jan Kiszka
On 2013-11-04 14:30, Alexander Graf wrote:
 
 On 04.11.2013, at 14:28, Jan Kiszka jan.kis...@siemens.com wrote:
 
 On 2013-10-07 18:53, Aneesh Kumar K.V wrote:
 From: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com

 Targets like ppc64 support different typed of KVM, one which use
 hypervisor mode and the other which doesn't. Add a new machine
 property kvm_type that helps in selecting the respective ones
 We also add a new QEMUMachine callback get_vm_type that helps
 in mapping the string representation of kvm type specified.

 Signed-off-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com

 ...

 diff --git a/vl.c b/vl.c
 index 4e709d5..7ecc581 100644
 --- a/vl.c
 +++ b/vl.c
 @@ -427,7 +427,12 @@ static QemuOptsList qemu_machine_opts = {
 .name = usb,
 .type = QEMU_OPT_BOOL,
 .help = Set on/off to enable/disable usb,
 +},{
 +.name = kvm_type,
 +.type = QEMU_OPT_STRING,
 +.help = Set to kvm type to be used in create vm ioctl,

 This does not sound like an appropriate documentation for an enduser.

 OTOH, I do not recall right now how these help strings can be obtained
 at all. One could intuitively try -machine sometype,?, but that's
 not implemented. Anyone?
 
 They should definitely show up in the man page. But yes, -machine kvm_type=? 
 would be helpful as well.

The man page is not generate from this C code, is it?

 
 As for -machine ? we are have a problem orthogonal to this patch. I agree 
 that we need some way to programmatically list all machine options.

I'm not saying that this is an issue of this patch, just that I wonder
how that .help message can be made visisble at at besides with the help
of an editor.

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux



[Qemu-devel] [PATCH uq/master] pci-assign: Remove dead code for direct I/O region access from userspace

2013-11-04 Thread Jan Kiszka
This feature was already deprecated back then in qemu-kvm, ie. before
pci-assign went upstream. assigned_dev_ioport_rw will never be invoked
with resource_fd  0.

Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---
 hw/i386/kvm/pci-assign.c | 56 +---
 1 file changed, 10 insertions(+), 46 deletions(-)

diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c
index 011764f..4e65110 100644
--- a/hw/i386/kvm/pci-assign.c
+++ b/hw/i386/kvm/pci-assign.c
@@ -154,55 +154,19 @@ static uint64_t assigned_dev_ioport_rw(AssignedDevRegion 
*dev_region,
 uint64_t val = 0;
 int fd = dev_region-region-resource_fd;
 
-if (fd = 0) {
-if (data) {
-DEBUG(pwrite data=% PRIx64 , size=%d, e_phys= TARGET_FMT_plx
-  , addr=TARGET_FMT_plx\n, *data, size, addr, addr);
-if (pwrite(fd, data, size, addr) != size) {
-error_report(%s - pwrite failed %s,
- __func__, strerror(errno));
-}
-} else {
-if (pread(fd, val, size, addr) != size) {
-error_report(%s - pread failed %s,
- __func__, strerror(errno));
-val = (1UL  (size * 8)) - 1;
-}
-DEBUG(pread val=% PRIx64 , size=%d, e_phys= TARGET_FMT_plx
-  , addr= TARGET_FMT_plx \n, val, size, addr, addr);
+if (data) {
+DEBUG(pwrite data=% PRIx64 , size=%d, e_phys= TARGET_FMT_plx
+  , addr=TARGET_FMT_plx\n, *data, size, addr, addr);
+if (pwrite(fd, data, size, addr) != size) {
+error_report(%s - pwrite failed %s, __func__, strerror(errno));
 }
 } else {
-uint32_t port = addr + dev_region-u.r_baseport;
-
-if (data) {
-DEBUG(out data=% PRIx64 , size=%d, e_phys= TARGET_FMT_plx
-  , host=%x\n, *data, size, addr, port);
-switch (size) {
-case 1:
-outb(*data, port);
-break;
-case 2:
-outw(*data, port);
-break;
-case 4:
-outl(*data, port);
-break;
-}
-} else {
-switch (size) {
-case 1:
-val = inb(port);
-break;
-case 2:
-val = inw(port);
-break;
-case 4:
-val = inl(port);
-break;
-}
-DEBUG(in data=% PRIx64 , size=%d, e_phys= TARGET_FMT_plx
-  , host=%x\n, val, size, addr, port);
+if (pread(fd, val, size, addr) != size) {
+error_report(%s - pread failed %s, __func__, strerror(errno));
+val = (1UL  (size * 8)) - 1;
 }
+DEBUG(pread val=% PRIx64 , size=%d, e_phys= TARGET_FMT_plx
+  , addr= TARGET_FMT_plx \n, val, size, addr, addr);
 }
 return val;
 }
-- 
1.8.1.1.298.ge7eed54



Re: [Qemu-devel] [PATCH] qemu: Broken -smb with latest SAMBA package. (Unsupported security=share option)

2013-11-04 Thread Jan Kiszka
On 2013-11-04 14:55, Michael Tokarev wrote:
 04.11.2013 00:06, Jan Kiszka wrote:
 On 2013-11-01 11:10, Michael Tokarev wrote:
 []
 If Jan picks it up, that's fine.  If not, I think it can go
 to the trivial patches queue.

 Works fine, applied to queues/slirp.
 
 Okay, thank you Jan.
 
 But this is not a trivial patch as the fix is not obvious for a reader
 (unless you know smb.conf semantics by heart).
 
 It's trivial for my understanding.  If we require that every
 change going to -trivial should be obvious to everyone, we
 should just close it right away.

Then we may need -less-trivial, because - to my understanding - -trivial
was once set up according to the rule that (most) QEMU hackers should be
able to understand that a trivial change is at least mostly harmless.

 And this area does not have an active maintainer anyway, at least
 according to MAINTAINERS and ./scripts/get_maintainer.pl.

Yeah, I think we have some holes there. That smb configuration
conceptually belongs to slirp is right, just not clear documented in our
script.

Jan




signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH] qemu: Broken -smb with latest SAMBA package. (Unsupported security=share option)

2013-11-03 Thread Jan Kiszka
On 2013-11-01 11:10, Michael Tokarev wrote:
 01.11.2013 13:54, Michael Büsch wrote:
 On Fri, 01 Nov 2013 13:32:49 +0400
 Michael Tokarev m...@tls.msk.ru wrote:

 That looks right.  Are you okay adding your Signed-off-by to the patch
 you initially submitted?  If yes, I'll make a formal patch submission
 upstream.

 Here you go.
 
 Thank you!
 
 Adding Jan as slirp maintainer, and my
 
 Reviewed-by: Michael Tokarev m...@tls.msk.ru
 
 If Jan picks it up, that's fine.  If not, I think it can go
 to the trivial patches queue.

Works fine, applied to queues/slirp.

But this is not a trivial patch as the fix is not obvious for a reader
(unless you know smb.conf semantics by heart).

Jan




signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [Bug 1246890] [NEW] AC97 sound card crashes QEMU

2013-11-01 Thread Jan Kiszka
On 2013-10-31 21:48, John Arbuckle wrote:
 Public bug reported:
 
 The AC97 sound card does not work. It stops QEMU on startup. The cause
 appears to be some kind of deadlock.
 
 Steps to reproduce:
 Just add -soundhw ac97 to QEMU's arguments. Example: qemu-system-ppc -soundhw 
 ac97
 
 The example above is all it takes to reproduce the problem.
 
 This problem has been observed on Mac OS X and Debian Linux.
 
 I question whether the ac97 support ever worked. It is a file that was
 taken from VirtualBox and added to QEMU. I do know that VirtualBox's
 support for the ac97 sound card works perfectly.
 
 The exact line of code that stops QEMU in its tracks is located in the
 file main-loop.c, in the function os_host_main_loop_wait(), the call
 made to qemu_mutex_lock_iothread(). The is where QEMU stops under Mac OS
 X.
 
 ** Affects: qemu
  Importance: Undecided
  Status: New
 

Maybe this is just a regression: I'm using ac97 for a win7 guest for a
while, and since recently (don't recall when precisely, some weeks
maybe) I'm getting main-loop: WARNING: I/O thread spun for 1000
iterations, a temporarily stuck guest and broken sound output. This
used to work fine. Someone has to bisect, I didn't find the time yet.

Jan



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH v2] cpu-exec: Fix compiler warning (-Werror=clobbered)

2013-10-31 Thread Jan Kiszka
On 2013-10-31 20:31, Stefan Weil wrote:
 Reloading of local variables after sigsetjmp is only needed for some
 buggy compilers.
 
 The code which should reload these variables causes compiler warnings
 with gcc 4.7 when compiler optimizations are enabled:
 
 cpu-exec.c:204:15: error:
  variable ‘cpu’ might be clobbered by ‘longjmp’ or ‘vfork’ [-Werror=clobbered]
 cpu-exec.c:207:15: error:
  variable ‘cc’ might be clobbered by ‘longjmp’ or ‘vfork’ [-Werror=clobbered]
 cpu-exec.c:202:28: error:
  argument ‘env’ might be clobbered by ‘longjmp’ or ‘vfork’ [-Werror=clobbered]
 
 Now this code is only used for compilers which need it
 (and gcc 4.5.x, x  0 which does not need it but won't give warnings).
 
 There were bug reports for clang and gcc 4.5.0, while gcc 4.5.1
 was reported to work fine without the reload code.
 
 Signed-off-by: Stefan Weil s...@weilnetz.de
 ---
 
 v2: Don't remove the code which causes the warnings, but use it
 only with clang or gcc  4.6.
 
  cpu-exec.c |8 ++--
  1 file changed, 6 insertions(+), 2 deletions(-)
 
 diff --git a/cpu-exec.c b/cpu-exec.c
 index 30cfa2a..fec20c3 100644
 --- a/cpu-exec.c
 +++ b/cpu-exec.c
 @@ -677,14 +677,18 @@ int cpu_exec(CPUArchState *env)
 only be set by a memory fault) */
  } /* for(;;) */
  } else {
 -/* Reload env after longjmp - the compiler may have smashed all
 - * local variables as longjmp is marked 'noreturn'. */
 +#if defined(__clang__) || !QEMU_GNUC_PREREQ(4, 6)
 +/* Some compilers wrongly smash all local variables after
 + * siglongjmp. There were bug reports for gcc 4.5.0 and clang.
 + * Reload essential local variables here for those compilers.
 + * gcc 4.7 would complain about this code (-Wclobbered). */
  cpu = current_cpu;
  env = cpu-env_ptr;
  #if !(defined(CONFIG_USER_ONLY)  \
(defined(TARGET_M68K) || defined(TARGET_PPC) || defined(TARGET_S390X)))
  cc = CPU_GET_CLASS(cpu);
  #endif
 +#endif /* __clang__ or old gcc */
  }
  } /* for(;;) */
  
 

Are all clang versions affected? Then this looks reasonable.

Jan



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH] cpu-exec: Fix compiler warning (-Werror=clobbered)

2013-10-29 Thread Jan Kiszka
On 2013-10-28 20:18, Stefan Weil wrote:
 Am 18.09.2013 09:48, schrieb Jan Kiszka:
 On 2013-09-18 09:26, Peter Maydell wrote:
 [...]
 And gcc's documentation of the 'noreturn' attribute specifically
 says it does not affect the exceptional path where the function
 returns via longjmp.
 OK, that is the clarifying bit of information.

 Now the question is if want to drop support for faulty compilers again,
 work around the false-positive warning, or avoid the issue differently
 than via reloading.

 Jan
 
 Recently commit 6c78f29a2424622bfc9c30dfbbc13404481eacb6
 added a third variable which is reloaded now. Obviously the clang
 compiler needs this workaround.
 
 Jan, can you remember whether the initial problems were also
 caused by clang? If yes, we might restrict the code to that compiler.
 This would avoid the -Wclobbered warnings with newer gcc while
 still fixing the code generated by clang.

Look up this thread: gcc 4.5.0

Jan




signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH v4 06/12] fix qemu_alloc/qemu_free for slirp subsystem

2013-10-18 Thread Jan Kiszka
On 2009-06-18 22:50, Jean-Christophe DUBOIS wrote:
 From: Jean-Christophe Dubois jcd@jcd-laptop.(none)
 
 Signed-off-by: Jean-Christophe DUBOIS j...@tribudubois.net
 ---
  slirp/socket.c |8 
  1 files changed, 4 insertions(+), 4 deletions(-)
 
 diff --git a/slirp/socket.c b/slirp/socket.c
 index 82d026c..e4d84d7 100644
 --- a/slirp/socket.c
 +++ b/slirp/socket.c
 @@ -53,7 +53,7 @@ socreate(void)
  {
struct socket *so;
  
 -  so = (struct socket *)malloc(sizeof(struct socket));
 +  so = (struct socket *)qemu_malloc(sizeof(struct socket));
if(so) {

qemu_malloc doesn't return NULL. So you should clean up more here, and
possibly elsewhere.

Jan

  memset(so, 0, sizeof(struct socket));
  so-so_state = SS_NOFDREF;
 @@ -82,7 +82,7 @@ sofree(struct socket *so)
if(so-so_next  so-so_prev)
  remque(so);  /* crashes if so is not in a queue */
  
 -  free(so);
 +  qemu_free(so);
  }
  
  size_t sopreprbuf(struct socket *so, struct iovec *iov, int *np)
 @@ -606,13 +606,13 @@ solisten(u_int port, u_int32_t laddr, u_int lport, int 
 flags)
   DEBUG_ARG(flags = %x, flags);
  
   if ((so = socreate()) == NULL) {
 -   /* free(so);  Not sofree() ??? free(NULL) == NOP */
 +   /* qemu_free(so);  Not sofree() ??? qemu_free(NULL) == NOP */
 return NULL;
   }
  
   /* Don't tcp_attach... we don't need so_snd nor so_rcv */
   if ((so-so_tcpcb = tcp_newtcpcb(so)) == NULL) {
 - free(so);
 + qemu_free(so);
   return NULL;
   }
   insque(so,tcb);
 




signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH 1/7] usb: remove old usb-host code

2013-10-09 Thread Jan Kiszka
On 2013-10-07 09:16, Gerd Hoffmann wrote:
 On Mi, 2013-10-02 at 15:14 +0200, Jan Kiszka wrote:
 On 2013-09-19 11:34, Gerd Hoffmann wrote:
 The usb-host code has been rewritten for qemu 1.5 to use libusb,
 the old code has been left in as temporary fallback.  Now we are
 two releases further out, targeting the 1.7 release.  No major
 issues with the new code poped up until now.  Time to remove it
 from tre tree.  Should we ever need it again for some reason --
 git has a copy for us in the history.

 Well, maybe not many users were actually exploiting the new libusb code,
 thus were able to produce feedback.

 Only very recent distros fulfill the need of = 1.0.13, so you naturally
 fall back to this code. I just realized that even the factory build of
 OpenSUSE is still on libusb-1.0.9. Current Ubuntu versions are on 1.0.12
 at best. Didn't check others so far.
 
 Ouch.  The 1.0.13 release is one year old by now.
 
 Fedora 19 is at 1.0.16 btw.
 
 So isn't this step a bit too early?
 
 There is always the 'git revert' option in case it turns out there are
 too many issues ...

So what to do? Do you expect all the other distros to catch up regarding
libusb until QEMU 1.7 is released?

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] [PATCH] cpu-exec(): also reload CPUClass *cc after longjmp return

2013-10-04 Thread Jan Kiszka
On 2013-10-03 18:05, Peter Maydell wrote:
 On 3 October 2013 23:09, Juergen Lock qem...@jelal.kn-bremen.de wrote:
 Local variable CPUClass *cc needs to be reloaded after return from longjmp
 too.  (This fixes the mips-softmmu crash observed on FreeBSD when qemu is
 built with clang.)

 Signed-off-by: Juergen Lock n...@jelal.kn-bremen.de
 Found-by: Dimitry Andric d...@freebsd.org

 --- a/cpu-exec.c
 +++ b/cpu-exec.c
 @@ -681,6 +681,10 @@ int cpu_exec(CPUArchState *env)
   * local variables as longjmp is marked 'noreturn'. */
  cpu = current_cpu;
  env = cpu-env_ptr;
 +#if !(defined(CONFIG_USER_ONLY)  \
 +  (defined(TARGET_M68K) || defined(TARGET_PPC) || 
 defined(TARGET_S390X)))
 +cc = CPU_GET_CLASS(cpu);
 +#endif
 
 This is a c compiler or libc bug -- the C standard says that this
 local variable should not be trashed by the longjmp. We were
 actually discussing removing the current workarounds there...

But we didn't decide if we should stop supporting the affected compiler
versions.

Does this issue also exist with the latest clang version available for
your platform?

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] [PATCH 1/7] usb: remove old usb-host code

2013-10-02 Thread Jan Kiszka
On 2013-09-19 11:34, Gerd Hoffmann wrote:
 The usb-host code has been rewritten for qemu 1.5 to use libusb,
 the old code has been left in as temporary fallback.  Now we are
 two releases further out, targeting the 1.7 release.  No major
 issues with the new code poped up until now.  Time to remove it
 from tre tree.  Should we ever need it again for some reason --
 git has a copy for us in the history.

Well, maybe not many users were actually exploiting the new libusb code,
thus were able to produce feedback.

Only very recent distros fulfill the need of = 1.0.13, so you naturally
fall back to this code. I just realized that even the factory build of
OpenSUSE is still on libusb-1.0.9. Current Ubuntu versions are on 1.0.12
at best. Didn't check others so far.

So isn't this step a bit too early?

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux



<    1   2   3   4   5   6   7   8   9   10   >