Re: [Qemu-devel] [questions] about using vfio to assign sr-iov vf to vm

[Zhang Haoyu]

>> Hi, all
>> I'm using VFIO to assign intel 82599 VF to VM, now I encounter a problem,
>> 82599 PF and its VFs belong to the same iommu_group, but I only want to 
>> assign some VFs to one VM, and some other VFs to another VM, ...,
>> so how to only unbind (part of) the VFs but PF?
>> I read the kernel doc vfio.txt, I'm not sure should I unbind all of the 
>> devices which belong to one iommu_group?
>> If so, because PF and its VFs belong to the same iommu_group, if I unbind 
>> the PF, its VFs also diappeared.
>> I think I misunderstand someting,
>> any advises?
>
>This occurs when the PF is installed behind components in the system
>that do not support PCIe Access Control Services (ACS).  The IOMMU group
>contains both the PF and the VF because upstream transactions can be
>re-routed downstream by these non-ACS components before being translated
>by the IOMMU.  Please provide 'sudo lspci -vvv', 'lspci -n', and kernel
>version and we might be able to give you some advise on how to work
>around the problem.  Thanks,
>
# lspci | grep Ether
02:00.0 Ethernet controller: Intel Corporation 82599EB 10-Gigabit SFI/SFP+ 
Network Connection (rev 01)
02:00.1 Ethernet controller: Intel Corporation 82599EB 10-Gigabit SFI/SFP+ 
Network Connection (rev 01)
08:00.0 Ethernet controller: Intel Corporation 82576 Gigabit Network Connection 
(rev 01)
08:00.1 Ethernet controller: Intel Corporation 82576 Gigabit Network Connection 
(rev 01)
09:00.0 Ethernet controller: Intel Corporation 82576 Gigabit Network Connection 
(rev 01)
09:00.1 Ethernet controller: Intel Corporation 82576 Gigabit Network Connection 
(rev 01)
0a:00.0 Ethernet controller: Intel Corporation 82576 Gigabit Network Connection 
(rev 01)
0a:00.1 Ethernet controller: Intel Corporation 82576 Gigabit Network Connection 
(rev 01)
0b:00.0 Ethernet controller: Intel Corporation 82576 Gigabit Network Connection 
(rev 01)
0b:00.1 Ethernet controller: Intel Corporation 82576 Gigabit Network Connection 
(rev 01)
0c:00.0 Ethernet controller: Realtek Semiconductor Co., Ltd. RTL-8110SC/8169SC 
Gigabit Ethernet (rev 10)

I want to direct-assign the VFs of intel 82599(02:00.0 or 02:00.1) to VM,
# lspci -t
-[:00]-+-00.0
   +-01.0-[01]--
   +-01.1-[02-03]--+-00.0
   |  \-00.1
   +-02.0
   +-06.0-[04]--
   +-16.0
   +-1a.0
   +-1c.0-[05-0b]00.0-[06-0b]--+-04.0-[07]--
   |   +-05.0-[08]--+-00.0
   |   |  \-00.1
   |   +-06.0-[09]--+-00.0
   |   |  \-00.1
   |   +-08.0-[0a]--+-00.0
   |   |  \-00.1
   |   \-09.0-[0b]--+-00.0
   |  \-00.1
   +-1d.0
   +-1e.0-[0c]00.0
   +-1f.0
   +-1f.2
   \-1f.3

lspci -vvv -s 02.00.0
02:00.0 Ethernet controller: Intel Corporation 82599EB 10-Gigabit SFI/SFP+ 
Network Connection (rev 01)
Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- 
Stepping- SERR- FastB2B- DisINTx+
Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- SERR- TAbort- SERR- TAbort- Reset- FastB2B-
PriDiscTmr- SecDiscTmr- DiscTmrStat- DiscTmrSERREn-
Capabilities: [88] Subsystem: Intel Corporation Xeon E3-1200 v2/3rd Gen 
Core processor PCI Express Root Port
Capabilities: [80] Power Management version 3
Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA 
PME(D0+,D1-,D2-,D3hot+,D3cold+)
Status: D0 NoSoftRst+ PME-Enable- DSel=0 DScale=0 PME-
Capabilities: [90] MSI: Enable+ Count=1/1 Maskable- 64bit-
Address: fee002f8  Data: 
Capabilities: [a0] Express (v2) Root Port (Slot+), MSI 00
DevCap: MaxPayload 128 bytes, PhantFunc 0, Latency L0s <64ns, 
L1 <1us
ExtTag- RBE+ FLReset-
DevCtl: Report errors: Correctable- Non-Fatal- Fatal- 
Unsupported-
RlxdOrd- ExtTag- PhantFunc- AuxPwr- NoSnoop-
MaxPayload 128 bytes, MaxReadReq 128 bytes
DevSta: CorrErr- UncorrErr- FatalErr- UnsuppReq- AuxPwr- 
TransPend-
LnkCap: Port #3, Speed 8GT/s, Width x8, ASPM L0s L1, Latency L0 
<256ns, L1 <8us
ClockPM- Surprise- LLActRep- BwNot+
LnkCtl: ASPM Disabled; RCB 64 bytes Disabled- Retrain- CommClk+
ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-
LnkSta: Speed 5GT/s, Width x8, TrErr- Train- SlotClk+ DLActive- 
BWMgmt+ ABWMgmt+
SltCap: AttnBtn- PwrCtrl- MRL- AttnInd- PwrInd- HotPlug- 
Surprise-
Slot #2, PowerLimit 75.000W; Interlock- NoCompl+
  

[Qemu-devel] [PATCH v4 6/8] intel-iommu: add supports for queued invalidation interface

[Le Tan]

Add supports for queued invalidation interface, an expended invalidation
interface with extended capabilities.

Signed-off-by: Le Tan 
---
 hw/i386/intel_iommu.c  | 373 -
 hw/i386/intel_iommu_internal.h |  27 ++-
 2 files changed, 393 insertions(+), 7 deletions(-)

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 8e67e04..60dec4f 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -314,6 +314,41 @@ static void vtd_report_dmar_fault(IntelIOMMUState *s, 
uint16_t source_id,
 }
 }
 
+/* Handle Invalidation Queue Errors of queued invalidation interface error
+ * conditions.
+ */
+static void vtd_handle_inv_queue_error(IntelIOMMUState *s)
+{
+uint32_t fsts_reg = vtd_get_long_raw(s, DMAR_FSTS_REG);
+
+vtd_set_clear_mask_long(s, DMAR_FSTS_REG, 0, VTD_FSTS_IQE);
+vtd_generate_fault_event(s, fsts_reg);
+}
+
+/* Set the IWC field and try to generate an invalidation completion interrupt 
*/
+static void vtd_generate_completion_event(IntelIOMMUState *s)
+{
+VTD_DPRINTF(INV, "completes an invalidation wait command with "
+"Interrupt Flag");
+if (vtd_get_long_raw(s, DMAR_ICS_REG) & VTD_ICS_IWC) {
+VTD_DPRINTF(INV, "there is a previous interrupt condition to be "
+"serviced by software, "
+"new invalidation event is not generated");
+return;
+}
+vtd_set_clear_mask_long(s, DMAR_ICS_REG, 0, VTD_ICS_IWC);
+vtd_set_clear_mask_long(s, DMAR_IECTL_REG, 0, VTD_IECTL_IP);
+if (vtd_get_long_raw(s, DMAR_IECTL_REG) & VTD_IECTL_IM) {
+VTD_DPRINTF(INV, "IM filed in IECTL_REG is set, new invalidation "
+"event is not generated");
+return;
+} else {
+/* Generate the interrupt event */
+vtd_generate_interrupt(s, DMAR_IEADDR_REG, DMAR_IEDATA_REG);
+vtd_set_clear_mask_long(s, DMAR_IECTL_REG, VTD_IECTL_IP, 0);
+}
+}
+
 static inline bool vtd_root_entry_present(VTDRootEntry *root)
 {
 return root->val & VTD_ROOT_ENTRY_P;
@@ -759,6 +794,54 @@ static uint64_t vtd_iotlb_flush(IntelIOMMUState *s, 
uint64_t val)
 return iaig;
 }
 
+static inline bool vtd_queued_inv_enable_check(IntelIOMMUState *s)
+{
+return s->iq_tail == 0;
+}
+
+static inline bool vtd_queued_inv_disable_check(IntelIOMMUState *s)
+{
+return s->qi_enabled && (s->iq_tail == s->iq_head) &&
+   (s->iq_last_desc_type == VTD_INV_DESC_WAIT);
+}
+
+static void vtd_handle_gcmd_qie(IntelIOMMUState *s, bool en)
+{
+uint64_t iqa_val = vtd_get_quad_raw(s, DMAR_IQA_REG);
+
+VTD_DPRINTF(INV, "Queued Invalidation Enable %s", (en ? "on" : "off"));
+if (en) {
+if (vtd_queued_inv_enable_check(s)) {
+s->iq = iqa_val & VTD_IQA_IQA_MASK;
+/* 2^(x+8) entries */
+s->iq_size = 1UL << ((iqa_val & VTD_IQA_QS) + 8);
+s->qi_enabled = true;
+VTD_DPRINTF(INV, "DMAR_IQA_REG 0x%"PRIx64, iqa_val);
+VTD_DPRINTF(INV, "Invalidation Queue addr 0x%"PRIx64 " size %d",
+s->iq, s->iq_size);
+/* Ok - report back to driver */
+vtd_set_clear_mask_long(s, DMAR_GSTS_REG, 0, VTD_GSTS_QIES);
+} else {
+VTD_DPRINTF(GENERAL, "error: can't enable Queued Invalidation: "
+"tail %"PRIu16, s->iq_tail);
+}
+} else {
+if (vtd_queued_inv_disable_check(s)) {
+/* disable Queued Invalidation */
+vtd_set_quad_raw(s, DMAR_IQH_REG, 0);
+s->iq_head = 0;
+s->qi_enabled = false;
+/* Ok - report back to driver */
+vtd_set_clear_mask_long(s, DMAR_GSTS_REG, VTD_GSTS_QIES, 0);
+} else {
+VTD_DPRINTF(GENERAL, "error: can't disable Queued Invalidation: "
+"head %"PRIu16 ", tail %"PRIu16
+", last_descriptor %"PRIu8,
+s->iq_head, s->iq_tail, s->iq_last_desc_type);
+}
+}
+}
+
 /* Set Root Table Pointer */
 static void vtd_handle_gcmd_srtp(IntelIOMMUState *s)
 {
@@ -804,6 +887,10 @@ static void vtd_handle_gcmd_write(IntelIOMMUState *s)
 /* Set/update the root-table pointer */
 vtd_handle_gcmd_srtp(s);
 }
+if (changed & VTD_GCMD_QIE) {
+/* Queued Invalidation Enable */
+vtd_handle_gcmd_qie(s, val & VTD_GCMD_QIE);
+}
 }
 
 /* Handle write to Context Command Register */
@@ -814,6 +901,11 @@ static void vtd_handle_ccmd_write(IntelIOMMUState *s)
 
 /* Context-cache invalidation request */
 if (val & VTD_CCMD_ICC) {
+if (s->qi_enabled) {
+VTD_DPRINTF(GENERAL, "error: Queued Invalidation enabled, "
+"should not use register-based invalidation");
+return;
+}
 ret = vtd_context_cache_invalidate(s, val);
 /* Invalidation completed. Change something to show */
 vtd_set_clear_mask

[Qemu-devel] [PATCH v4 7/8] intel-iommu: add context-cache to cache context-entry

[Le Tan]

Add context-cache to cache context-entry encountered on a page-walk. Each
VTDAddressSpace has a member of VTDContextCacheEntry which represents an entry
in the context-cache. Since devices with different bus_num and devfn have their
respective VTDAddressSpace, this will be a good way to reference the cached
entries.
Each VTDContextCacheEntry will have a context_cache_gen and the cached entry
is valid only when context_cache_gen equals IntelIOMMUState.context_cache_gen.

Signed-off-by: Le Tan 
---
 hw/i386/intel_iommu.c  | 188 +++--
 hw/i386/intel_iommu_internal.h |  23 +++--
 hw/pci-host/q35.c  |   1 +
 include/hw/i386/intel_iommu.h  |  22 +
 4 files changed, 199 insertions(+), 35 deletions(-)

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 60dec4f..c514310 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -27,6 +27,7 @@
 #ifdef DEBUG_INTEL_IOMMU
 enum {
 DEBUG_GENERAL, DEBUG_CSR, DEBUG_INV, DEBUG_MMU, DEBUG_FLOG,
+DEBUG_CACHE,
 };
 #define VTD_DBGBIT(x)   (1 << DEBUG_##x)
 static int vtd_dbgflags = VTD_DBGBIT(GENERAL) | VTD_DBGBIT(CSR);
@@ -131,6 +132,33 @@ static uint64_t vtd_set_clear_mask_quad(IntelIOMMUState 
*s, hwaddr addr,
 return new_val;
 }
 
+/* Reset all the gen of VTDAddressSpace to zero and set the gen of
+ * IntelIOMMUState to 1.
+ */
+static void vtd_reset_context_cache(IntelIOMMUState *s)
+{
+VTDAddressSpace **pvtd_as;
+VTDAddressSpace *vtd_as;
+uint32_t bus_it;
+uint32_t devfn_it;
+
+VTD_DPRINTF(CACHE, "global context_cache_gen=1");
+for (bus_it = 0; bus_it < VTD_PCI_BUS_MAX; ++bus_it) {
+pvtd_as = s->address_spaces[bus_it];
+if (!pvtd_as) {
+continue;
+}
+for (devfn_it = 0; devfn_it < VTD_PCI_DEVFN_MAX; ++devfn_it) {
+vtd_as = pvtd_as[devfn_it];
+if (!vtd_as) {
+continue;
+}
+vtd_as->context_cache_entry.context_cache_gen = 0;
+}
+}
+s->context_cache_gen = 1;
+}
+
 /* Given the reg addr of both the message data and address, generate an
  * interrupt via MSI.
  */
@@ -651,11 +679,13 @@ static inline bool vtd_is_interrupt_addr(hwaddr addr)
  * @is_write: The access is a write operation
  * @entry: IOMMUTLBEntry that contain the addr to be translated and result
  */
-static void vtd_do_iommu_translate(IntelIOMMUState *s, uint8_t bus_num,
+static void vtd_do_iommu_translate(VTDAddressSpace *vtd_as, uint8_t bus_num,
uint8_t devfn, hwaddr addr, bool is_write,
IOMMUTLBEntry *entry)
 {
+IntelIOMMUState *s = vtd_as->iommu_state;
 VTDContextEntry ce;
+VTDContextCacheEntry *cc_entry = &vtd_as->context_cache_entry;
 uint64_t slpte;
 uint32_t level;
 uint16_t source_id = vtd_make_source_id(bus_num, devfn);
@@ -686,18 +716,35 @@ static void vtd_do_iommu_translate(IntelIOMMUState *s, 
uint8_t bus_num,
 return;
 }
 }
-
-ret_fr = vtd_dev_to_context_entry(s, bus_num, devfn, &ce);
-is_fpd_set = ce.lo & VTD_CONTEXT_ENTRY_FPD;
-if (ret_fr) {
-ret_fr = -ret_fr;
-if (is_fpd_set && vtd_is_qualified_fault(ret_fr)) {
-VTD_DPRINTF(FLOG, "fault processing is disabled for DMA requests "
-"through this context-entry (with FPD Set)");
-} else {
-vtd_report_dmar_fault(s, source_id, addr, ret_fr, is_write);
+/* Try to fetch context-entry from cache first */
+if (cc_entry->context_cache_gen == s->context_cache_gen) {
+VTD_DPRINTF(CACHE, "hit context-cache bus %d devfn %d "
+"(hi %"PRIx64 " lo %"PRIx64 " gen %"PRIu32 ")",
+bus_num, devfn, cc_entry->context_entry.hi,
+cc_entry->context_entry.lo, cc_entry->context_cache_gen);
+ce = cc_entry->context_entry;
+is_fpd_set = ce.lo & VTD_CONTEXT_ENTRY_FPD;
+} else {
+ret_fr = vtd_dev_to_context_entry(s, bus_num, devfn, &ce);
+is_fpd_set = ce.lo & VTD_CONTEXT_ENTRY_FPD;
+if (ret_fr) {
+ret_fr = -ret_fr;
+if (is_fpd_set && vtd_is_qualified_fault(ret_fr)) {
+VTD_DPRINTF(FLOG, "fault processing is disabled for DMA "
+"requests through this context-entry "
+"(with FPD Set)");
+} else {
+vtd_report_dmar_fault(s, source_id, addr, ret_fr, is_write);
+}
+return;
 }
-return;
+/* Update context-cache */
+VTD_DPRINTF(CACHE, "update context-cache bus %d devfn %d "
+"(hi %"PRIx64 " lo %"PRIx64 " gen %"PRIu32 "->%"PRIu32 ")",
+bus_num, devfn, ce.hi, ce.lo,
+cc_entry->context_cache_gen, s->context_cache_gen);
+cc_entry->context_entry = ce;
+cc_entry->context_cache_gen = s->conte

[Qemu-devel] [PATCH v4 5/8] intel-iommu: fix coding style issues around in q35.c and machine.c

[Le Tan]

Fix coding style issues around in hw/pci-host/q35.c and hw/core/machine.c.

Signed-off-by: Le Tan 
---
 hw/core/machine.c | 10 +++---
 hw/pci-host/q35.c | 11 ++-
 2 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index 0708de5..f0046d6 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -284,10 +284,14 @@ static void machine_initfn(Object *obj)
  machine_set_dump_guest_core,
  NULL);
 object_property_add_bool(obj, "mem-merge",
- machine_get_mem_merge, machine_set_mem_merge, 
NULL);
-object_property_add_bool(obj, "usb", machine_get_usb, machine_set_usb, 
NULL);
+ machine_get_mem_merge,
+ machine_set_mem_merge, NULL);
+object_property_add_bool(obj, "usb",
+ machine_get_usb,
+ machine_set_usb, NULL);
 object_property_add_str(obj, "firmware",
-machine_get_firmware, machine_set_firmware, NULL);
+machine_get_firmware,
+machine_set_firmware, NULL);
 object_property_add_bool(obj, "iommu",
  machine_get_iommu,
  machine_set_iommu, NULL);
diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index ad8f1b9..5618ad2 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -405,12 +405,13 @@ static int mch_init(PCIDevice *d)
 memory_region_add_subregion_overlap(mch->system_memory, 0xa,
 &mch->smram_region, 1);
 memory_region_set_enabled(&mch->smram_region, false);
-init_pam(DEVICE(mch), mch->ram_memory, mch->system_memory, 
mch->pci_address_space,
- &mch->pam_regions[0], PAM_BIOS_BASE, PAM_BIOS_SIZE);
+init_pam(DEVICE(mch), mch->ram_memory, mch->system_memory,
+ mch->pci_address_space, &mch->pam_regions[0],
+ PAM_BIOS_BASE, PAM_BIOS_SIZE);
 for (i = 0; i < 12; ++i) {
-init_pam(DEVICE(mch), mch->ram_memory, mch->system_memory, 
mch->pci_address_space,
- &mch->pam_regions[i+1], PAM_EXPAN_BASE + i * PAM_EXPAN_SIZE,
- PAM_EXPAN_SIZE);
+init_pam(DEVICE(mch), mch->ram_memory, mch->system_memory,
+ mch->pci_address_space, &mch->pam_regions[i+1],
+ PAM_EXPAN_BASE + i * PAM_EXPAN_SIZE, PAM_EXPAN_SIZE);
 }
 /* Intel IOMMU (VT-d) */
 if (qemu_opt_get_bool(qemu_get_machine_opts(), "iommu", false)) {
-- 
1.9.1

[Qemu-devel] [PATCH v4 8/8] intel-iommu: add IOTLB using hash table

[Le Tan]

Add IOTLB to cache information about the translation of input-addresses. IOTLB
use a GHashTable as cache. The key of the hash table is the logical-OR of gfn
and source id after left-shifting.

Signed-off-by: Le Tan 
---
 hw/i386/intel_iommu.c  | 213 -
 hw/i386/intel_iommu_internal.h |  34 ++-
 include/hw/i386/intel_iommu.h  |  11 ++-
 3 files changed, 251 insertions(+), 7 deletions(-)

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index c514310..0a4282a 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -132,6 +132,35 @@ static uint64_t vtd_set_clear_mask_quad(IntelIOMMUState 
*s, hwaddr addr,
 return new_val;
 }
 
+/* GHashTable functions */
+static gboolean vtd_uint64_equal(gconstpointer v1, gconstpointer v2)
+{
+return *((const uint64_t *)v1) == *((const uint64_t *)v2);
+}
+
+static guint vtd_uint64_hash(gconstpointer v)
+{
+return (guint)*(const uint64_t *)v;
+}
+
+static gboolean vtd_hash_remove_by_domain(gpointer key, gpointer value,
+  gpointer user_data)
+{
+VTDIOTLBEntry *entry = (VTDIOTLBEntry *)value;
+uint16_t domain_id = *(uint16_t *)user_data;
+return entry->domain_id == domain_id;
+}
+
+static gboolean vtd_hash_remove_by_page(gpointer key, gpointer value,
+gpointer user_data)
+{
+VTDIOTLBEntry *entry = (VTDIOTLBEntry *)value;
+VTDIOTLBPageInvInfo *info = (VTDIOTLBPageInvInfo *)user_data;
+uint64_t gfn = info->gfn & info->mask;
+return (entry->domain_id == info->domain_id) &&
+((entry->gfn & info->mask) == gfn);
+}
+
 /* Reset all the gen of VTDAddressSpace to zero and set the gen of
  * IntelIOMMUState to 1.
  */
@@ -159,6 +188,48 @@ static void vtd_reset_context_cache(IntelIOMMUState *s)
 s->context_cache_gen = 1;
 }
 
+static void vtd_reset_iotlb(IntelIOMMUState *s)
+{
+assert(s->iotlb);
+g_hash_table_remove_all(s->iotlb);
+}
+
+static VTDIOTLBEntry *vtd_lookup_iotlb(IntelIOMMUState *s, uint16_t source_id,
+   hwaddr addr)
+{
+uint64_t key;
+
+key = (addr >> VTD_PAGE_SHIFT_4K) |
+   ((uint64_t)(source_id) << VTD_IOTLB_SID_SHIFT);
+return g_hash_table_lookup(s->iotlb, &key);
+
+}
+
+static void vtd_update_iotlb(IntelIOMMUState *s, uint16_t source_id,
+ uint16_t domain_id, hwaddr addr, uint64_t slpte,
+ bool read_flags, bool write_flags)
+{
+VTDIOTLBEntry *entry = g_malloc(sizeof(*entry));
+uint64_t *key = g_malloc(sizeof(*key));
+uint64_t gfn = addr >> VTD_PAGE_SHIFT_4K;
+
+VTD_DPRINTF(CACHE, "update iotlb sid 0x%"PRIx16 " gpa 0x%"PRIx64
+" slpte 0x%"PRIx64 " did 0x%"PRIx16, source_id, addr, slpte,
+domain_id);
+if (g_hash_table_size(s->iotlb) >= VTD_IOTLB_MAX_SIZE) {
+VTD_DPRINTF(CACHE, "iotlb exceeds size limit, forced to reset");
+vtd_reset_iotlb(s);
+}
+
+entry->gfn = gfn;
+entry->domain_id = domain_id;
+entry->slpte = slpte;
+entry->read_flags = read_flags;
+entry->write_flags = write_flags;
+*key = gfn | ((uint64_t)(source_id) << VTD_IOTLB_SID_SHIFT);
+g_hash_table_replace(s->iotlb, key, entry);
+}
+
 /* Given the reg addr of both the message data and address, generate an
  * interrupt via MSI.
  */
@@ -693,6 +764,7 @@ static void vtd_do_iommu_translate(VTDAddressSpace *vtd_as, 
uint8_t bus_num,
 bool is_fpd_set = false;
 bool reads = true;
 bool writes = true;
+VTDIOTLBEntry *iotlb_entry;
 
 /* Check if the request is in interrupt address range */
 if (vtd_is_interrupt_addr(addr)) {
@@ -716,6 +788,17 @@ static void vtd_do_iommu_translate(VTDAddressSpace 
*vtd_as, uint8_t bus_num,
 return;
 }
 }
+/* Try to fetch slpte form IOTLB */
+iotlb_entry = vtd_lookup_iotlb(s, source_id, addr);
+if (iotlb_entry) {
+VTD_DPRINTF(CACHE, "hit iotlb sid 0x%"PRIx16 " gpa 0x%"PRIx64
+" slpte 0x%"PRIx64 " did 0x%"PRIx16, source_id, addr,
+iotlb_entry->slpte, iotlb_entry->domain_id);
+slpte = iotlb_entry->slpte;
+reads = iotlb_entry->read_flags;
+writes = iotlb_entry->write_flags;
+goto out;
+}
 /* Try to fetch context-entry from cache first */
 if (cc_entry->context_cache_gen == s->context_cache_gen) {
 VTD_DPRINTF(CACHE, "hit context-cache bus %d devfn %d "
@@ -760,6 +843,9 @@ static void vtd_do_iommu_translate(VTDAddressSpace *vtd_as, 
uint8_t bus_num,
 return;
 }
 
+vtd_update_iotlb(s, source_id, VTD_CONTEXT_ENTRY_DID(ce.hi), addr, slpte,
+ reads, writes);
+out:
 entry->iova = addr & VTD_PAGE_MASK_4K;
 entry->translated_addr = vtd_get_slpte_addr(slpte) & VTD_PAGE_MASK_4K;
 entry->addr_mask = ~VTD_PAGE_MASK_4K;
@@ -859,6 +945,29 @@ static uint64_t 
vtd_context_cache_inv

[Qemu-devel] [PATCH v4 3/8] intel-iommu: add DMAR table to ACPI tables

[Le Tan]

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 
---
 hw/i386/acpi-build.c | 39 +++
 hw/i386/acpi-defs.h  | 40 
 2 files changed, 79 insertions(+)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 816c6d9..ac56a63 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -47,6 +47,7 @@
 #include "hw/i386/ich9.h"
 #include "hw/pci/pci_bus.h"
 #include "hw/pci-host/q35.h"
+#include "hw/i386/intel_iommu.h"
 
 #include "hw/i386/q35-acpi-dsdt.hex"
 #include "hw/i386/acpi-dsdt.hex"
@@ -1350,6 +1351,30 @@ build_mcfg_q35(GArray *table_data, GArray *linker, 
AcpiMcfgInfo *info)
 }
 
 static void
+build_dmar_q35(GArray *table_data, GArray *linker)
+{
+int dmar_start = table_data->len;
+
+AcpiTableDmar *dmar;
+AcpiDmarHardwareUnit *drhd;
+
+dmar = acpi_data_push(table_data, sizeof(*dmar));
+dmar->host_address_width = VTD_HOST_ADDRESS_WIDTH - 1;
+dmar->flags = 0;/* No intr_remap for now */
+
+/* DMAR Remapping Hardware Unit Definition structure */
+drhd = acpi_data_push(table_data, sizeof(*drhd));
+drhd->type = cpu_to_le16(ACPI_DMAR_TYPE_HARDWARE_UNIT);
+drhd->length = cpu_to_le16(sizeof(*drhd));   /* No device scope now */
+drhd->flags = ACPI_DMAR_INCLUDE_PCI_ALL;
+drhd->pci_segment = cpu_to_le16(0);
+drhd->address = cpu_to_le64(Q35_HOST_BRIDGE_IOMMU_ADDR);
+
+build_header(linker, table_data, (void *)(table_data->data + dmar_start),
+ "DMAR", table_data->len - dmar_start, 1);
+}
+
+static void
 build_dsdt(GArray *table_data, GArray *linker, AcpiMiscInfo *misc)
 {
 AcpiTableHeader *dsdt;
@@ -1470,6 +1495,16 @@ static bool acpi_get_mcfg(AcpiMcfgInfo *mcfg)
 return true;
 }
 
+static bool acpi_has_iommu(void)
+{
+bool ambiguous;
+Object *intel_iommu;
+
+intel_iommu = object_resolve_path_type("", TYPE_INTEL_IOMMU_DEVICE,
+   &ambiguous);
+return intel_iommu && !ambiguous;
+}
+
 static
 void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables)
 {
@@ -1539,6 +1574,10 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables 
*tables)
 acpi_add_table(table_offsets, tables->table_data);
 build_mcfg_q35(tables->table_data, tables->linker, &mcfg);
 }
+if (acpi_has_iommu()) {
+acpi_add_table(table_offsets, tables->table_data);
+build_dmar_q35(tables->table_data, tables->linker);
+}
 
 /* Add tables supplied by user (if any) */
 for (u = acpi_table_first(); u; u = acpi_table_next(u)) {
diff --git a/hw/i386/acpi-defs.h b/hw/i386/acpi-defs.h
index e93babb..93f424b 100644
--- a/hw/i386/acpi-defs.h
+++ b/hw/i386/acpi-defs.h
@@ -314,4 +314,44 @@ struct AcpiTableMcfg {
 } QEMU_PACKED;
 typedef struct AcpiTableMcfg AcpiTableMcfg;
 
+/* DMAR - DMA Remapping table r2.2 */
+struct AcpiTableDmar {
+ACPI_TABLE_HEADER_DEF
+uint8_t host_address_width; /* Maximum DMA physical addressability */
+uint8_t flags;
+uint8_t reserved[10];
+} QEMU_PACKED;
+typedef struct AcpiTableDmar AcpiTableDmar;
+
+/* Masks for Flags field above */
+#define ACPI_DMAR_INTR_REMAP1
+#define ACPI_DMAR_X2APIC_OPT_OUT(1 << 1)
+
+/* Values for sub-structure type for DMAR */
+enum {
+ACPI_DMAR_TYPE_HARDWARE_UNIT = 0,   /* DRHD */
+ACPI_DMAR_TYPE_RESERVED_MEMORY = 1, /* RMRR */
+ACPI_DMAR_TYPE_ATSR = 2,/* ATSR */
+ACPI_DMAR_TYPE_HARDWARE_AFFINITY = 3,   /* RHSR */
+ACPI_DMAR_TYPE_ANDD = 4,/* ANDD */
+ACPI_DMAR_TYPE_RESERVED = 5 /* Reserved for furture use */
+};
+
+/*
+ * Sub-structures for DMAR
+ */
+/* Type 0: Hardware Unit Definition */
+struct AcpiDmarHardwareUnit {
+uint16_t type;
+uint16_t length;
+uint8_t flags;
+uint8_t reserved;
+uint16_t pci_segment;   /* The PCI Segment associated with this unit */
+uint64_t address;   /* Base address of remapping hardware register-set */
+} QEMU_PACKED;
+typedef struct AcpiDmarHardwareUnit AcpiDmarHardwareUnit;
+
+/* Masks for Flags field above */
+#define ACPI_DMAR_INCLUDE_PCI_ALL   1
+
 #endif
-- 
1.9.1

[Qemu-devel] [PATCH v4 4/8] intel-iommu: add Intel IOMMU emulation to q35 and add a machine option "iommu" as a switch

[Le Tan]

Add Intel IOMMU emulation to q35 chipset and expose it to the guest.
1. Add a machine option. Users can use "-machine iommu=on|off" in the command
line to enable/disable Intel IOMMU. The default is off.
2. Accroding to the machine option, q35 will initialize the Intel IOMMU and
use pci_setup_iommu() to setup q35_host_dma_iommu() as the IOMMU function for
the pci bus.
3. q35_host_dma_iommu() will return different address space according to the
bus_num and devfn of the device.

Signed-off-by: Le Tan 
---
 hw/core/machine.c | 17 +
 hw/pci-host/q35.c | 46 ++
 include/hw/boards.h   |  1 +
 include/hw/pci-host/q35.h |  2 ++
 qemu-options.hx   |  5 -
 vl.c  |  4 
 6 files changed, 74 insertions(+), 1 deletion(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index 7a66c57..0708de5 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -235,6 +235,20 @@ static void machine_set_firmware(Object *obj, const char 
*value, Error **errp)
 ms->firmware = g_strdup(value);
 }
 
+static bool machine_get_iommu(Object *obj, Error **errp)
+{
+MachineState *ms = MACHINE(obj);
+
+return ms->iommu;
+}
+
+static void machine_set_iommu(Object *obj, bool value, Error **errp)
+{
+MachineState *ms = MACHINE(obj);
+
+ms->iommu = value;
+}
+
 static void machine_initfn(Object *obj)
 {
 object_property_add_str(obj, "accel",
@@ -274,6 +288,9 @@ static void machine_initfn(Object *obj)
 object_property_add_bool(obj, "usb", machine_get_usb, machine_set_usb, 
NULL);
 object_property_add_str(obj, "firmware",
 machine_get_firmware, machine_set_firmware, NULL);
+object_property_add_bool(obj, "iommu",
+ machine_get_iommu,
+ machine_set_iommu, NULL);
 }
 
 static void machine_finalize(Object *obj)
diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index a0a3068..ad8f1b9 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -347,6 +347,48 @@ static void mch_reset(DeviceState *qdev)
 mch_update(mch);
 }
 
+static AddressSpace *q35_host_dma_iommu(PCIBus *bus, void *opaque, int devfn)
+{
+IntelIOMMUState *s = opaque;
+VTDAddressSpace **pvtd_as;
+int bus_num = pci_bus_num(bus);
+
+assert(0 <= bus_num && bus_num <= VTD_PCI_BUS_MAX);
+assert(0 <= devfn && devfn <= VTD_PCI_DEVFN_MAX);
+
+pvtd_as = s->address_spaces[bus_num];
+if (!pvtd_as) {
+/* No corresponding free() */
+pvtd_as = g_malloc0(sizeof(VTDAddressSpace *) * VTD_PCI_DEVFN_MAX);
+s->address_spaces[bus_num] = pvtd_as;
+}
+if (!pvtd_as[devfn]) {
+pvtd_as[devfn] = g_malloc0(sizeof(VTDAddressSpace));
+
+pvtd_as[devfn]->bus_num = (uint8_t)bus_num;
+pvtd_as[devfn]->devfn = (uint8_t)devfn;
+pvtd_as[devfn]->iommu_state = s;
+memory_region_init_iommu(&pvtd_as[devfn]->iommu, OBJECT(s),
+ &s->iommu_ops, "intel_iommu", UINT64_MAX);
+address_space_init(&pvtd_as[devfn]->as,
+   &pvtd_as[devfn]->iommu, "intel_iommu");
+}
+return &pvtd_as[devfn]->as;
+}
+
+static void mch_init_dmar(MCHPCIState *mch)
+{
+PCIBus *pci_bus = PCI_BUS(qdev_get_parent_bus(DEVICE(mch)));
+
+mch->iommu = INTEL_IOMMU_DEVICE(qdev_create(NULL, 
TYPE_INTEL_IOMMU_DEVICE));
+object_property_add_child(OBJECT(mch), "intel-iommu",
+  OBJECT(mch->iommu), NULL);
+qdev_init_nofail(DEVICE(mch->iommu));
+sysbus_mmio_map(SYS_BUS_DEVICE(mch->iommu), 0, Q35_HOST_BRIDGE_IOMMU_ADDR);
+
+pci_setup_iommu(pci_bus, q35_host_dma_iommu, mch->iommu);
+}
+
 static int mch_init(PCIDevice *d)
 {
 int i;
@@ -370,6 +412,10 @@ static int mch_init(PCIDevice *d)
  &mch->pam_regions[i+1], PAM_EXPAN_BASE + i * PAM_EXPAN_SIZE,
  PAM_EXPAN_SIZE);
 }
+/* Intel IOMMU (VT-d) */
+if (qemu_opt_get_bool(qemu_get_machine_opts(), "iommu", false)) {
+mch_init_dmar(mch);
+}
 return 0;
 }
 
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 605a970..dfb6718 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -123,6 +123,7 @@ struct MachineState {
 bool mem_merge;
 bool usb;
 char *firmware;
+bool iommu;
 
 ram_addr_t ram_size;
 ram_addr_t maxram_size;
diff --git a/include/hw/pci-host/q35.h b/include/hw/pci-host/q35.h
index d9ee978..025d6e6 100644
--- a/include/hw/pci-host/q35.h
+++ b/include/hw/pci-host/q35.h
@@ -33,6 +33,7 @@
 #include "hw/acpi/acpi.h"
 #include "hw/acpi/ich9.h"
 #include "hw/pci-host/pam.h"
+#include "hw/i386/intel_iommu.h"
 
 #define TYPE_Q35_HOST_DEVICE "q35-pcihost"
 #define Q35_HOST_DEVICE(obj) \
@@ -60,6 +61,7 @@ typedef struct MCHPCIState {
 uint64_t pci_hole64_size;
 PcGuestInfo *guest_info;
 uint32_t short_root_bus;
+IntelIOMMUState *iommu;
 } MCHPCIState;
 
 typedef s

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

[Le Tan]

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.

Features implemented for now are:
1. Response to important CSRs accesses;
2. DMAR (DMA remapping) without PASID support;
3. Primary fault logging;
4. Support both register-based and queued invalidation for IOTLB and context
   cache invalidation;
5. Add DMAR table to ACPI tables to expose VT-d to BIOS;
6. Add "-machine iommu=on|off" option to enable/disable VT-d;
7. Only one DMAR unit for all the devices of PCI Segment 0;
8. Context-cache and IOTLB.

Testing:
1. L1 guest with Linux with intel_iommu=on can interact with VT-d and boot
smoothly, and there exists information about VT-d in the log of kernel;
2. Run L1 with VT-d, L2 guest with Linux can boot smoothly withou PCI device
passthrough;
3. Run L1 with VT-d and "-soundhw ac97 (QEMU_AUDIO_DRV=alsa)", then assign the
sound card to L2; L2 can boot smoothly with legacy PCI assignment and I can
hear the music played in L2 from the host speakers;
4. Jailhouse hypervisor can run smoothly (tested by Jan).
5. Run L1 with VT-d and e1000 network card, then assign e1000 to L2; L2 will be
STUCK when booting. This still remains unsolved now. As far as I know, I suppose
that the L2 crashes when doing e1000_probe(). The QEMU of L1 will dump
something with "KVM: entry failed, hardware error 0x0", and the KVM of host
will print "nested_vmx_exit_handled failed vm entry 7". Unlike assigning the
sound card, after being assigned to L2, there is no translation entry of e1000
through VT-d, which I think means that e1000 doesn't issue any DMA access during
the boot of L2. Sometimes the kernel of L2 will print "divide error" during
booting. Maybe it results from the lack of reset mechanism.
6. VFIO is tested and is similar to legacy pci assignment.

TODO:
1. Fix the bug of legacy PCI assignment;
2. Add unit test for DMAR ACPI table;
3. Add support for PCIE-to-PCIE bridge.

Changes since v3:
*address reviewing suggestions given by Jan and Michael
-implement Context-cache and IOTLB
-remove 'inline' keyword from most functions
-rename all the functions with prefix vtd_
-clean up constant definitions

Changes since v2:
*address reviewing suggestions given by Jan
-add support for primary fault logging
-add support for queued invalidation

Changes since v1:
*address reviewing suggestions given by Michael, Paolo, Stefan and Jan
-split intel_iommu.h to include/hw/i386/intel_iommu.h and
 hw/i386/intel_iommu_internal.h
-change the copyright information
-change D() to VTD_DPRINTF()
-remove dead code
-rename constant definitions with consistent prefix VTD_
-rename some struct definitions according to QEMU standard
-rename some CSRs access functions
-use endian-save functions to access CSRs
-change machine option to "iommu=on|off"

Thanks very much!

Git trees:
https://github.com/tamlok/qemu

Le Tan (8):
  iommu: add is_write as a parameter to the translate function of
MemoryRegionIOMMUOps
  intel-iommu: introduce Intel IOMMU (VT-d) emulation
  intel-iommu: add DMAR table to ACPI tables
  intel-iommu: add Intel IOMMU emulation to q35 and add a machine option
"iommu" as a switch
  intel-iommu: fix coding style issues around in q35.c and machine.c
  intel-iommu: add supports for queued invalidation interface
  intel-iommu: add context-cache to cache context-entry
  intel-iommu: add IOTLB using hash table

 exec.c |2 +-
 hw/alpha/typhoon.c |3 +-
 hw/core/machine.c  |   27 +-
 hw/i386/Makefile.objs  |1 +
 hw/i386/acpi-build.c   |   39 +
 hw/i386/acpi-defs.h|   40 +
 hw/i386/intel_iommu.c  | 1963 
 hw/i386/intel_iommu_internal.h |  389 
 hw/pci-host/apb.c  |3 +-
 hw/pci-host/q35.c  |   58 +-
 hw/ppc/spapr_iommu.c   |3 +-
 include/exec/memory.h  |2 +-
 include/hw/boards.h|1 +
 include/hw/i386/intel_iommu.h  |  120 +++
 include/hw/pci-host/q35.h  |2 +
 qemu-options.hx|5 +-
 vl.c   |4 +
 17 files changed, 2648 insertions(+), 14 deletions(-)
 create mode 100644 hw/i386/intel_iommu.c
 create mode 100644 hw/i386/intel_iommu_internal.h
 create mode 100644 include/hw/i386/intel_iommu.h

-- 
1.9.1

[Qemu-devel] [PATCH v4 1/8] iommu: add is_write as a parameter to the translate function of MemoryRegionIOMMUOps

[Le Tan]

Add a bool variable is_write as a parameter to the translate function of
MemoryRegionIOMMUOps to indicate the operation of the access. It can be
used for correct fault reporting from within the callback.
Change the interface of related functions.

Signed-off-by: Le Tan 
---
 exec.c| 2 +-
 hw/alpha/typhoon.c| 3 ++-
 hw/pci-host/apb.c | 3 ++-
 hw/ppc/spapr_iommu.c  | 3 ++-
 include/exec/memory.h | 2 +-
 5 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/exec.c b/exec.c
index 765bd94..5ccc106 100644
--- a/exec.c
+++ b/exec.c
@@ -373,7 +373,7 @@ MemoryRegion *address_space_translate(AddressSpace *as, 
hwaddr addr,
 break;
 }
 
-iotlb = mr->iommu_ops->translate(mr, addr);
+iotlb = mr->iommu_ops->translate(mr, addr, is_write);
 addr = ((iotlb.translated_addr & ~iotlb.addr_mask)
 | (addr & iotlb.addr_mask));
 len = MIN(len, (addr | iotlb.addr_mask) - addr + 1);
diff --git a/hw/alpha/typhoon.c b/hw/alpha/typhoon.c
index 67a1070..31947d9 100644
--- a/hw/alpha/typhoon.c
+++ b/hw/alpha/typhoon.c
@@ -660,7 +660,8 @@ static bool window_translate(TyphoonWindow *win, hwaddr 
addr,
 /* Handle PCI-to-system address translation.  */
 /* TODO: A translation failure here ought to set PCI error codes on the
Pchip and generate a machine check interrupt.  */
-static IOMMUTLBEntry typhoon_translate_iommu(MemoryRegion *iommu, hwaddr addr)
+static IOMMUTLBEntry typhoon_translate_iommu(MemoryRegion *iommu, hwaddr addr,
+ bool is_write)
 {
 TyphoonPchip *pchip = container_of(iommu, TyphoonPchip, iommu);
 IOMMUTLBEntry ret;
diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
index d238a84..0e0e0ee 100644
--- a/hw/pci-host/apb.c
+++ b/hw/pci-host/apb.c
@@ -203,7 +203,8 @@ static AddressSpace *pbm_pci_dma_iommu(PCIBus *bus, void 
*opaque, int devfn)
 return &is->iommu_as;
 }
 
-static IOMMUTLBEntry pbm_translate_iommu(MemoryRegion *iommu, hwaddr addr)
+static IOMMUTLBEntry pbm_translate_iommu(MemoryRegion *iommu, hwaddr addr,
+ bool is_write)
 {
 IOMMUState *is = container_of(iommu, IOMMUState, iommu);
 hwaddr baseaddr, offset;
diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c
index f6e32a4..6c91d8e 100644
--- a/hw/ppc/spapr_iommu.c
+++ b/hw/ppc/spapr_iommu.c
@@ -59,7 +59,8 @@ static sPAPRTCETable *spapr_tce_find_by_liobn(uint32_t liobn)
 return NULL;
 }
 
-static IOMMUTLBEntry spapr_tce_translate_iommu(MemoryRegion *iommu, hwaddr 
addr)
+static IOMMUTLBEntry spapr_tce_translate_iommu(MemoryRegion *iommu, hwaddr 
addr,
+   bool is_write)
 {
 sPAPRTCETable *tcet = container_of(iommu, sPAPRTCETable, iommu);
 uint64_t tce;
diff --git a/include/exec/memory.h b/include/exec/memory.h
index e2c8e3e..834543d 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -129,7 +129,7 @@ typedef struct MemoryRegionIOMMUOps MemoryRegionIOMMUOps;
 
 struct MemoryRegionIOMMUOps {
 /* Return a TLB entry that contains a given address. */
-IOMMUTLBEntry (*translate)(MemoryRegion *iommu, hwaddr addr);
+IOMMUTLBEntry (*translate)(MemoryRegion *iommu, hwaddr addr, bool 
is_write);
 };
 
 typedef struct CoalescedMemoryRange CoalescedMemoryRange;
-- 
1.9.1

[Qemu-devel] [PATCH v4 2/8] intel-iommu: introduce Intel IOMMU (VT-d) emulation

[Le Tan]

Add support for emulating Intel IOMMU according to the VT-d specification for
the q35 chipset machine. Implement the logics for DMAR (DMA remapping) without
PASID support. The emulation supports register-based invalidation and primary
fault logging.

Signed-off-by: Le Tan 
---
 hw/i386/Makefile.objs  |1 +
 hw/i386/intel_iommu.c  | 1257 
 hw/i386/intel_iommu_internal.h |  333 +++
 include/hw/i386/intel_iommu.h  |   89 +++
 4 files changed, 1680 insertions(+)
 create mode 100644 hw/i386/intel_iommu.c
 create mode 100644 hw/i386/intel_iommu_internal.h
 create mode 100644 include/hw/i386/intel_iommu.h

diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
index 48014ab..6936111 100644
--- a/hw/i386/Makefile.objs
+++ b/hw/i386/Makefile.objs
@@ -2,6 +2,7 @@ obj-$(CONFIG_KVM) += kvm/
 obj-y += multiboot.o smbios.o
 obj-y += pc.o pc_piix.o pc_q35.o
 obj-y += pc_sysfw.o
+obj-y += intel_iommu.o
 obj-$(CONFIG_XEN) += ../xenpv/ xen/
 
 obj-y += kvmvapic.o
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
new file mode 100644
index 000..8e67e04
--- /dev/null
+++ b/hw/i386/intel_iommu.c
@@ -0,0 +1,1257 @@
+/*
+ * QEMU emulation of an Intel IOMMU (VT-d)
+ *   (DMA Remapping device)
+ *
+ * Copyright (C) 2013 Knut Omang, Oracle 
+ * Copyright (C) 2014 Le Tan, 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see .
+ */
+
+#include "hw/sysbus.h"
+#include "exec/address-spaces.h"
+#include "intel_iommu_internal.h"
+
+/*#define DEBUG_INTEL_IOMMU*/
+#ifdef DEBUG_INTEL_IOMMU
+enum {
+DEBUG_GENERAL, DEBUG_CSR, DEBUG_INV, DEBUG_MMU, DEBUG_FLOG,
+};
+#define VTD_DBGBIT(x)   (1 << DEBUG_##x)
+static int vtd_dbgflags = VTD_DBGBIT(GENERAL) | VTD_DBGBIT(CSR);
+
+#define VTD_DPRINTF(what, fmt, ...) do { \
+if (vtd_dbgflags & VTD_DBGBIT(what)) { \
+fprintf(stderr, "(vtd)%s: " fmt "\n", __func__, \
+## __VA_ARGS__); } \
+} while (0)
+#else
+#define VTD_DPRINTF(what, fmt, ...) do {} while (0)
+#endif
+
+static void vtd_define_quad(IntelIOMMUState *s, hwaddr addr, uint64_t val,
+uint64_t wmask, uint64_t w1cmask)
+{
+stq_le_p(&s->csr[addr], val);
+stq_le_p(&s->wmask[addr], wmask);
+stq_le_p(&s->w1cmask[addr], w1cmask);
+}
+
+static void vtd_define_quad_wo(IntelIOMMUState *s, hwaddr addr, uint64_t mask)
+{
+stq_le_p(&s->womask[addr], mask);
+}
+
+static void vtd_define_long(IntelIOMMUState *s, hwaddr addr, uint32_t val,
+uint32_t wmask, uint32_t w1cmask)
+{
+stl_le_p(&s->csr[addr], val);
+stl_le_p(&s->wmask[addr], wmask);
+stl_le_p(&s->w1cmask[addr], w1cmask);
+}
+
+static void vtd_define_long_wo(IntelIOMMUState *s, hwaddr addr, uint32_t mask)
+{
+stl_le_p(&s->womask[addr], mask);
+}
+
+/* "External" get/set operations */
+static void vtd_set_quad(IntelIOMMUState *s, hwaddr addr, uint64_t val)
+{
+uint64_t oldval = ldq_le_p(&s->csr[addr]);
+uint64_t wmask = ldq_le_p(&s->wmask[addr]);
+uint64_t w1cmask = ldq_le_p(&s->w1cmask[addr]);
+stq_le_p(&s->csr[addr],
+ ((oldval & ~wmask) | (val & wmask)) & ~(w1cmask & val));
+}
+
+static void vtd_set_long(IntelIOMMUState *s, hwaddr addr, uint32_t val)
+{
+uint32_t oldval = ldl_le_p(&s->csr[addr]);
+uint32_t wmask = ldl_le_p(&s->wmask[addr]);
+uint32_t w1cmask = ldl_le_p(&s->w1cmask[addr]);
+stl_le_p(&s->csr[addr],
+ ((oldval & ~wmask) | (val & wmask)) & ~(w1cmask & val));
+}
+
+static uint64_t vtd_get_quad(IntelIOMMUState *s, hwaddr addr)
+{
+uint64_t val = ldq_le_p(&s->csr[addr]);
+uint64_t womask = ldq_le_p(&s->womask[addr]);
+return val & ~womask;
+}
+
+static uint32_t vtd_get_long(IntelIOMMUState *s, hwaddr addr)
+{
+uint32_t val = ldl_le_p(&s->csr[addr]);
+uint32_t womask = ldl_le_p(&s->womask[addr]);
+return val & ~womask;
+}
+
+/* "Internal" get/set operations */
+static uint64_t vtd_get_quad_raw(IntelIOMMUState *s, hwaddr addr)
+{
+return ldq_le_p(&s->csr[addr]);
+}
+
+static uint32_t vtd_get_long_raw(IntelIOMMUState *s, hwaddr addr)
+{
+return ldl_le_p(&s->csr[addr]);
+}
+
+static void vtd_set_quad_raw(IntelIOMMUState *s, hwaddr addr, uint64_t val)
+{
+stq_le_p(&s->csr[addr], val);
+}
+
+static uint32_t vtd_set_clear_mask_long(IntelIOMMUState *s, hwaddr addr,
+uint32

[Qemu-devel] [Bug 1062220] Re: qemu-system-arm crashed with SIGABRT in cpu_abort()

[Apport retracing service]

** Tags added: utopic

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

Title:
  qemu-system-arm crashed with SIGABRT in cpu_abort()

Status in QEMU:
  New
Status in “qemu” package in Ubuntu:
  Incomplete
Status in “qemu-linaro” package in Ubuntu:
  Incomplete

Bug description:
  -kernel u-boot.bin

  ProblemType: Crash
  DistroRelease: Ubuntu 12.10
  Package: qemu-system 1.2.0-2012.09-0ubuntu1
  ProcVersionSignature: Ubuntu 3.5.0-10.10-generic 3.5.1
  Uname: Linux 3.5.0-10-generic x86_64
  NonfreeKernelModules: nvidia
  ApportVersion: 2.6.1-0ubuntu1
  Architecture: amd64
  CrashCounter: 1
  Date: Fri Oct  5 19:30:23 2012
  ExecutablePath: /usr/bin/qemu-system-arm
  InstallationMedia: Ubuntu 11.10 "Oneiric Ocelot" - Alpha amd64 (20110804)
  ProcCmdline: qemu-system-arm -M versatilepb -kernel u-boot.bin
  Signal: 6
  SourcePackage: qemu-linaro
  StacktraceTop:
   raise () from /lib/x86_64-linux-gnu/libc.so.6
   abort () from /lib/x86_64-linux-gnu/libc.so.6
   ?? ()
   ?? ()
   ?? ()
  Title: qemu-system-arm crashed with SIGABRT in raise()
  UpgradeStatus: Upgraded to quantal on 2012-08-11 (54 days ago)
  UserGroups: adm admin cdrom dialout lpadmin plugdev sambashare vboxusers

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

[Qemu-devel] [question] qemu-2.0.0 difference between download from http://wiki.qemu.org/Download and download http://git.qemu.org/qemu.git

[Zhang Haoyu]

Hi,

I download one copy of qemu-2.0.0 from http://wiki.qemu.org/Download,
and then download another copy of qemu-2.0.0 from http://git.qemu.org/qemu.git,
I compared them, found that dtc, pixman, roms are missed in the latter.
so If want to use qemu-2.0.1 to build my emulator, 
should I still add these external libs/sources (incorporated in the qemu-2.0.0 
from http://wiki.qemu.org/Download) to qemu-2.0.1 to build the emulator?

Thanks,
Zhang Haoyu

Re: [Qemu-devel] [PATCH 0/3] target-xtensa: fix loading uImage kernels on MMUv2 cores

[Max Filippov]

On Tue, Aug 12, 2014 at 8:22 AM, Max Filippov  wrote:
> Hi,
>
> this series fixes loading uImage kernels on MMUv2 xtensa cores.
>
> U-boot for xtensa always treats uImage load address as virtual address.
> This is important when booting uImage on xtensa core with MMUv2, because
> MMUv2 has fixed non-identity virtual-to-physical mapping after reset.
>
> I add two new functions: load_uboot_image_header that loads uImage header
> and load_uimage_at that loads uImage at the specified address, and use them
> to query uImage load address and load uImage at the correctly translated
> address.
>
> Max Filippov (3):
>   hw/core/loader: implement load_uboot_image_header
>   hw/core/loader: implement load_uimage_at
>   target-xtensa: treat uImage load address as virtual
>
>  hw/core/loader.c| 62 
> +
>  hw/xtensa/xtfpga.c  |  9 +++-
>  include/hw/loader.h |  4 
>  3 files changed, 60 insertions(+), 15 deletions(-)

Michael, Alexander, Gerd, Igor, Peter or anyone interested,

could you please review the changes to the generic uimage loader code
made in this series?

-- 
Thanks.
-- Max

Re: [Qemu-devel] [000/108] Patch Round-up for stable 2.0.1, freeze on 2014-08-12

[Eric Blake]

On 08/15/2014 03:01 PM, Michael Roth wrote:

>>> I tried to compile on Fedora 20, but had to backport this to get it to work:
>>>
>>> Luiz Capitulino
>>> a49db98d fpu: softfloat: drop INLINE macro
>>>
>>
>> ping
> 
> Hmm, unfortunately I didn't see this until after 2.0.1 was tagged/pushed. My
> understanding however was that the build issues you were seeing were the 
> result
> of a dirty build directory. Can you confirm whether or not the build issue
> is present in v2.0.1 (now tagged in origin)? If a build fix is still required
> I can push a minor v2.0.2 update monday and skip v2.0.1 for the
> announcement/tarball.

Using Fedora 20, I did a fresh clone to v2.0.1, followed by:

 ./configure --enable-kvm --enable-system --disable-user \
   --target-list=x86_64-softmmu --enable-debug

and make failed:

In file included from /usr/include/spice-server/spice.h:25:0,
 from /home/eblake/qemu-tmp/include/ui/qemu-spice.h:25,
 from qemu-char.c:84:
/usr/include/spice-1/spice/macros.h:105:0: error: "INLINE" redefined
[-Werror]
 #define INLINE inline
 ^

Bummer. It's unrelated to a dirty build directory.  I don't know how
many others will be impacted, to know if it warrants an immediate 2.0.2
(if it's just me hitting it, I don't mind calling 2.0.1 good; and distro
packagers can do the trivial backport that I did).

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] vexpress-a9: coreboot is unable to push any data on stack

[Peter Maydell]

On 15 August 2014 19:02, Piotr Król  wrote:
> On Fri, Aug 15, 2014 at 05:10:04PM +0100, Peter Maydell wrote:
>> For your stack issues, it looks like your code is trying to
>> use the area which is the flash as the stack. Since flash
>> isn't writeable, we ignore the writes and it's not very
>> useful for stack. It looks like your code is assuming that
>> the low memory is RAM, not flash -- so how does your
>> code work on real hardware? Do you try to use the
>> software controllable remapping to copy from the flash
>> into RAM before using the stack, or something else?
>
> Peter thanks for your reply. I'm not coreboot developer so cannot
> advocate for their decision.

I'm not looking for advocacy, just for an explanation of
what it's doing, and in particular whether anybody's
ever tested this on real hardware. (Running guest code
only on QEMU is a recipe for bugs, because our emulation
is often not very accurate and if you only test on QEMU
you can end up accidentally relying on our bugs.)

> Bootblock that I sent is from build
> targeted on qemu and developer who initially wrote the code suggest
> using it with '-kernel' parameter. I would like to fix this code
> according to correct memory map.
>
> Comment in hw/arm/vexpress.c say that, as you wrote below, Versatile
> Express got two possible memory maps. Can you point me to exact
> documentation that you use as reference for vexpress implementation ?

The common motherboard docs:
http://infocenter.arm.com/help/topic/com.arm.doc.dui0447j/index.html
A9 daughterboard docs:
http://infocenter.arm.com/help/topic/com.arm.doc.dui0448h/index.html
A15 daughterboard docs:
http://infocenter.arm.com/help/topic/com.arm.doc.dui0604e/index.html

thanks
-- PMM

Re: [Qemu-devel] QEMU, self-modifying code, and Windows 7 64-bit (no KVM)

[Hulin, Patrick - 0559 - MITLL]

On Aug 15, 2014, at 4:48 PM, Paolo Bonzini  wrote:

> Il 13/08/2014 20:36, Hulin, Patrick - 0559 - MITLL ha scritto:
>> Hi QEMU devs,
>> 
>> QEMU 2.10 does not currently run Windows 7 64-bit without KVM. There
>> have been a few threads about this over the past few years (such as
>> https://bugs.launchpad.net/qemu/+bug/921208 and
>> http://lists.gnu.org/archive/html/qemu-devel/2012-09/msg02603.html),
>> but the problem was never resolved. I think I've identified the
>> cause, but I am not sure what the correct way to fix it is. I'm
>> working on PANDA, a set of analysis extensions to QEMU
>> (github.com/moyix/panda) and I'd really like to be able to use our
>> analyses on Windows 7 64-bit.
>> 
>> There are two issues right now. The first is that QEMU is missing a
>> CPUID bit (for debug extensions, CPUID_DE) because the feature isn't
>> implemented in QEMU. This can easily be hacked around by just
>> enabling the bit, but I imagine you all aren't excited about
>> advertising features that don't exist.
> 
> Not too worried about it either.
> 
> The two aspects of CPUID.DE are: 1) support for CR4.DE and raising an
> exception for DR4 and DR5 access; 2) I/O breakpoints.  Now, QEMU always
> raises the exception even if CR4.DE=0, and doesn't complain if you set
> bits of CR4 that ought to be reserved according to CPUID.  And I/O
> breakpoints aren't that hard to implement.  Having a full implementation
> of CPUID.DE wouldn't be hard, but I wouldn't have much problems with
> just setting the bit in TCG_FEATURES and noting that it is only
> partially implemented.
> 
>> The second issue is that both
>> the installer and the OS itself fail with blue screens of
>> DRIVER_IRQL_NOT_LESS_OR_EQUAL or KMODE_EXCEPTION_NOT_HANDLED (due to
>> illegal instruction). This is a little trickier.
> 
> Indeed.  Thanks for debugging it.
> 
>> Before executing a translation block, QEMU write-protects (using
>> host MMU features) the _host_ page that contains the section of guest 
>> memory on which the guest TB code lives.
> 
> Are you sure?  My knowledge of this code is only cursory, but I think
> that only applies to user-mode emulation.  System emulation uses softmmu
> exclusively to trap such writes (TLB_NOTDIRTY or something like that).

Certainly possible. I don’t really understand this code either.

> 
>> In this case, the write is 8 bytes and unaligned, so it gets split
>> into 8 single-byte writes. In stock QEMU, these writes are done in
>> reverse order (see the loop in softmmu_template.h, line 402). The
>> third decryption xor from Kernel Patch Protection should hit 4 bytes
>> that are in the current TB and 4 bytes in the TB afterwards in linear
>> order. Since this happens in reverse order, and the last 4 bytes of
>> the write do not intersect the current TB, those writes happen
>> successfully and QEMU's memory is modified. The 4th byte in linear
>> order (the 5th in temporal order) then triggers the
>> current_tb_modified flag and cpu_restore_state, longjmp'ing out.
>> 
>> I am not sure how to fix this issue. For now, in our tool, PANDA, we
>> have just reversed the order of the loop. But that change will fail
>> in any situation in which the write happens off the front end of the
>> TB and then the self-modifying code loops back to the previous TB.
>> This modification enables Windows 7 x64 to run successfully without
>> KVM, which is all we really need for our purposes.
> 
> Would it work to just call tb_invalidate_phys_page_range before the
> helper_ret_stb loop?
> 
> Paolo

Maybe. I think there’s another issue, which is that QEMU’s ending up in the I/O 
read/write code instead of the normal memory RW. This could be QEMU messing up, 
it could be PatchGuard doing something weird, or it could be me 
misunderstanding what’s going on. I never really figured out how the control 
flow works here.

I’m going to look at this more next week—I’ll see if I have more info then.

[Qemu-devel] [Bug 1350435] Re: tcg.c:1693: tcg fatal error

[LocutusOfBorg]

I didn't install a VM because:
-I'm on ubuntu 14.04 on my laptop
-I use pbuilder-dist (from ubuntu-dev-tools) armhf that uses qemu as underlying 
virtualization system.

the fact is:
why on my machine it doesn't show this error?

possible solution:
-because qemu+pbuilder-dist is not multithreaded?

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

Title:
  tcg.c:1693: tcg fatal error

Status in launchpad-buildd:
  Triaged
Status in QEMU:
  New
Status in “qemu” package in Ubuntu:
  Confirmed

Bug description:
  this started happening after the launchpad buildd trusty deploy
  
https://code.launchpad.net/~costamagnagianfranco/+archive/ubuntu/firefox/+build/6224439

  
  debconf-updatepo
  qemu: uncaught target signal 11 (Segmentation fault) - core dumped
  Segmentation fault (core dumped)
  qemu: uncaught target signal 11 (Segmentation fault) - core dumped
  Segmentation fault (core dumped)
  /build/buildd/qemu-2.0.0+dfsg/tcg/tcg.c:1693: tcg fatal error
  /build/buildd/qemu-2.0.0+dfsg/tcg/tcg.c:1693: tcg fatal error

  this seems to be the patch needed
  https://patches.linaro.org/32473/

To manage notifications about this bug go to:
https://bugs.launchpad.net/launchpad-buildd/+bug/1350435/+subscriptions

[Qemu-devel] [Bug 1350435] Re: tcg.c:1693: tcg fatal error

[Serge Hallyn]

Perhaps i don't understand what you are doing.

My understanding was that a package is being built (in the buildds)
under qemu.  Qemu is failing due to tcg failure.  We want to tes
twhether a qemu patch fixes it.

That's why I suggest installing the new qemu package on your host, using
it to run a new server VM, and building the problematic package inside
that VM.  That way we can see whether the proposed qemu package fixes
the build error.

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

Title:
  tcg.c:1693: tcg fatal error

Status in launchpad-buildd:
  Triaged
Status in QEMU:
  New
Status in “qemu” package in Ubuntu:
  Confirmed

Bug description:
  this started happening after the launchpad buildd trusty deploy
  
https://code.launchpad.net/~costamagnagianfranco/+archive/ubuntu/firefox/+build/6224439

  
  debconf-updatepo
  qemu: uncaught target signal 11 (Segmentation fault) - core dumped
  Segmentation fault (core dumped)
  qemu: uncaught target signal 11 (Segmentation fault) - core dumped
  Segmentation fault (core dumped)
  /build/buildd/qemu-2.0.0+dfsg/tcg/tcg.c:1693: tcg fatal error
  /build/buildd/qemu-2.0.0+dfsg/tcg/tcg.c:1693: tcg fatal error

  this seems to be the patch needed
  https://patches.linaro.org/32473/

To manage notifications about this bug go to:
https://bugs.launchpad.net/launchpad-buildd/+bug/1350435/+subscriptions

Re: [Qemu-devel] [000/108] Patch Round-up for stable 2.0.1, freeze on 2014-08-12

[Michael Roth]

Quoting Eric Blake (2014-08-15 13:43:21)
> On 08/07/2014 02:21 PM, Eric Blake wrote:
> > On 08/06/2014 02:38 PM, Michael Roth wrote:
> >> Hi everyone,
> >>
> >> The following new patches are queued for QEMU stable v2.0.1:
> >>
> >>   https://github.com/mdroth/qemu/commits/stable-2.0-staging
> >>
> >> The release is planned for 2014-08-15:
> >>
> >>   http://wiki.qemu.org/Planning/2.0
> >>
> >> Please respond here or CC qemu-sta...@nongnu.org on any patches you
> >> think should be included in the release.
> >>
> >> Due to delays, this is the final planned release for the 2.0.0 series.
> >> We will return to the standard 2-release cycle for 2.1 (one midway
> >> during 2.2 development cycle, one immediately following 2.2 release)
> >>
> >> Testing/feedback is greatly appreciated.
> >>
> > 
> > I tried to compile on Fedora 20, but had to backport this to get it to work:
> > 
> > Luiz Capitulino
> > a49db98d fpu: softfloat: drop INLINE macro
> > 
> 
> ping

Hmm, unfortunately I didn't see this until after 2.0.1 was tagged/pushed. My
understanding however was that the build issues you were seeing were the result
of a dirty build directory. Can you confirm whether or not the build issue
is present in v2.0.1 (now tagged in origin)? If a build fix is still required
I can push a minor v2.0.2 update monday and skip v2.0.1 for the
announcement/tarball.

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

Re: [Qemu-devel] QEMU, self-modifying code, and Windows 7 64-bit (no KVM)

[Paolo Bonzini]

Il 13/08/2014 20:36, Hulin, Patrick - 0559 - MITLL ha scritto:
> Hi QEMU devs,
> 
> QEMU 2.10 does not currently run Windows 7 64-bit without KVM. There
> have been a few threads about this over the past few years (such as
> https://bugs.launchpad.net/qemu/+bug/921208 and
> http://lists.gnu.org/archive/html/qemu-devel/2012-09/msg02603.html),
> but the problem was never resolved. I think I've identified the
> cause, but I am not sure what the correct way to fix it is. I'm
> working on PANDA, a set of analysis extensions to QEMU
> (github.com/moyix/panda) and I'd really like to be able to use our
> analyses on Windows 7 64-bit.
> 
> There are two issues right now. The first is that QEMU is missing a
> CPUID bit (for debug extensions, CPUID_DE) because the feature isn't
> implemented in QEMU. This can easily be hacked around by just
> enabling the bit, but I imagine you all aren't excited about
> advertising features that don't exist.

Not too worried about it either.

The two aspects of CPUID.DE are: 1) support for CR4.DE and raising an
exception for DR4 and DR5 access; 2) I/O breakpoints.  Now, QEMU always
raises the exception even if CR4.DE=0, and doesn't complain if you set
bits of CR4 that ought to be reserved according to CPUID.  And I/O
breakpoints aren't that hard to implement.  Having a full implementation
of CPUID.DE wouldn't be hard, but I wouldn't have much problems with
just setting the bit in TCG_FEATURES and noting that it is only
partially implemented.

> The second issue is that both
> the installer and the OS itself fail with blue screens of
> DRIVER_IRQL_NOT_LESS_OR_EQUAL or KMODE_EXCEPTION_NOT_HANDLED (due to
> illegal instruction). This is a little trickier.

Indeed.  Thanks for debugging it.

> Before executing a translation block, QEMU write-protects (using
> host MMU features) the _host_ page that contains the section of guest 
> memory on which the guest TB code lives.

Are you sure?  My knowledge of this code is only cursory, but I think
that only applies to user-mode emulation.  System emulation uses softmmu
exclusively to trap such writes (TLB_NOTDIRTY or something like that).

> In this case, the write is 8 bytes and unaligned, so it gets split
> into 8 single-byte writes. In stock QEMU, these writes are done in
> reverse order (see the loop in softmmu_template.h, line 402). The
> third decryption xor from Kernel Patch Protection should hit 4 bytes
> that are in the current TB and 4 bytes in the TB afterwards in linear
> order. Since this happens in reverse order, and the last 4 bytes of
> the write do not intersect the current TB, those writes happen
> successfully and QEMU's memory is modified. The 4th byte in linear
> order (the 5th in temporal order) then triggers the
> current_tb_modified flag and cpu_restore_state, longjmp'ing out.
>
> I am not sure how to fix this issue. For now, in our tool, PANDA, we
> have just reversed the order of the loop. But that change will fail
> in any situation in which the write happens off the front end of the
> TB and then the self-modifying code loops back to the previous TB.
> This modification enables Windows 7 x64 to run successfully without
> KVM, which is all we really need for our purposes.

Would it work to just call tb_invalidate_phys_page_range before the
helper_ret_stb loop?

Paolo

Re: [Qemu-devel] [V2 PATCH 7/8] target-ppc: Bug Fix: srawi

[Richard Henderson]

On 08/12/2014 03:45 AM, Tom Musta wrote:
> For 64 bit implementations, the special case of a shift by zero
> should result in the sign extension of the least significant 32 bits
> of the source GPR (not a direct copy of the 64 bit source GPR).
> 
> Example:
> 
> R3 A6212433228F41DC
> srawi 3,3,0
> R3 expected : 228F41DC
> R3 actual   : A6212433228F41DC (without this patch)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [V2 PATCH 4/8] target-ppc: Bug Fix: mullw

[Richard Henderson]

On 08/12/2014 03:45 AM, Tom Musta wrote:
> +#else
>  tcg_gen_mul_tl(cpu_gpr[rD(ctx->opcode)], cpu_gpr[rA(ctx->opcode)],
> cpu_gpr[rB(ctx->opcode)]);
>  tcg_gen_ext32s_tl(cpu_gpr[rD(ctx->opcode)], cpu_gpr[rD(ctx->opcode)]);
> +#endif

Note that the sign-extension can be dropped, since this is ifdef PPC32.
Perhaps even better to use tcg_gen_mul_i32 explicitly?  That at least
type-checks the arguments are all 32-bit as well.


r~

Re: [Qemu-devel] [V2 PATCH 2/8] target-ppc: Bug Fix: rlwnm

[Richard Henderson]

On 08/12/2014 03:45 AM, Tom Musta wrote:
> The rlwnm specification includes the ROTL32 operation, which is defined
> to be a left rotation of two copies of the least significant 32 bits of
> the source GPR.
> 
> The current implementation is incorrect on 64-bit implementations in that
> it rotates a single copy of the least significant 32 bits, padding with
> zeroes in the most significant bits.
> 
> Fix the code to properly implement this ROTL32 operation.
> 
> Example:
> 
> R3 = 0002
> R4 = 7FFF
> rlwnm 3,3,4,31,16
> R3 expected : 00010001
> R3 actual   : 0001 (without this patch)
> 
> Signed-off-by: Tom Musta 
> ---
>  target-ppc/translate.c |   18 +-
>  1 files changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index a27d063..48f13a9 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -1695,7 +1695,7 @@ static void gen_rlwnm(DisasContext *ctx)
>  uint32_t mb, me;
>  TCGv t0;
>  #if defined(TARGET_PPC64)
> -TCGv_i32 t1, t2;
> +TCGv t1;
>  #endif
>  
>  mb = MB(ctx->opcode);
> @@ -1703,14 +1703,11 @@ static void gen_rlwnm(DisasContext *ctx)
>  t0 = tcg_temp_new();
>  tcg_gen_andi_tl(t0, cpu_gpr[rB(ctx->opcode)], 0x1f);
>  #if defined(TARGET_PPC64)
> -t1 = tcg_temp_new_i32();
> -t2 = tcg_temp_new_i32();
> -tcg_gen_trunc_i64_i32(t1, cpu_gpr[rS(ctx->opcode)]);
> -tcg_gen_trunc_i64_i32(t2, t0);
> -tcg_gen_rotl_i32(t1, t1, t2);
> -tcg_gen_extu_i32_i64(t0, t1);
> -tcg_temp_free_i32(t1);
> -tcg_temp_free_i32(t2);
> +t1 = tcg_temp_new_i64();
> +tcg_gen_deposit_i64(t1, cpu_gpr[rS(ctx->opcode)],
> +cpu_gpr[rS(ctx->opcode)], 32, 32);
> +tcg_gen_rotl_i64(t0, t1, t0);
> +tcg_temp_free_i64(t1);
>  #else
>  tcg_gen_rotl_i32(t0, cpu_gpr[rS(ctx->opcode)], t0);
>  #endif
> @@ -1721,6 +1718,9 @@ static void gen_rlwnm(DisasContext *ctx)
>  #endif
>  tcg_gen_andi_tl(cpu_gpr[rA(ctx->opcode)], t0, MASK(mb, me));
>  } else {
> +#if defined(TARGET_PPC64)
> +tcg_gen_andi_tl(t0, t0, MASK(32, 63));
> +#endif

Similarly, checking early for the MB==0, ME==31 case and generating a straight
32-bit rotate would be good.  Note that ANDI will already special case for the
constant -1, so there's no need to check for that as a mov.


r~

Re: [Qemu-devel] [V2 PATCH 1/8] target-ppc: Bug Fix: rlwinm

[Richard Henderson]

On 08/12/2014 03:45 AM, Tom Musta wrote:
> The rlwinm specification includes the ROTL32 operation, which is defined
> to be a left rotation of two copies of the least significant 32 bits of
> the source GPR.
> 
> The current implementation is incorrect on 64-bit implementations in that
> it rotates a single copy of the least significant 32 bits, padding with
> zeroes in the most significant bits.
> 
> Fix the code to properly implement this ROTL32 operation.
> 
> Example:
> R3 = F7487D82EC6F75DF
> rlwinm 3,3,5,12,4
> 
> R3 expected : 8DEEBBFD880EBBFD
> R3 actual   : 880EBBFD (without this fix)
> 
> Signed-off-by: Tom Musta 
> ---
>  target-ppc/translate.c |8 +++-
>  1 files changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index b23933f..a27d063 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -1672,11 +1672,9 @@ static void gen_rlwinm(DisasContext *ctx)
>  } else {
>  TCGv t0 = tcg_temp_new();
>  #if defined(TARGET_PPC64)
> -TCGv_i32 t1 = tcg_temp_new_i32();
> -tcg_gen_trunc_i64_i32(t1, cpu_gpr[rS(ctx->opcode)]);
> -tcg_gen_rotli_i32(t1, t1, sh);
> -tcg_gen_extu_i32_i64(t0, t1);
> -tcg_temp_free_i32(t1);
> +tcg_gen_deposit_i64(t0, cpu_gpr[rS(ctx->opcode)],
> +cpu_gpr[rS(ctx->opcode)], 32, 32);
> +tcg_gen_rotli_i64(t0, t0, sh);
>  #else
>  tcg_gen_rotli_i32(t0, cpu_gpr[rS(ctx->opcode)], sh);
>  #endif
> 

You might want to retain a special case for mb==0 and me==31 which would be a
straight 32-bit rotate with subsequent zero-extension.  Just like we've got
special cases for the normal shifts.


r~

Re: [Qemu-devel] [PULL 00/62] Block patches

[Paolo Bonzini]

Il 15/08/2014 15:10, Kevin Wolf ha scritto:
> Am 15.08.2014 um 14:41 hat Peter Maydell geschrieben:
>> On 8 August 2014 18:39, Kevin Wolf  wrote:
>>> The following changes since commit 69f87f713069f1f70f86cb65883f7d43e3aa21de:
>>>
>>>   Merge remote-tracking branch 
>>> 'remotes/pmaydell/tags/pull-target-arm-20140804' into staging (2014-08-04 
>>> 15:01:38 +0100)
>>>
>>> are available in the git repository at:
>>>
>>>
>>>   git://repo.or.cz/qemu/kevin.git tags/for-upstream
>>>
>>> for you to fetch changes up to d61a5d3fdfe1d15ded921a5c25307bec8cf424b1:
>>>
>>>   block: Catch !bs->drv in bdrv_check() (2014-08-08 11:10:36 +0200)
>>
>> Hi. I'm afraid this doesn't build on w32:
>> [...]
> 
> Sorry about that. I dropped the faulty series and pushed again. The tag
> is now at 908bcd540f489f7adf2d804347905b0025d808d3 and builds for me
> with mingw.

There are patches on the list to provide sockets on Win32 and let Max's
patches work.  I'll respin when I'm back.

Paolo

[Qemu-devel] Using cache=writeback safely on qemu 1.4.0 and later

[Andrew Martin]

Hello,

I am running several qemu-kvm VM servers on Ubuntu 12.04 with qemu-kvm 1.4.0.
Most of the guests are also running Ubuntu 12.04. I am using qcow2 disk images
with the virtio driver in almost all cases, and am storing the disks on two
types of storage devices: 
* a shared NFS server mounted on each host 
* local ext4 filesystems on each host (on top of an md RAID1 of HDDs)

It seems like the consensus for NFS is to use cache=none in order to support
live migration between hosts.

For local ext4 filesystems, I see from this message and elsewhere that
cache=writeback is now the default disk cache mode, and is safe on ext4:
http://lists.gnu.org/archive/html/qemu-devel/2012-02/msg02689.html

All of the hosts and guests are using ext4 (with the default
data=ordered,barrier). Thus it seems probable that the kernel (3.2.x or newer)
should support flushing correctly? Is there a way to check in the guest is
utilizing WCE (or any other required components) correctly?

I recently experienced UPS failure on several hosts which caused a hard
shutdown. After restarting, 3 of the guests had corruption on their disks and
required a fairly long fsck to fix. Afterwards, data that had been written to
the disks several hours before the crash was corrupted, which makes me think
that it was never fsync()-ed to the non-volatile storage.

Is it safe in this setup to use cache=writeback? Or, should I use
cache=writethrough instead?

Thanks,

Andrew Martin

Re: [Qemu-devel] [PATCH 8/8] target-ppc: Bug Fix: srad

[Richard Henderson]

On 08/11/2014 09:23 AM, Tom Musta wrote:
> Fix the check for carry in the srad helper to properly construct
> the mask -- a "1ULL" must be used (instead of "1") in order to
> get the desired result.
> 
> Signed-off-by: Tom Musta 
> ---
>  target-ppc/int_helper.c |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v1 00/17] dataplane: optimization and multi virtqueue support

[Paolo Bonzini]

Il 14/08/2014 11:39, Stefan Hajnoczi ha scritto:
> That begs the question whether you should look at PCI passthrough
> instead?

Being able to use logical volumes, or to access multiple remote LUNs
through a single FC card in the host is an obvious reason to avoid PCI
passthrough.

Paolo

Re: [Qemu-devel] [PATCH 6/8] target-ppc: Bug Fix: mulldo OV Detection

[Richard Henderson]

On 08/11/2014 09:23 AM, Tom Musta wrote:
> Fix the code to properly detect overflow; the 128 bit signed
> product must have all zeroes or all ones in the first 65 bits
> otherwise OV should be set.
> 
> Signed-off-by: Tom Musta 
> ---
>  target-ppc/int_helper.c |   14 --
>  1 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/target-ppc/int_helper.c b/target-ppc/int_helper.c
> index f6e8846..e83a25d 100644
> --- a/target-ppc/int_helper.c
> +++ b/target-ppc/int_helper.c
> @@ -32,12 +32,22 @@ uint64_t helper_mulldo(CPUPPCState *env, uint64_t arg1, 
> uint64_t arg2)
>  uint64_t tl;
>  
>  muls64(&tl, (uint64_t *)&th, arg1, arg2);
> -/* If th != 0 && th != -1, then we had an overflow */
> -if (likely((uint64_t)(th + 1) <= 1)) {
> +
> +/* th should either contain all 1 bits or all 0 bits and should
> + * match the sign bit of tl; otherwise we have overflowed. */
> +
> +if ((int64_t)tl < 0) {
> +if (likely(th == -1LL)) {
> +env->ov = 0;
> +} else {
> +env->so = env->ov = 1;
> +}
> +} else if (likely(th == 0LL)) {
>  env->ov = 0;
>  } else {
>  env->so = env->ov = 1;
>  }
> +

As far as it goes,
Reviewed-by: Richard Henderson 

But we can just as easily implement this inline, as we do for mullwo.  We've
added a tcg_gen_muls2_i64 since this code was written.


r~

Re: [Qemu-devel] [PATCH v1 00/17] dataplane: optimization and multi virtqueue support

[Paolo Bonzini]

Il 14/08/2014 12:46, Kevin Wolf ha scritto:
> So to finally reply with some numbers... I'm running fio tests based on
> Ming's configuration on a loop-mounted tmpfs image using dataplane.

I'm not sure tmpfs is a particularly useful comparison, since it doesn't
support O_DIRECT.  O_DIRECT over ramdisk ("modprobe brd rd_nr=1
rd_size=524288 max_part=1", either directly or via a filesystem) is
probably a better benchmark.

Also, I'm not sure how the I/O scheduler works over tmpfs.  A ramdisk
should just do the right thing.  (Are you using deadline or cfq?)

> | Random throughput | Sequential throughput
> +---+---
> master  | 442 MB/s  | 730 MB/s
> base| 453 MB/s  | 757 MB/s
> bypass (Ming)   | 461 MB/s  | 734 MB/s
> coroutine   | 468 MB/s  | 716 MB/s
> bypass (Paolo)  | 476 MB/s  | 682 MB/s

This is pretty large, but it really smells like either a setup problem
or a kernel bug...

Paolo

Re: [Qemu-devel] [PATCH 5/8] target-ppc: Bug Fix: mullwo

[Richard Henderson]

On 08/11/2014 09:23 AM, Tom Musta wrote:
>  tcg_gen_muls2_i32(t0, t1, t0, t1);
>  tcg_gen_ext_i32_tl(cpu_gpr[rD(ctx->opcode)], t0);
> +#if defined(TARGET_PPC64)
> +tcg_gen_ext_i32_tl(t2, t1);
> +tcg_gen_deposit_i64(cpu_gpr[rD(ctx->opcode)],
> +cpu_gpr[rD(ctx->opcode)], t2, 32, 32);
> +tcg_temp_free(t2);
> +#endif

This is

  tcg_gen_muls2_i32(t0, t0, t0, t1);
#if defined(TARGET_PPC64)
  tcg_gen_concat_i32_i64(cpu_gpr[rD(ctx->opcode)], t0, t1);
#else
  tcg_gen_mov_i32(cpu_gpr[rD(ctx->opcode)], t0);
#endif


r~

Re: [Qemu-devel] [PATCH 4/8] target-ppc: Bug Fix: mullw

[Richard Henderson]

On 08/11/2014 09:23 AM, Tom Musta wrote:
> For 64-bit implementations, the mullw result is the 64 bit product
> of the sign-extended least significant 32 bits of the source
> registers.
> 
> Fix the code to properly sign extend the source operands and produce
> a 64 bit product.
> 
> Signed-off-by: Tom Musta 
> ---
>  target-ppc/translate.c |   11 +++
>  1 files changed, 11 insertions(+), 0 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH 3/8] target-ppc: Bug Fix: rlwimi

[Richard Henderson]

On 08/11/2014 09:23 AM, Tom Musta wrote:
> Also fix the special case of MB=31 and ME=0 to copy the entire contents
> of the source GPR.

Err, that's not what you did.

>  if (likely(sh == 0 && mb == 0 && me == 31)) {
> +#if defined(TARGET_PPC64)
> +tcg_gen_mov_i64(cpu_gpr[rA(ctx->opcode)], cpu_gpr[rS(ctx->opcode)]);
> +#else
>  tcg_gen_ext32u_tl(cpu_gpr[rA(ctx->opcode)], 
> cpu_gpr[rS(ctx->opcode)]);
> +#endif

This is the reverse condition.  Which, true enough, should not be implemented
with ext32u for PPC64.  But a MOV isn't right either, it is

  deposit(ra, rs, 0, 32)

Which does point out that we should probably implement anything MB <= ME and SH
== 31 - ME with the deposit opcode.


r~

Re: [Qemu-devel] [PATCH 2/8] target-ppc: Bug Fix: rlwnm

[Richard Henderson]

On 08/11/2014 09:23 AM, Tom Musta wrote:
> +#if defined(TARGET_PPC64)
> +tcg_gen_andi_tl(t0, t0, MASK(32, 63));
> +#endif

Err. this is just simple zero-extension.

>  tcg_gen_mov_tl(cpu_gpr[rA(ctx->opcode)], t0);

Why not combine with the mov as

  tcg_gen_ext32u_tl

which will just expand to a mov for PPC32.


r~

Re: [Qemu-devel] [PATCH 1/8] target-ppc: Bug Fix: rlwinm

[Richard Henderson]

On 08/15/2014 08:34 AM, Richard Henderson wrote:
> On 08/11/2014 09:23 AM, Tom Musta wrote:
>> The rlwinm specification includes the ROTL32 operation, which is defined
>> to be a left rotation of two copies of the least significant 32 bits of
>> the source GPR.
>>
>> The current implementation is incorrect on 64-bit implementations in that
>> it rotates a single copy of the least significant 32 bits, padding with
>> zeroes in the most significant bits.
> 
> Yes, it does describe rotate_32 as a double-copy of the low 32 bits.  But it
> also describes the mask as having "0 bits elsewhere".  Thus, post mask, I 
> don't
> see how you could distinguish the implementations.
> 
> Have you an example that doesn't work with the current code?

Let me guess the answer myself -- it's MB > ME, and wraparound of the mask.

I think I was distracted by the text "For all the uses given above, the
high-order 32 bits of register RA are cleared." without clearly reading the
examples.


r~

Re: [Qemu-devel] [000/108] Patch Round-up for stable 2.0.1, freeze on 2014-08-12

[Eric Blake]

On 08/07/2014 02:21 PM, Eric Blake wrote:
> On 08/06/2014 02:38 PM, Michael Roth wrote:
>> Hi everyone,
>>
>> The following new patches are queued for QEMU stable v2.0.1:
>>
>>   https://github.com/mdroth/qemu/commits/stable-2.0-staging
>>
>> The release is planned for 2014-08-15:
>>
>>   http://wiki.qemu.org/Planning/2.0
>>
>> Please respond here or CC qemu-sta...@nongnu.org on any patches you
>> think should be included in the release.
>>
>> Due to delays, this is the final planned release for the 2.0.0 series.
>> We will return to the standard 2-release cycle for 2.1 (one midway
>> during 2.2 development cycle, one immediately following 2.2 release)
>>
>> Testing/feedback is greatly appreciated.
>>
> 
> I tried to compile on Fedora 20, but had to backport this to get it to work:
> 
> Luiz Capitulino
> a49db98d fpu: softfloat: drop INLINE macro
> 

ping

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v2 1/2] QEMUSizedBuffer based QEMUFile

[Eric Blake]

On 08/07/2014 04:24 AM, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" 
> 
> This is based on Stefan and Joel's patch that creates a QEMUFile that goes
> to a memory buffer; from:
> 
> http://lists.gnu.org/archive/html/qemu-devel/2013-03/msg05036.html
> 
> Using the QEMUFile interface, this patch adds support functions for
> operating on in-memory sized buffers that can be written to or read from.
> 
> Signed-off-by: Stefan Berger 
> Signed-off-by: Joel Schopp 
> 
> For minor tweeks/rebase I've done to it:
> Signed-off-by: Dr. David Alan Gilbert 
> ---
>  include/migration/qemu-file.h |  28 +++
>  include/qemu/typedefs.h   |   1 +
>  qemu-file.c   | 410 
> ++
>  3 files changed, 439 insertions(+)
> 

>  
> +QEMUSizedBuffer *qsb_create(const uint8_t *buffer, size_t len);
> +QEMUSizedBuffer *qsb_clone(const QEMUSizedBuffer *);
> +void qsb_free(QEMUSizedBuffer *);
> +size_t qsb_set_length(QEMUSizedBuffer *qsb, size_t length);

Just on the prototype, I wonder if this should return ssize_t, to allow
for the possibility of failure...

> +/**
> + * Create a QEMUSizedBuffer
> + * This type of buffer uses scatter-gather lists internally and
> + * can grow to any size. Any data array in the scatter-gather list
> + * can hold different amount of bytes.
> + *
> + * @buffer: Optional buffer to copy into the QSB
> + * @len: size of initial buffer; if @buffer is given, buffer must
> + *   hold at least len bytes
> + *
> + * Returns a pointer to a QEMUSizedBuffer
> + */
> +QEMUSizedBuffer *qsb_create(const uint8_t *buffer, size_t len)
> +{
> +QEMUSizedBuffer *qsb;
> +size_t alloc_len, num_chunks, i, to_copy;
> +size_t chunk_size = (len > QSB_MAX_CHUNK_SIZE)
> +? QSB_MAX_CHUNK_SIZE
> +: QSB_CHUNK_SIZE;
> +
> +if (len == 0) {
> +/* we want to allocate at least one chunk */
> +len = QSB_CHUNK_SIZE;
> +}

So if I call qsb_create("", 0), len is now QSB_CHUNK_SIZE...

> +
> +num_chunks = DIV_ROUND_UP(len, chunk_size);

...num_chunks is now 1...

> +alloc_len = num_chunks * chunk_size;
> +
> +qsb = g_new0(QEMUSizedBuffer, 1);
> +qsb->iov = g_new0(struct iovec, num_chunks);
> +qsb->n_iov = num_chunks;
> +
> +for (i = 0; i < num_chunks; i++) {
> +qsb->iov[i].iov_base = g_malloc0(chunk_size);
> +qsb->iov[i].iov_len = chunk_size;
> +if (buffer) {

...we enter the loop, and have a buffer to copy...

> +to_copy = (len - qsb->used) > chunk_size
> +  ? chunk_size : (len - qsb->used);
> +memcpy(qsb->iov[i].iov_base, &buffer[qsb->used], to_copy);

...and proceed to dereference QSB_CHUNK_SIZE bytes beyond the end of the
empty string that we are converting to a buffer.  Oops.

Might be as simple as adding if (buffer) { assert(len); } before
modifying len.

> +/**
> + * Set the length of the buffer; the primary usage of this
> + * function is to truncate the number of used bytes in the buffer.
> + * The size will not be extended beyond the current number of
> + * allocated bytes in the QEMUSizedBuffer.
> + *
> + * @qsb: A QEMUSizedBuffer
> + * @new_len: The new length of bytes in the buffer
> + *
> + * Returns the number of bytes the buffer was truncated or extended
> + * to.

Confusing; I'd suggest:

Returns the resulting (possibly new) size of the buffer.  Oh, and my
earlier question appears to be answered - this function can't fail.

> +ssize_t qsb_get_buffer(const QEMUSizedBuffer *qsb, off_t start,
> +   size_t count, uint8_t **buf)
> +{
> +uint8_t *buffer;
> +const struct iovec *iov;
> +size_t to_copy, all_copy;
> +ssize_t index;
> +off_t s_off;
> +off_t d_off = 0;
> +char *s;
> +
> +if (start > qsb->used) {
> +return 0;
> +}

It feels confusing that this can return 0 while leaving buf un-malloc'd.
 It's probably simpler to callers if a non-negative return value ALWAYS
guarantees that buf is valid.


> +
> +index = qsb_get_iovec(qsb, start, &s_off);
> +if (index < 0) {
> +return 0;
> +}

Wait - how is a negative value possible?  Since we already checked
against qsb->used, can't this be assert(index >= 0)?

> +static ssize_t qsb_grow(QEMUSizedBuffer *qsb, size_t new_size)
> +{
> +size_t needed_chunks, i;
> +size_t chunk_size = QSB_CHUNK_SIZE;
> +
> +if (qsb->size < new_size) {
> +needed_chunks = DIV_ROUND_UP(new_size - qsb->size,
> + chunk_size);
> +

Hmm. Original creation will use chunks larger than QSB_CHUNK_SIZE (up to
the maximum chunk size), presumably for efficiency.  Should this
function likewise use larger chunks if the size to be grown is larger
than the default chunk size?

> +qsb->iov = g_realloc_n(qsb->iov, qsb->n_iov + needed_chunks,
> +   sizeof(struct iovec));
> +if (qsb->iov == NULL)

Re: [Qemu-devel] [PATCH 1/8] target-ppc: Bug Fix: rlwinm

[Richard Henderson]

On 08/11/2014 09:23 AM, Tom Musta wrote:
> The rlwinm specification includes the ROTL32 operation, which is defined
> to be a left rotation of two copies of the least significant 32 bits of
> the source GPR.
> 
> The current implementation is incorrect on 64-bit implementations in that
> it rotates a single copy of the least significant 32 bits, padding with
> zeroes in the most significant bits.

Yes, it does describe rotate_32 as a double-copy of the low 32 bits.  But it
also describes the mask as having "0 bits elsewhere".  Thus, post mask, I don't
see how you could distinguish the implementations.

Have you an example that doesn't work with the current code?


r~

Re: [Qemu-devel] [PATCH v6 02/10] monitor: fix access freed memory

[Luiz Capitulino]

On Thu, 14 Aug 2014 12:30:10 +0200
"Michael S. Tsirkin"  wrote:

> On Thu, Aug 14, 2014 at 03:29:13PM +0800, zhanghailiang wrote:
> > The function monitor_fdset_dup_fd_find_remove() references member of 
> > 'mon_fdset'
> > which may be freed in function monitor_fdset_cleanup()
> > 
> > Reviewed-by: Gonglei 
> > Signed-off-by: zhanghailiang 
> 
> Id doesn't make sense after fdset is gone.
> A cleaner way would be return -1 within that remove
> clause.
> 
> --->
> 
> monitor: fix use after free
> 
> The function monitor_fdset_dup_fd_find_remove() references member of
> 'mon_fdset' which - when remove flag is set - may be freed in function
> monitor_fdset_cleanup().
> remove is set by monitor_fdset_dup_fd_remove which in practice
> does not need the returned value, so make it void,
> and return -1 from monitor_fdset_dup_fd_find_remove.
> 
> Signed-off-by: Michael S. Tsirkin 

Looks correct to me, can you post as a toplevel patch please?

> 
> ---
> 
> diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
> index 3d6929d..78a5fc8 100644
> --- a/include/monitor/monitor.h
> +++ b/include/monitor/monitor.h
> @@ -64,7 +64,7 @@ AddfdInfo *monitor_fdset_add_fd(int fd, bool has_fdset_id, 
> int64_t fdset_id,
>  Error **errp);
>  int monitor_fdset_get_fd(int64_t fdset_id, int flags);
>  int monitor_fdset_dup_fd_add(int64_t fdset_id, int dup_fd);
> -int monitor_fdset_dup_fd_remove(int dup_fd);
> +void monitor_fdset_dup_fd_remove(int dup_fd);
>  int monitor_fdset_dup_fd_find(int dup_fd);
>  
>  #endif /* !MONITOR_H */
> diff --git a/monitor.c b/monitor.c
> index 5bc70a6..ef28328 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -2541,8 +2541,10 @@ static int monitor_fdset_dup_fd_find_remove(int 
> dup_fd, bool remove)
>  if (QLIST_EMPTY(&mon_fdset->dup_fds)) {
>  monitor_fdset_cleanup(mon_fdset);
>  }
> +return -1;
> +} else {
> +return mon_fdset->id;
>  }
> -return mon_fdset->id;
>  }
>  }
>  }
> @@ -2554,9 +2556,9 @@ int monitor_fdset_dup_fd_find(int dup_fd)
>  return monitor_fdset_dup_fd_find_remove(dup_fd, false);
>  }
>  
> -int monitor_fdset_dup_fd_remove(int dup_fd)
> +void monitor_fdset_dup_fd_remove(int dup_fd)
>  {
> -return monitor_fdset_dup_fd_find_remove(dup_fd, true);
> +monitor_fdset_dup_fd_find_remove(dup_fd, true);
>  }
>  
>  int monitor_handle_fd_param(Monitor *mon, const char *fdname)
> diff --git a/stubs/fdset-remove-fd.c b/stubs/fdset-remove-fd.c
> index b3886d9..7f6d61e 100644
> --- a/stubs/fdset-remove-fd.c
> +++ b/stubs/fdset-remove-fd.c
> @@ -1,7 +1,6 @@
>  #include "qemu-common.h"
>  #include "monitor/monitor.h"
>  
> -int monitor_fdset_dup_fd_remove(int dupfd)
> +void monitor_fdset_dup_fd_remove(int dupfd)
>  {
> -return -1;
>  }
> 

Re: [Qemu-devel] [PATCH v3 00/14] drop qapi nested structs

[Luiz Capitulino]

On Tue,  5 Aug 2014 19:14:19 -0600
Eric Blake  wrote:

> According to this email:
> https://lists.gnu.org/archive/html/qemu-devel/2014-05/msg00708.html
> we want to repurpose 'data': { 'name': {dict...} } in qapi files
> for future use of designating default values of optional parameters.
> But to do that, we must first nuke existing use of that syntax for
> declaring nested structs.  Enhancing the testsuite while at it
> never hurts.

I've skimmed over the series (as Markus is on top of it anyways) and it
looks great to me. Waiting for v4 :)

Re: [Qemu-devel] vexpress-a9: coreboot is unable to push any data on stack

[Piotr Król]

On Fri, Aug 15, 2014 at 05:10:04PM +0100, Peter Maydell wrote:
> For your stack issues, it looks like your code is trying to
> use the area which is the flash as the stack. Since flash
> isn't writeable, we ignore the writes and it's not very
> useful for stack. It looks like your code is assuming that
> the low memory is RAM, not flash -- so how does your
> code work on real hardware? Do you try to use the
> software controllable remapping to copy from the flash
> into RAM before using the stack, or something else?

Peter thanks for your reply. I'm not coreboot developer so cannot
advocate for their decision. Bootblock that I sent is from build
targeted on qemu and developer who initially wrote the code suggest
using it with '-kernel' parameter. I would like to fix this code
according to correct memory map.

Comment in hw/arm/vexpress.c say that, as you wrote below, Versatile
Express got two possible memory maps. Can you point me to exact
documentation that you use as reference for vexpress implementation ?

> 
> In terms of where we go from here, we have two
> choices:
>  (1) leave address 0 as RAM, not flash; this means
> legacy guest binaries that work only on QEMU and
> not on real hardware will still work, but the -bios
> option won't be of much use. (This is more or less
> reverting to the 2.0 situation.)
>  (2) bring it in to line with vexpress-a15 (which is
> effectively how 2.1 shipped), so 0 is always flash
> and never RAM. This is consistent but (as you've
> found) binaries assuming that 0 is a RAM alias
> will stop working.
> 

Assuming option (2) contain strategy for future releases it should be
priority. I will try to fix coreboot binary according to this advice.

One more time thanks for explanation.

Thanks,
Piotr Król

Re: [Qemu-devel] [PULL 00/24] Linux-user updates

[Peter Maydell]

On 15 August 2014 12:01,   wrote:
> A usual set of improvements and bugfixes. The binfmt flag addition is an
> ABI break so endusers need to update their binfmt registering scripts.

Ugh, really? I didn't realize that when I saw that patch
go past; I'd like to look at it in more detail before I
apply this, since "break everybody's working setup"
doesn't seem like a great idea...

thanks
-- PMM

Re: [Qemu-devel] [PULL 00/12] post-2.1 bugfixes

[Peter Maydell]

On 14 August 2014 17:08, Michael S. Tsirkin  wrote:
> The following changes since commit 2d591ce2aeebf9620ff527c7946844a3122afeec:
>
>   Merge remote-tracking branch 'remotes/mdroth/qga-pull-2014-08-08' into 
> staging (2014-08-08 14:16:05 +0100)
>
> are available in the git repository at:
>
>   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
>
> for you to fetch changes up to 260cb1c409f6b41fc284ece95462e458d7fda6c4:
>
>   pc: Get rid of pci-info leftovers (2014-08-14 13:22:25 +0200)
>
> 
> post-2.1 bugfixes
>
> A bunch of fixes that missed 2.1 by a small margin.
> If we do 2.1.1, some of these would be good candidates,
> added Cc qemu-stable as appropriate.
>
> Signed-off-by: Michael S. Tsirkin 

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PATCH for-2.2 v3 0/3] nbd: Adapt for dataplane

[Stefan Hajnoczi]

On Fri, Aug 15, 2014 at 03:31:48PM +0200, Max Reitz wrote:
> On 15.08.2014 15:08, Kevin Wolf wrote:
> >Am 20.06.2014 um 21:57 hat Max Reitz geschrieben:
> >>For the NBD server to work with dataplane, it needs to correctly access
> >>the exported BDS. It makes the most sense to run both in the same
> >>AioContext, therefore this series implements methods for tracking a
> >>BDS's AioContext and makes NBD make use of this for keeping the clients
> >>connected to that BDS in the same AioContext.
> >>
> >>This series breaks compilation of NBD on Windows, because
> >>aio_set_fd_handler() is not available there yet. It should therefore not
> >>be merged until that function is available (which will probably not
> >>happen before qemu 2.2).
> >>
> >>
> >>v3:
> >>  - Patch 1: Drop aio_notify(), because aio_set_fd_handler() will call it
> >>anyway [Stefan]
> >>
> >>v2:
> >>  - Patch 1: Drop NBDClient::restart_write; checking whether
> >>NBDClient::send_coroutine is not NULL suffices [Paolo]
> >Dropped from the block branch because it breaks the win32 build:
> >
> >nbd.c: In function 'nbd_set_handlers':
> >nbd.c:1323:9: warning: implicit declaration of function 'aio_set_fd_handler' 
> >[-Wimplicit-function-declaration]
> >  aio_set_fd_handler(client->exp->ctx, client->sock,
> >  ^
> >nbd.c:1323:9: warning: nested extern declaration of 'aio_set_fd_handler' 
> >[-Wnested-externs]
> >
> >And the resulting undefined references during linking.
> 
> As stated by the cover letter you're replying to. :-) Sorry I did not pay
> enough attention when you merged this series to your tree, I should have
> explicitly warned you.
> 
> Right, we need to implement aio_set_fd_handler() for the win32/win64 build
> before this series can really be merged. Paolo said this could be done until
> 2.2, so that's why this series is marked "for-2.2". I guess I'll have to
> keep a close look on qemu-devel whenever someone posts something resembling
> aio_set_fd_handler() for win32/win64.

Paolo's "[PATCH 00/25] AioContext & threadpool" is what we need.
Waiting for him to respin.

Stefan


pgpesz3fDyM0v.pgp
Description: PGP signature

[Qemu-devel] [PULL 54/55] image-fuzzer: Reduce number of generator functions in __init__

[Stefan Hajnoczi]

From: Maria Kustova 

Some issues can be found only when a fuzzed image has a partial structure,
e.g. has L1/L2 tables but no refcount ones. Generation of an entirely
defined image limits these cases. Now the Image constructor creates only
a header and a backing file name (if any), other image elements are generated
in the 'create_image' API.

Signed-off-by: Maria Kustova 
Signed-off-by: Stefan Hajnoczi 
---
 tests/image-fuzzer/qcow2/layout.py | 304 +
 1 file changed, 143 insertions(+), 161 deletions(-)

diff --git a/tests/image-fuzzer/qcow2/layout.py 
b/tests/image-fuzzer/qcow2/layout.py
index deed9ea..730c771 100644
--- a/tests/image-fuzzer/qcow2/layout.py
+++ b/tests/image-fuzzer/qcow2/layout.py
@@ -21,6 +21,7 @@ import struct
 import fuzz
 from math import ceil
 from os import urandom
+from itertools import chain
 
 MAX_IMAGE_SIZE = 10 * (1 << 20)
 # Standard sizes
@@ -36,7 +37,7 @@ class Field(object):
 of value necessary for its packing to binary form, an offset from
 the beginning of the image, a value and a name.
 
-The field can be iterated as a list [format, offset, value].
+The field can be iterated as a list [format, offset, value, name].
 """
 
 __slots__ = ('fmt', 'offset', 'value', 'name')
@@ -48,7 +49,7 @@ class Field(object):
 self.name = name
 
 def __iter__(self):
-return iter([self.fmt, self.offset, self.value])
+return iter([self.fmt, self.offset, self.value, self.name])
 
 def __repr__(self):
 return "Field(fmt='%s', offset=%d, value=%s, name=%s)" % \
@@ -59,15 +60,14 @@ class FieldsList(object):
 
 """List of fields.
 
-The class allows access to a field in the list by its name and joins
-several list in one via in-place addition.
+The class allows access to a field in the list by its name.
 """
 
 def __init__(self, meta_data=None):
 if meta_data is None:
 self.data = []
 else:
-self.data = [Field(f[0], f[1], f[2], f[3])
+self.data = [Field(*f)
  for f in meta_data]
 
 def __getitem__(self, name):
@@ -76,10 +76,6 @@ class FieldsList(object):
 def __iter__(self):
 return iter(self.data)
 
-def __iadd__(self, other):
-self.data += other.data
-return self
-
 def __len__(self):
 return len(self.data)
 
@@ -93,75 +89,31 @@ class Image(object):
 a file.
 """
 
-@staticmethod
-def _size_params():
-"""Generate a random image size aligned to a random correct
-cluster size.
-"""
-cluster_bits = random.randrange(9, 21)
-cluster_size = 1 << cluster_bits
-img_size = random.randrange(0, MAX_IMAGE_SIZE + 1, cluster_size)
-return (cluster_bits, img_size)
-
-@staticmethod
-def _get_available_clusters(used, number):
-"""Return a set of indices of not allocated clusters.
-
-'used' contains indices of currently allocated clusters.
-All clusters that cannot be allocated between 'used' clusters will have
-indices appended to the end of 'used'.
+def __init__(self, backing_file_name=None):
+"""Create a random valid qcow2 image with the correct header and stored
+backing file name.
 """
-append_id = max(used) + 1
-free = set(range(1, append_id)) - used
-if len(free) >= number:
-return set(random.sample(free, number))
-else:
-return free | set(range(append_id, append_id + number - len(free)))
-
-@staticmethod
-def _get_adjacent_clusters(used, size):
-"""Return an index of the first cluster in the sequence of free ones.
-
-'used' contains indices of currently allocated clusters. 'size' is the
-length of the sequence of free clusters.
-If the sequence of 'size' is not available between 'used' clusters, its
-first index will be append to the end of 'used'.
-"""
-def get_cluster_id(lst, length):
-"""Return the first index of the sequence of the specified length
-or None if the sequence cannot be inserted in the list.
-"""
-if len(lst) != 0:
-pairs = []
-pair = (lst[0], 1)
-for i in range(1, len(lst)):
-if lst[i] == lst[i-1] + 1:
-pair = (lst[i], pair[1] + 1)
-else:
-pairs.append(pair)
-pair = (lst[i], 1)
-pairs.append(pair)
-random.shuffle(pairs)
-for x, s in pairs:
-if s >= length:
-return x - length + 1
-return None
-
-append_id = max(used) + 1
-free = list(set(range(1, append_id)) - used)
-idx = get_cluster_id(free, size)
-if idx is None:
-return append_id
-

[Qemu-devel] [PULL 53/55] image-fuzzer: Add generators of L1/L2 tables

[Stefan Hajnoczi]

From: Maria Kustova 

Entries in L1/L2 entries are based on a portion of random guest clusters.
L2 entries contain offsets to host image clusters filled with random data.
Clusters for L1/L2 tables and guest data are selected randomly.

Signed-off-by: Maria Kustova 
Signed-off-by: Stefan Hajnoczi 
---
 tests/image-fuzzer/qcow2/layout.py | 255 +++--
 1 file changed, 190 insertions(+), 65 deletions(-)

diff --git a/tests/image-fuzzer/qcow2/layout.py 
b/tests/image-fuzzer/qcow2/layout.py
index 4c08202..deed9ea 100644
--- a/tests/image-fuzzer/qcow2/layout.py
+++ b/tests/image-fuzzer/qcow2/layout.py
@@ -19,6 +19,8 @@
 import random
 import struct
 import fuzz
+from math import ceil
+from os import urandom
 
 MAX_IMAGE_SIZE = 10 * (1 << 20)
 # Standard sizes
@@ -102,7 +104,66 @@ class Image(object):
 return (cluster_bits, img_size)
 
 @staticmethod
-def _header(cluster_bits, img_size, backing_file_name=None):
+def _get_available_clusters(used, number):
+"""Return a set of indices of not allocated clusters.
+
+'used' contains indices of currently allocated clusters.
+All clusters that cannot be allocated between 'used' clusters will have
+indices appended to the end of 'used'.
+"""
+append_id = max(used) + 1
+free = set(range(1, append_id)) - used
+if len(free) >= number:
+return set(random.sample(free, number))
+else:
+return free | set(range(append_id, append_id + number - len(free)))
+
+@staticmethod
+def _get_adjacent_clusters(used, size):
+"""Return an index of the first cluster in the sequence of free ones.
+
+'used' contains indices of currently allocated clusters. 'size' is the
+length of the sequence of free clusters.
+If the sequence of 'size' is not available between 'used' clusters, its
+first index will be append to the end of 'used'.
+"""
+def get_cluster_id(lst, length):
+"""Return the first index of the sequence of the specified length
+or None if the sequence cannot be inserted in the list.
+"""
+if len(lst) != 0:
+pairs = []
+pair = (lst[0], 1)
+for i in range(1, len(lst)):
+if lst[i] == lst[i-1] + 1:
+pair = (lst[i], pair[1] + 1)
+else:
+pairs.append(pair)
+pair = (lst[i], 1)
+pairs.append(pair)
+random.shuffle(pairs)
+for x, s in pairs:
+if s >= length:
+return x - length + 1
+return None
+
+append_id = max(used) + 1
+free = list(set(range(1, append_id)) - used)
+idx = get_cluster_id(free, size)
+if idx is None:
+return append_id
+else:
+return idx
+
+@staticmethod
+def _alloc_data(img_size, cluster_size):
+"""Return a set of random indices of clusters allocated for guest data.
+"""
+num_of_cls = img_size/cluster_size
+return set(random.sample(range(1, num_of_cls + 1),
+ random.randint(0, num_of_cls)))
+
+def create_header(self, cluster_bits, backing_file_name=None):
 """Generate a random valid header."""
 meta_header = [
 ['>4s', 0, "QFI\xfb", 'magic'],
@@ -110,7 +171,7 @@ class Image(object):
 ['>Q', 8, 0, 'backing_file_offset'],
 ['>I', 16, 0, 'backing_file_size'],
 ['>I', 20, cluster_bits, 'cluster_bits'],
-['>Q', 24, img_size, 'size'],
+['>Q', 24, self.image_size, 'size'],
 ['>I', 32, 0, 'crypt_method'],
 ['>I', 36, 0, 'l1_size'],
 ['>Q', 40, 0, 'l1_table_offset'],
@@ -126,63 +187,59 @@ class Image(object):
 ['>I', 96, 4, 'refcount_order'],
 ['>I', 100, 0, 'header_length']
 ]
-v_header = FieldsList(meta_header)
+self.header = FieldsList(meta_header)
 
-if v_header['version'][0].value == 2:
-v_header['header_length'][0].value = 72
+if self.header['version'][0].value == 2:
+self.header['header_length'][0].value = 72
 else:
-v_header['incompatible_features'][0].value = random.getrandbits(2)
-v_header['compatible_features'][0].value = random.getrandbits(1)
-v_header['header_length'][0].value = 104
-
-max_header_len = struct.calcsize(v_header['header_length'][0].fmt) + \
- v_header['header_length'][0].offset
+self.header['incompatible_features'][0].value = \
+random.getrandbits(2)
+self.header['compatible_features'][0].value = random.getrandbits(1)
+self.header['header_l

[Qemu-devel] [PULL 52/55] image-fuzzer: Add fuzzing functions for L1/L2 table entries

[Stefan Hajnoczi]

From: Maria Kustova 

Signed-off-by: Maria Kustova 
Signed-off-by: Stefan Hajnoczi 
---
 tests/image-fuzzer/qcow2/fuzz.py | 28 
 1 file changed, 28 insertions(+)

diff --git a/tests/image-fuzzer/qcow2/fuzz.py b/tests/image-fuzzer/qcow2/fuzz.py
index a53c84f..57527f9 100644
--- a/tests/image-fuzzer/qcow2/fuzz.py
+++ b/tests/image-fuzzer/qcow2/fuzz.py
@@ -325,3 +325,31 @@ def feature_name(current):
 truncate_string(STRING_V, 46)  # Fuzz padding (field length = 46)
 ]
 return selector(current, constraints, string_validator)
+
+
+def l1_entry(current):
+"""Fuzz an entry of the L1 table."""
+constraints = UINT64_V
+# Reserved bits are ignored
+# Added a possibility when only flags are fuzzed
+offset = 0x7fff & random.choice([selector(current,
+  constraints),
+ current])
+is_cow = random.randint(0, 1)
+return offset + (is_cow << UINT64_M)
+
+
+def l2_entry(current):
+"""Fuzz an entry of an L2 table."""
+constraints = UINT64_V
+# Reserved bits are ignored
+# Add a possibility when only flags are fuzzed
+offset = 0x3ffe & random.choice([selector(current,
+  constraints),
+ current])
+is_compressed = random.randint(0, 1)
+is_cow = random.randint(0, 1)
+is_zero = random.randint(0, 1)
+value = offset + (is_cow << UINT64_M) + \
+(is_compressed << UINT64_M - 1) + is_zero
+return value
-- 
1.9.3

[Qemu-devel] [PULL 51/55] docs: Expand the list of supported image elements with L1/L2 tables

[Stefan Hajnoczi]

From: Maria Kustova 

Signed-off-by: Maria Kustova 
Signed-off-by: Stefan Hajnoczi 
---
 docs/image-fuzzer.txt | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/docs/image-fuzzer.txt b/docs/image-fuzzer.txt
index e73b182..0d0005d 100644
--- a/docs/image-fuzzer.txt
+++ b/docs/image-fuzzer.txt
@@ -125,8 +125,7 @@ If a fuzzer configuration is specified, then it has the 
next interpretation:
 will be always fuzzed for every test. This case is useful for regression
 testing.
 
-For now only header fields and header extensions are generated.
-
+For now only header fields, header extensions and L1/L2 tables are generated.
 
 Module interfaces
 -
-- 
1.9.3

[Qemu-devel] [PULL 55/55] qcow2: fix new_blocks double-free in alloc_refcount_block()

[Stefan Hajnoczi]

Commit de82815db1c89da058b7fb941dab137d6d9ab738 ("qcow2: Handle failure
for potentially large allocations") introduced a double-free of
new_blocks in the alloc_refcount_block() error path.

The qemu-iotests qcow2 026 test case was failing because qemu-io
segfaulted.

Make sure new_blocks is NULL after we free it the first time.

Reviewed-by: Kevin Wolf 
Signed-off-by: Stefan Hajnoczi 
---
 block/qcow2-refcount.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index d60e2fe..3b77470 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -381,6 +381,7 @@ static int alloc_refcount_block(BlockDriverState *bs,
 ret = bdrv_pwrite_sync(bs->file, meta_offset, new_blocks,
 blocks_clusters * s->cluster_size);
 g_free(new_blocks);
+new_blocks = NULL;
 if (ret < 0) {
 goto fail_table;
 }
-- 
1.9.3

[Qemu-devel] [PULL 48/55] image-fuzzer: Fuzzing functions for qcow2 images

[Stefan Hajnoczi]

From: Maria Kustova 

The fuzz submodule of the qcow2 image generator contains fuzzing functions for
image fields.
Each fuzzing function contains a list of constraints and a call of a helper
function that randomly selects a fuzzed value satisfied to one of constraints.
For now constraints include only known as invalid or potentially dangerous
values. But after investigation of code coverage by fuzz tests they will be
expanded by heuristic values based on inner checks and flows of a program
under test.

Now fuzzing of a header, header extensions and a backing file name is
supported.

Reviewed-by: Stefan Hajnoczi 
Signed-off-by: Maria Kustova 
Signed-off-by: Stefan Hajnoczi 
---
 tests/image-fuzzer/qcow2/fuzz.py | 327 +++
 1 file changed, 327 insertions(+)
 create mode 100644 tests/image-fuzzer/qcow2/fuzz.py

diff --git a/tests/image-fuzzer/qcow2/fuzz.py b/tests/image-fuzzer/qcow2/fuzz.py
new file mode 100644
index 000..a53c84f
--- /dev/null
+++ b/tests/image-fuzzer/qcow2/fuzz.py
@@ -0,0 +1,327 @@
+# Fuzzing functions for qcow2 fields
+#
+# Copyright (C) 2014 Maria Kustova 
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+#
+
+import random
+
+
+UINT8 = 0xff
+UINT32 = 0x
+UINT64 = 0x
+# Most significant bit orders
+UINT32_M = 31
+UINT64_M = 63
+# Fuzz vectors
+UINT8_V = [0, 0x10, UINT8/4, UINT8/2 - 1, UINT8/2, UINT8/2 + 1, UINT8 - 1,
+   UINT8]
+UINT32_V = [0, 0x100, 0x1000, 0x1, 0x10, UINT32/4, UINT32/2 - 1,
+UINT32/2, UINT32/2 + 1, UINT32 - 1, UINT32]
+UINT64_V = UINT32_V + [0x100, 0x1000, 0x1, UINT64/4,
+   UINT64/2 - 1, UINT64/2, UINT64/2 + 1, UINT64 - 1,
+   UINT64]
+STRING_V = ['%s%p%x%d', '.1024d', '%.2049d', '%p%p%p%p', '%x%x%x%x',
+'%d%d%d%d', '%s%s%s%s', '%999s', '%08x', '%%20d', '%%20n',
+'%%20x', '%%20s', '%s%s%s%s%s%s%s%s%s%s', '%p%p%p%p%p%p%p%p%p%p',
+'%#0123456x%08x%x%s%p%d%n%o%u%c%h%l%q%j%z%Z%t%i%e%g%f%a%C%S%08x%%',
+'%s x 129', '%x x 257']
+
+
+def random_from_intervals(intervals):
+"""Select a random integer number from the list of specified intervals.
+
+Each interval is a tuple of lower and upper limits of the interval. The
+limits are included. Intervals in a list should not overlap.
+"""
+total = reduce(lambda x, y: x + y[1] - y[0] + 1, intervals, 0)
+r = random.randint(0, total - 1) + intervals[0][0]
+for x in zip(intervals, intervals[1:]):
+r = r + (r > x[0][1]) * (x[1][0] - x[0][1] - 1)
+return r
+
+
+def random_bits(bit_ranges):
+"""Generate random binary mask with ones in the specified bit ranges.
+
+Each bit_ranges is a list of tuples of lower and upper limits of bit
+positions will be fuzzed. The limits are included. Random amount of bits
+in range limits will be set to ones. The mask is returned in decimal
+integer format.
+"""
+bit_numbers = []
+# Select random amount of random positions in bit_ranges
+for rng in bit_ranges:
+bit_numbers += random.sample(range(rng[0], rng[1] + 1),
+ random.randint(0, rng[1] - rng[0] + 1))
+val = 0
+# Set bits on selected positions to ones
+for bit in bit_numbers:
+val |= 1 << bit
+return val
+
+
+def truncate_string(strings, length):
+"""Return strings truncated to specified length."""
+if type(strings) == list:
+return [s[:length] for s in strings]
+else:
+return strings[:length]
+
+
+def validator(current, pick, choices):
+"""Return a value not equal to the current selected by the pick
+function from choices.
+"""
+while True:
+val = pick(choices)
+if not val == current:
+return val
+
+
+def int_validator(current, intervals):
+"""Return a random value from intervals not equal to the current.
+
+This function is useful for selection from valid values except current one.
+"""
+return validator(current, random_from_intervals, intervals)
+
+
+def bit_validator(current, bit_ranges):
+"""Return a random bit mask not equal to the current.
+
+This function is useful for selection from valid values except current one.
+"""
+return validator(current, random_bits, bit_ranges)
+
+
+def string_validator(current, strings):

[Qemu-devel] [PULL 49/55] image-fuzzer: Generator of fuzzed qcow2 images

[Stefan Hajnoczi]

From: Maria Kustova 

The layout submodule of the qcow2 package creates a random valid image,
randomly selects some amount of its fields, fuzzes them and write the fuzzed
image to the file. Fuzzing process can be controlled by an external
configuration.

Reviewed-by: Stefan Hajnoczi 
Signed-off-by: Maria Kustova 
Signed-off-by: Stefan Hajnoczi 
---
 tests/image-fuzzer/qcow2/layout.py | 369 +
 1 file changed, 369 insertions(+)
 create mode 100644 tests/image-fuzzer/qcow2/layout.py

diff --git a/tests/image-fuzzer/qcow2/layout.py 
b/tests/image-fuzzer/qcow2/layout.py
new file mode 100644
index 000..4c08202
--- /dev/null
+++ b/tests/image-fuzzer/qcow2/layout.py
@@ -0,0 +1,369 @@
+# Generator of fuzzed qcow2 images
+#
+# Copyright (C) 2014 Maria Kustova 
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+#
+
+import random
+import struct
+import fuzz
+
+MAX_IMAGE_SIZE = 10 * (1 << 20)
+# Standard sizes
+UINT32_S = 4
+UINT64_S = 8
+
+
+class Field(object):
+
+"""Atomic image element (field).
+
+The class represents an image field as quadruple of a data format
+of value necessary for its packing to binary form, an offset from
+the beginning of the image, a value and a name.
+
+The field can be iterated as a list [format, offset, value].
+"""
+
+__slots__ = ('fmt', 'offset', 'value', 'name')
+
+def __init__(self, fmt, offset, val, name):
+self.fmt = fmt
+self.offset = offset
+self.value = val
+self.name = name
+
+def __iter__(self):
+return iter([self.fmt, self.offset, self.value])
+
+def __repr__(self):
+return "Field(fmt='%s', offset=%d, value=%s, name=%s)" % \
+(self.fmt, self.offset, str(self.value), self.name)
+
+
+class FieldsList(object):
+
+"""List of fields.
+
+The class allows access to a field in the list by its name and joins
+several list in one via in-place addition.
+"""
+
+def __init__(self, meta_data=None):
+if meta_data is None:
+self.data = []
+else:
+self.data = [Field(f[0], f[1], f[2], f[3])
+ for f in meta_data]
+
+def __getitem__(self, name):
+return [x for x in self.data if x.name == name]
+
+def __iter__(self):
+return iter(self.data)
+
+def __iadd__(self, other):
+self.data += other.data
+return self
+
+def __len__(self):
+return len(self.data)
+
+
+class Image(object):
+
+""" Qcow2 image object.
+
+This class allows to create qcow2 images with random valid structures and
+values, fuzz them via external qcow2.fuzz module and write the result to
+a file.
+"""
+
+@staticmethod
+def _size_params():
+"""Generate a random image size aligned to a random correct
+cluster size.
+"""
+cluster_bits = random.randrange(9, 21)
+cluster_size = 1 << cluster_bits
+img_size = random.randrange(0, MAX_IMAGE_SIZE + 1, cluster_size)
+return (cluster_bits, img_size)
+
+@staticmethod
+def _header(cluster_bits, img_size, backing_file_name=None):
+"""Generate a random valid header."""
+meta_header = [
+['>4s', 0, "QFI\xfb", 'magic'],
+['>I', 4, random.randint(2, 3), 'version'],
+['>Q', 8, 0, 'backing_file_offset'],
+['>I', 16, 0, 'backing_file_size'],
+['>I', 20, cluster_bits, 'cluster_bits'],
+['>Q', 24, img_size, 'size'],
+['>I', 32, 0, 'crypt_method'],
+['>I', 36, 0, 'l1_size'],
+['>Q', 40, 0, 'l1_table_offset'],
+['>Q', 48, 0, 'refcount_table_offset'],
+['>I', 56, 0, 'refcount_table_clusters'],
+['>I', 60, 0, 'nb_snapshots'],
+['>Q', 64, 0, 'snapshots_offset'],
+['>Q', 72, 0, 'incompatible_features'],
+['>Q', 80, 0, 'compatible_features'],
+['>Q', 88, 0, 'autoclear_features'],
+# Only refcount_order = 4 is supported by current (07.2014)
+# implementation of QEMU
+['>I', 96, 4, 'refcount_order'],
+['>I', 100, 0, 'header_length']
+]
+v_header = FieldsList(meta_header)
+
+if v_header['version'][0].value == 2:
+v_header['header_length'][0].value = 72
+else:
+   

[Qemu-devel] [PULL 45/55] ide: only constrain read/write requests to drive size, not other types

[Stefan Hajnoczi]

From: Michael Tokarev 

Commit 58ac321135a introduced a check to ide dma processing which
constrains all requests to drive size.  However, apparently, some
valid requests (like TRIM) does not fit in this constraint, and
fails in 2.1.  So check the range only for reads and writes.

Cc: qemu-sta...@nongnu.org
Signed-off-by: Michael Tokarev 
Signed-off-by: Markus Armbruster 
Signed-off-by: Stefan Hajnoczi 
---
 hw/ide/core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 82dd4af..b48127f 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -692,7 +692,8 @@ void ide_dma_cb(void *opaque, int ret)
sector_num, n, s->dma_cmd);
 #endif
 
-if (!ide_sect_range_ok(s, sector_num, n)) {
+if ((s->dma_cmd == IDE_DMA_READ || s->dma_cmd == IDE_DMA_WRITE) &&
+!ide_sect_range_ok(s, sector_num, n)) {
 dma_buf_commit(s);
 ide_dma_error(s);
 return;
-- 
1.9.3

[Qemu-devel] [PULL 50/55] image-fuzzer: Public API for image-fuzzer/runner/runner.py

[Stefan Hajnoczi]

From: Maria Kustova 

__init__.py provides the public API required by the test runner

Reviewed-by: Stefan Hajnoczi 
Signed-off-by: Maria Kustova 
Signed-off-by: Stefan Hajnoczi 
---
 tests/image-fuzzer/qcow2/__init__.py | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 tests/image-fuzzer/qcow2/__init__.py

diff --git a/tests/image-fuzzer/qcow2/__init__.py 
b/tests/image-fuzzer/qcow2/__init__.py
new file mode 100644
index 000..e2ebe19
--- /dev/null
+++ b/tests/image-fuzzer/qcow2/__init__.py
@@ -0,0 +1 @@
+from layout import create_image
-- 
1.9.3

[Qemu-devel] [PULL 47/55] image-fuzzer: Tool for fuzz tests execution

[Stefan Hajnoczi]

From: Maria Kustova 

The purpose of the test runner is to prepare the test environment (e.g. create
a work directory, a test image, etc), execute a program under test with
parameters, indicate a test failure if the program was killed during the test
execution and collect core dumps, logs and other test artifacts.

The test runner doesn't depend on an image format, so it can be used with any
external image generator.

[Fixed path to qcow2 format module "qcow2" instead of "../qcow2" since
runner.py is no longer in a sub-directory.
--Stefan]

Reviewed-by: Stefan Hajnoczi 
Signed-off-by: Maria Kustova 
Signed-off-by: Stefan Hajnoczi 
---
 tests/image-fuzzer/runner.py | 405 +++
 1 file changed, 405 insertions(+)
 create mode 100755 tests/image-fuzzer/runner.py

diff --git a/tests/image-fuzzer/runner.py b/tests/image-fuzzer/runner.py
new file mode 100755
index 000..58079d3
--- /dev/null
+++ b/tests/image-fuzzer/runner.py
@@ -0,0 +1,405 @@
+#!/usr/bin/env python
+
+# Tool for running fuzz tests
+#
+# Copyright (C) 2014 Maria Kustova 
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+#
+
+import sys
+import os
+import signal
+import subprocess
+import random
+import shutil
+from itertools import count
+import getopt
+import StringIO
+import resource
+
+try:
+import json
+except ImportError:
+try:
+import simplejson as json
+except ImportError:
+print >>sys.stderr, \
+"Warning: Module for JSON processing is not found.\n" \
+"'--config' and '--command' options are not supported."
+
+# Backing file sizes in MB
+MAX_BACKING_FILE_SIZE = 10
+MIN_BACKING_FILE_SIZE = 1
+
+
+def multilog(msg, *output):
+""" Write an object to all of specified file descriptors."""
+for fd in output:
+fd.write(msg)
+fd.flush()
+
+
+def str_signal(sig):
+""" Convert a numeric value of a system signal to the string one
+defined by the current operational system.
+"""
+for k, v in signal.__dict__.items():
+if v == sig:
+return k
+
+
+def run_app(fd, q_args):
+"""Start an application with specified arguments and return its exit code
+or kill signal depending on the result of execution.
+"""
+devnull = open('/dev/null', 'r+')
+process = subprocess.Popen(q_args, stdin=devnull,
+   stdout=subprocess.PIPE,
+   stderr=subprocess.PIPE)
+out, err = process.communicate()
+fd.write(out)
+fd.write(err)
+return process.returncode
+
+
+class TestException(Exception):
+"""Exception for errors risen by TestEnv objects."""
+pass
+
+
+class TestEnv(object):
+
+"""Test object.
+
+The class sets up test environment, generates backing and test images
+and executes application under tests with specified arguments and a test
+image provided.
+
+All logs are collected.
+
+The summary log will contain short descriptions and statuses of tests in
+a run.
+
+The test log will include application (e.g. 'qemu-img') logs besides info
+sent to the summary log.
+"""
+
+def __init__(self, test_id, seed, work_dir, run_log,
+ cleanup=True, log_all=False):
+"""Set test environment in a specified work directory.
+
+Path to qemu-img and qemu-io will be retrieved from 'QEMU_IMG' and
+'QEMU_IO' environment variables.
+"""
+if seed is not None:
+self.seed = seed
+else:
+self.seed = str(random.randint(0, sys.maxint))
+random.seed(self.seed)
+
+self.init_path = os.getcwd()
+self.work_dir = work_dir
+self.current_dir = os.path.join(work_dir, 'test-' + test_id)
+self.qemu_img = os.environ.get('QEMU_IMG', 'qemu-img')\
+  .strip().split(' ')
+self.qemu_io = os.environ.get('QEMU_IO', 'qemu-io').strip().split(' ')
+self.commands = [['qemu-img', 'check', '-f', 'qcow2', '$test_img'],
+ ['qemu-img', 'info', '-f', 'qcow2', '$test_img'],
+ ['qemu-io', '$test_img', '-c', 'read $off $len'],
+ ['qemu-io', '$test_img', '-c', 'write $off $len'],
+ ['qemu-io', '$test_img', '-c',
+  'aio_read $off $len'],
+

[Qemu-devel] [PULL 42/55] libqos: Correct mask to align size to PAGE_SIZE in malloc-pc

[Stefan Hajnoczi]

From: Marc Marí 

Reviewed-by: John Snow 
Reviewed-by: Stefan Hajnoczi 
Signed-off-by: Paolo Bonzini 
Signed-off-by: Marc Marí 
Signed-off-by: Stefan Hajnoczi 
---
 tests/libqos/malloc-pc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/libqos/malloc-pc.c b/tests/libqos/malloc-pc.c
index 63af60a..be1d97f 100644
--- a/tests/libqos/malloc-pc.c
+++ b/tests/libqos/malloc-pc.c
@@ -36,7 +36,7 @@ static uint64_t pc_alloc(QGuestAllocator *allocator, size_t 
size)
 
 
 size += (PAGE_SIZE - 1);
-size &= PAGE_SIZE;
+size &= -PAGE_SIZE;
 
 g_assert_cmpint((s->start + size), <=, s->end);
 
-- 
1.9.3

[Qemu-devel] [PULL 39/55] qemu-options: add missing -drive discard option to cmdline help

[Stefan Hajnoczi]

From: Peter Lieven 

Signed-off-by: Peter Lieven 
Reviewed-by: Eric Blake 
Signed-off-by: Stefan Hajnoczi 
---
 qemu-options.hx | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 96516c1..44e3be3 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -427,7 +427,7 @@ DEF("drive", HAS_ARG, QEMU_OPTION_drive,
 "   [,serial=s][,addr=A][,rerror=ignore|stop|report]\n"
 "   
[,werror=ignore|stop|report|enospc][,id=name][,aio=threads|native]\n"
 "   [,readonly=on|off][,copy-on-read=on|off]\n"
-"   [,detect-zeroes=on|off|unmap]\n"
+"   [,discard=ignore|unmap][,detect-zeroes=on|off|unmap]\n"
 "   [[,bps=b]|[[,bps_rd=r][,bps_wr=w]]]\n"
 "   [[,iops=i]|[[,iops_rd=r][,iops_wr=w]]]\n"
 "   [[,bps_max=bm]|[[,bps_rd_max=rm][,bps_wr_max=wm]]]\n"
-- 
1.9.3

[Qemu-devel] [PULL 46/55] docs: Specification for the image fuzzer

[Stefan Hajnoczi]

From: Maria Kustova 

'Overall fuzzer requirements' chapter contains the current product vision and
features done and to be done. This chapter is still in progress.

Signed-off-by: Maria Kustova 
Signed-off-by: Stefan Hajnoczi 
---
 docs/image-fuzzer.txt | 239 ++
 1 file changed, 239 insertions(+)
 create mode 100644 docs/image-fuzzer.txt

diff --git a/docs/image-fuzzer.txt b/docs/image-fuzzer.txt
new file mode 100644
index 000..e73b182
--- /dev/null
+++ b/docs/image-fuzzer.txt
@@ -0,0 +1,239 @@
+# Specification for the fuzz testing tool
+#
+# Copyright (C) 2014 Maria Kustova 
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+
+
+Image fuzzer
+
+
+Description
+---
+
+The goal of the image fuzzer is to catch crashes of qemu-io/qemu-img
+by providing to them randomly corrupted images.
+Test images are generated from scratch and have valid inner structure with some
+elements, e.g. L1/L2 tables, having random invalid values.
+
+
+Test runner
+---
+
+The test runner generates test images, executes tests utilizing generated
+images, indicates their results and collects all test related artifacts (logs,
+core dumps, test images, backing files).
+The test means execution of all available commands under test with the same
+generated test image.
+By default, the test runner generates new tests and executes them until
+keyboard interruption. But if a test seed is specified via the '--seed' runner
+parameter, then only one test with this seed will be executed, after its finish
+the runner will exit.
+
+The runner uses an external image fuzzer to generate test images. An image
+generator should be specified as a mandatory parameter of the test runner.
+Details about interactions between the runner and fuzzers see "Module
+interfaces".
+
+The runner activates generation of core dumps during test executions, but it
+assumes that core dumps will be generated in the current working directory.
+For comprehensive test results, please, set up your test environment
+properly.
+
+Paths to binaries under test (SUTs) qemu-img and qemu-io are retrieved from
+environment variables. If the environment check fails the runner will
+use SUTs installed in system paths.
+qemu-img is required for creation of backing files, so it's mandatory to set
+the related environment variable if it's not installed in the system path.
+For details about environment variables see qemu-iotests/check.
+
+The runner accepts a JSON array of fields expected to be fuzzed via the
+'--config' argument, e.g.
+
+   '[["feature_name_table"], ["header", "l1_table_offset"]]'
+
+Each sublist can have one or two strings defining image structure elements.
+In the latter case a parent element should be placed on the first position,
+and a field name on the second one.
+
+The runner accepts a list of commands under test as a JSON array via
+the '--command' argument. Each command is a list containing a SUT and all its
+arguments, e.g.
+
+   runner.py -c '[["qemu-io", "$test_img", "-c", "write $off $len"]]'
+ /tmp/test ../qcow2
+
+For variable arguments next aliases can be used:
+- $test_img for a fuzzed img
+- $off for an offset in the fuzzed image
+- $len for a data size
+
+Values for last two aliases will be generated based on a size of a virtual
+disk of the generated image.
+In case when no commands are specified the runner will execute commands from
+the default list:
+- qemu-img check
+- qemu-img info
+- qemu-img convert
+- qemu-io -c read
+- qemu-io -c write
+- qemu-io -c aio_read
+- qemu-io -c aio_write
+- qemu-io -c flush
+- qemu-io -c discard
+- qemu-io -c truncate
+
+
+Qcow2 image generator
+-
+
+The 'qcow2' generator is a Python package providing 'create_image' method as
+a single public API. See details in 'Test runner/image fuzzer' chapter of
+'Module interfaces'.
+
+Qcow2 contains two submodules: fuzz.py and layout.py.
+
+'fuzz.py' contains all fuzzing functions, one per image field. It's assumed
+that after code analysis every field will have own constraints for its value.
+For now only universal potentially dangerous values are used, e.g. type limits
+for integers or unsafe symbols as '%s' for strings. For bitmasks random amount
+of bits are set to ones. All fuzzed values are checked on non-equality to th

[Qemu-devel] [PULL 44/55] virtio-blk: Correct bug in support for flexible descriptor layout

[Stefan Hajnoczi]

From: Marc Marí 

Without this correction, only a three descriptor layout is accepted, and
requests with just two descriptors are not completed and no error message is
displayed.

Signed-off-by: Stefan Hajnoczi 
Signed-off-by: Marc Marí 
Signed-off-by: Stefan Hajnoczi 
---
 hw/block/virtio-blk.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index c241c50..302c39e 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -404,19 +404,19 @@ void virtio_blk_handle_request(VirtIOBlockReq *req, 
MultiReqBuffer *mrb)
  * NB: per existing s/n string convention the string is
  * terminated by '\0' only when shorter than buffer.
  */
-strncpy(req->elem.in_sg[0].iov_base,
-s->blk.serial ? s->blk.serial : "",
-MIN(req->elem.in_sg[0].iov_len, VIRTIO_BLK_ID_BYTES));
+const char *serial = s->blk.serial ? s->blk.serial : "";
+size_t size = MIN(strlen(serial) + 1,
+  MIN(iov_size(in_iov, in_num),
+  VIRTIO_BLK_ID_BYTES));
+iov_from_buf(in_iov, in_num, 0, serial, size);
 virtio_blk_req_complete(req, VIRTIO_BLK_S_OK);
 virtio_blk_free_request(req);
 } else if (type & VIRTIO_BLK_T_OUT) {
-qemu_iovec_init_external(&req->qiov, &req->elem.out_sg[1],
- req->elem.out_num - 1);
+qemu_iovec_init_external(&req->qiov, iov, out_num);
 virtio_blk_handle_write(req, mrb);
 } else if (type == VIRTIO_BLK_T_IN || type == VIRTIO_BLK_T_BARRIER) {
 /* VIRTIO_BLK_T_IN is 0, so we can't just & it. */
-qemu_iovec_init_external(&req->qiov, &req->elem.in_sg[0],
- req->elem.in_num - 1);
+qemu_iovec_init_external(&req->qiov, in_iov, in_num);
 virtio_blk_handle_read(req);
 } else {
 virtio_blk_req_complete(req, VIRTIO_BLK_S_UNSUPP);
-- 
1.9.3

[Qemu-devel] [PULL 34/55] dataplane: stop trying on notifier error

[Stefan Hajnoczi]

From: Cornelia Huck 

If we fail to set up guest or host notifiers, there's no use trying again
every time the guest kicks, so disable dataplane in that case.

Acked-by: Christian Borntraeger 
Signed-off-by: Cornelia Huck 
Signed-off-by: Stefan Hajnoczi 
---
 hw/block/dataplane/virtio-blk.c | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index 94e1a29..24a6b71 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -28,6 +28,7 @@ struct VirtIOBlockDataPlane {
 bool started;
 bool starting;
 bool stopping;
+bool disabled;
 
 VirtIOBlkConf *blk;
 
@@ -220,7 +221,7 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
 VirtQueue *vq;
 int r;
 
-if (s->started) {
+if (s->started || s->disabled) {
 return;
 }
 
@@ -274,6 +275,7 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
 k->set_guest_notifiers(qbus->parent, 1, false);
   fail_guest_notifiers:
 vring_teardown(&s->vring, s->vdev, 0);
+s->disabled = true;
   fail_vring:
 s->starting = false;
 }
@@ -284,6 +286,13 @@ void virtio_blk_data_plane_stop(VirtIOBlockDataPlane *s)
 BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(s->vdev)));
 VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
 VirtIOBlock *vblk = VIRTIO_BLK(s->vdev);
+
+
+/* Better luck next time. */
+if (s->disabled) {
+s->disabled = false;
+return;
+}
 if (!s->started || s->stopping) {
 return;
 }
-- 
1.9.3

[Qemu-devel] [PULL 30/55] qemu-char: using qemu_set_nonblock() instead of fcntl(O_NONBLOCK)

[Stefan Hajnoczi]

From: Gonglei 

Technically, fcntl(soc, F_SETFL, O_NONBLOCK)
is incorrect since it clobbers all other file flags.
We can use F_GETFL to get the current flags, set or
clear the O_NONBLOCK flag, then use F_SETFL to set the flags.

Using the qemu_set_nonblock() wrapper.

Signed-off-by: Wangxin 
Signed-off-by: Gonglei 
Reviewed-by: Eric Blake 
Signed-off-by: Stefan Hajnoczi 
---
 qemu-char.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index 6964a2d..b1e6a0a 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -975,7 +975,7 @@ static CharDriverState *qemu_chr_open_fd(int fd_in, int 
fd_out)
 s = g_malloc0(sizeof(FDCharDriver));
 s->fd_in = io_channel_from_fd(fd_in);
 s->fd_out = io_channel_from_fd(fd_out);
-fcntl(fd_out, F_SETFL, O_NONBLOCK);
+qemu_set_nonblock(fd_out);
 s->chr = chr;
 chr->opaque = s;
 chr->chr_add_watch = fd_chr_add_watch;
@@ -1062,7 +1062,7 @@ static CharDriverState *qemu_chr_open_stdio(ChardevStdio 
*opts)
 }
 old_fd0_flags = fcntl(0, F_GETFL);
 tcgetattr (0, &oldtty);
-fcntl(0, F_SETFL, O_NONBLOCK);
+qemu_set_nonblock(0);
 atexit(term_exit);
 
 chr = qemu_chr_open_fd(0, 1);
-- 
1.9.3

[Qemu-devel] [PULL 38/55] parallels: 2TB+ parallels images support

[Stefan Hajnoczi]

From: "Denis V. Lunev" 

Parallels has released in the recent updates of Parallels Server 5/6
new addition to his image format. Images with signature WithouFreSpacExt
have offsets in the catalog coded not as offsets in sectors (multiple
of 512 bytes) but offsets coded in blocks (i.e. header->tracks * 512)

In this case all 64 bits of header->nb_sectors are used for image size.

This patch implements support of this for qemu-img and also adds specific
check for an incorrect image. Images with block size greater than
INT_MAX/513 are not supported. The biggest available Parallels image
cluster size in the field is 1 Mb. Thus this limit will not hurt
anyone.

Signed-off-by: Denis V. Lunev 
CC: Jeff Cody 
CC: Kevin Wolf 
CC: Stefan Hajnoczi 
Reviewed-by: Jeff Cody 
Signed-off-by: Stefan Hajnoczi 
---
 block/parallels.c | 25 -
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/block/parallels.c b/block/parallels.c
index f9e18b4..1774ab8 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -30,6 +30,7 @@
 /**/
 
 #define HEADER_MAGIC "WithoutFreeSpace"
+#define HEADER_MAGIC2 "WithouFreSpacExt"
 #define HEADER_VERSION 2
 #define HEADER_SIZE 64
 
@@ -54,6 +55,8 @@ typedef struct BDRVParallelsState {
 unsigned int catalog_size;
 
 unsigned int tracks;
+
+unsigned int off_multiplier;
 } BDRVParallelsState;
 
 static int parallels_probe(const uint8_t *buf, int buf_size, const char 
*filename)
@@ -63,7 +66,8 @@ static int parallels_probe(const uint8_t *buf, int buf_size, 
const char *filenam
 if (buf_size < HEADER_SIZE)
 return 0;
 
-if (!memcmp(ph->magic, HEADER_MAGIC, 16) &&
+if ((!memcmp(ph->magic, HEADER_MAGIC, 16) ||
+!memcmp(ph->magic, HEADER_MAGIC2, 16)) &&
 (le32_to_cpu(ph->version) == HEADER_VERSION))
 return 100;
 
@@ -85,21 +89,31 @@ static int parallels_open(BlockDriverState *bs, QDict 
*options, int flags,
 goto fail;
 }
 
+bs->total_sectors = le64_to_cpu(ph.nb_sectors);
+
 if (le32_to_cpu(ph.version) != HEADER_VERSION) {
 goto fail_format;
 }
-if (memcmp(ph.magic, HEADER_MAGIC, 16)) {
+if (!memcmp(ph.magic, HEADER_MAGIC, 16)) {
+s->off_multiplier = 1;
+bs->total_sectors = 0x & bs->total_sectors;
+} else if (!memcmp(ph.magic, HEADER_MAGIC2, 16)) {
+s->off_multiplier = le32_to_cpu(ph.tracks);
+} else {
 goto fail_format;
 }
 
-bs->total_sectors = 0x & le64_to_cpu(ph.nb_sectors);
-
 s->tracks = le32_to_cpu(ph.tracks);
 if (s->tracks == 0) {
 error_setg(errp, "Invalid image: Zero sectors per track");
 ret = -EINVAL;
 goto fail;
 }
+if (s->tracks > INT32_MAX/513) {
+error_setg(errp, "Invalid image: Too big cluster");
+ret = -EFBIG;
+goto fail;
+}
 
 s->catalog_size = le32_to_cpu(ph.catalog_entries);
 if (s->catalog_size > INT_MAX / 4) {
@@ -143,7 +157,8 @@ static int64_t seek_to_sector(BlockDriverState *bs, int64_t 
sector_num)
 /* not allocated */
 if ((index > s->catalog_size) || (s->catalog_bitmap[index] == 0))
 return -1;
-return (uint64_t)(s->catalog_bitmap[index] + offset) * 512;
+return
+((uint64_t)s->catalog_bitmap[index] * s->off_multiplier + offset) * 
512;
 }
 
 static int parallels_read(BlockDriverState *bs, int64_t sector_num,
-- 
1.9.3

[Qemu-devel] [PULL 27/55] cmd646: switch cmd646_update_irq() to accept PCIDevice instead of PCIIDEState

[Stefan Hajnoczi]

From: Mark Cave-Ayland 

This is in preparation for adding configuration space accessors which accept
PCIDevice as a parameter.

Signed-off-by: Mark Cave-Ayland 
Signed-off-by: Stefan Hajnoczi 
---
 hw/ide/cmd646.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
index c3c6c53..11a3e52 100644
--- a/hw/ide/cmd646.c
+++ b/hw/ide/cmd646.c
@@ -48,7 +48,7 @@
 #define UDIDETCR0  0x73
 #define UDIDETCR1  0x7B
 
-static void cmd646_update_irq(PCIIDEState *d);
+static void cmd646_update_irq(PCIDevice *pd);
 
 static uint64_t cmd646_cmd_read(void *opaque, hwaddr addr,
 unsigned size)
@@ -205,7 +205,7 @@ static void bmdma_write(void *opaque, hwaddr addr,
 pci_dev->config[MRDMODE] =
 (pci_dev->config[MRDMODE] & ~0x30) | (val & 0x30);
 cmd646_update_dma_interrupts(pci_dev);
-cmd646_update_irq(bm->pci_dev);
+cmd646_update_irq(pci_dev);
 break;
 case 2:
 bm->status = (val & 0x60) | (bm->status & 1) | (bm->status & ~val & 
0x06);
@@ -245,9 +245,8 @@ static void bmdma_setup_bar(PCIIDEState *d)
 
 /* XXX: call it also when the MRDMODE is changed from the PCI config
registers */
-static void cmd646_update_irq(PCIIDEState *d)
+static void cmd646_update_irq(PCIDevice *pd)
 {
-PCIDevice *pd = PCI_DEVICE(d);
 int pci_level;
 
 pci_level = ((pd->config[MRDMODE] & MRDMODE_INTR_CH0) &&
@@ -271,7 +270,7 @@ static void cmd646_set_irq(void *opaque, int channel, int 
level)
 pd->config[MRDMODE] &= ~irq_mask;
 }
 cmd646_update_dma_interrupts(pd);
-cmd646_update_irq(d);
+cmd646_update_irq(pd);
 }
 
 static void cmd646_reset(void *opaque)
-- 
1.9.3

[Qemu-devel] [PULL 37/55] parallels: split check for parallels format in parallels_open

[Stefan Hajnoczi]

From: "Denis V. Lunev" 

and rework error path a bit. There is no difference at the moment, but
the code will be definitely shorter when additional processing will
be required for WithouFreSpacExt

Signed-off-by: Denis V. Lunev 
CC: Jeff Cody 
CC: Kevin Wolf 
CC: Stefan Hajnoczi 
Reviewed-by: Jeff Cody 
Signed-off-by: Stefan Hajnoczi 
---
 block/parallels.c | 13 -
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/block/parallels.c b/block/parallels.c
index e0cf1d9..f9e18b4 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -85,11 +85,11 @@ static int parallels_open(BlockDriverState *bs, QDict 
*options, int flags,
 goto fail;
 }
 
-if (memcmp(ph.magic, HEADER_MAGIC, 16) ||
-(le32_to_cpu(ph.version) != HEADER_VERSION)) {
-error_setg(errp, "Image not in Parallels format");
-ret = -EINVAL;
-goto fail;
+if (le32_to_cpu(ph.version) != HEADER_VERSION) {
+goto fail_format;
+}
+if (memcmp(ph.magic, HEADER_MAGIC, 16)) {
+goto fail_format;
 }
 
 bs->total_sectors = 0x & le64_to_cpu(ph.nb_sectors);
@@ -124,6 +124,9 @@ static int parallels_open(BlockDriverState *bs, QDict 
*options, int flags,
 qemu_co_mutex_init(&s->lock);
 return 0;
 
+fail_format:
+error_setg(errp, "Image not in Parallels format");
+ret = -EINVAL;
 fail:
 g_free(s->catalog_bitmap);
 return ret;
-- 
1.9.3

[Qemu-devel] [PULL 43/55] libqos: Change free function called in malloc

[Stefan Hajnoczi]

From: Marc Marí 

Reviewed-by: John Snow 
Reviewed-by: Stefan Hajnoczi 
Signed-off-by: Marc Marí 
Signed-off-by: Stefan Hajnoczi 
---
 tests/libqos/malloc.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/libqos/malloc.h b/tests/libqos/malloc.h
index 46f6000..5565381 100644
--- a/tests/libqos/malloc.h
+++ b/tests/libqos/malloc.h
@@ -32,7 +32,7 @@ static inline uint64_t guest_alloc(QGuestAllocator 
*allocator, size_t size)
 
 static inline void guest_free(QGuestAllocator *allocator, uint64_t addr)
 {
-allocator->alloc(allocator, addr);
+allocator->free(allocator, addr);
 }
 
 #endif
-- 
1.9.3

[Qemu-devel] [PULL 35/55] parallels: extend parallels format header with actual data values

[Stefan Hajnoczi]

From: "Denis V. Lunev" 

Parallels image format has several additional fields inside:
- nb_sectors is actually 64 bit wide. Upper 32bits are not used for
  images with signature "WithoutFreeSpace" and must be explicitly
  zeroed according to Parallels. They will be used for images with
  signature "WithouFreSpacExt"
- inuse is magic which means that the image is currently opened for
  read/write or was not closed correctly, the magic is 0x746f6e59
- data_off is the location of the first data block. It can be zero
  and in this case data starts just beyond the header aligned to
  512 bytes. Though this field does not matter for read-only driver

This patch adds these values to struct parallels_header and adds
proper handling of nb_sectors for currently supported WithoutFreeSpace
images.

WithouFreSpacExt will be covered in next patches.

Signed-off-by: Denis V. Lunev 
CC: Kevin Wolf 
CC: Stefan Hajnoczi 
CC: Jeff Cody 
Reviewed-by: Jeff Cody 
Signed-off-by: Stefan Hajnoczi 
---
 block/parallels.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/block/parallels.c b/block/parallels.c
index 7325678..59cfad6 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -41,8 +41,10 @@ struct parallels_header {
 uint32_t cylinders;
 uint32_t tracks;
 uint32_t catalog_entries;
-uint32_t nb_sectors;
-char padding[24];
+uint64_t nb_sectors;
+uint32_t inuse;
+uint32_t data_off;
+char padding[12];
 } QEMU_PACKED;
 
 typedef struct BDRVParallelsState {
@@ -90,7 +92,7 @@ static int parallels_open(BlockDriverState *bs, QDict 
*options, int flags,
 goto fail;
 }
 
-bs->total_sectors = le32_to_cpu(ph.nb_sectors);
+bs->total_sectors = 0x & le64_to_cpu(ph.nb_sectors);
 
 s->tracks = le32_to_cpu(ph.tracks);
 if (s->tracks == 0) {
-- 
1.9.3

[Qemu-devel] [PULL 28/55] cmd646: allow MRDMODE interrupt status bits clearing from PCI config space

[Stefan Hajnoczi]

From: Mark Cave-Ayland 

Make sure that we also update the normal DMA interrupt status bits at the
same time, and alter the IRQ if being cleared accordingly.

Signed-off-by: Mark Cave-Ayland 
Signed-off-by: Stefan Hajnoczi 
---
 hw/ide/cmd646.c | 32 ++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
index 11a3e52..b8dc4ab 100644
--- a/hw/ide/cmd646.c
+++ b/hw/ide/cmd646.c
@@ -243,8 +243,6 @@ static void bmdma_setup_bar(PCIIDEState *d)
 }
 }
 
-/* XXX: call it also when the MRDMODE is changed from the PCI config
-   registers */
 static void cmd646_update_irq(PCIDevice *pd)
 {
 int pci_level;
@@ -283,6 +281,30 @@ static void cmd646_reset(void *opaque)
 }
 }
 
+static uint32_t cmd646_pci_config_read(PCIDevice *d,
+   uint32_t address, int len)
+{
+return pci_default_read_config(d, address, len);
+}
+
+static void cmd646_pci_config_write(PCIDevice *d, uint32_t addr, uint32_t val,
+int l)
+{
+uint32_t i;
+
+pci_default_write_config(d, addr, val, l);
+
+for (i = addr; i < addr + l; i++) {
+switch (i) {
+case MRDMODE:
+cmd646_update_dma_interrupts(d);
+break;
+}
+}
+
+cmd646_update_irq(d);
+}
+
 /* CMD646 PCI IDE controller */
 static int pci_cmd646_ide_initfn(PCIDevice *dev)
 {
@@ -299,6 +321,10 @@ static int pci_cmd646_ide_initfn(PCIDevice *dev)
 pci_conf[CNTRL] |= CNTRL_EN_CH1; /* enable IDE1 */
 }
 
+/* Set write-to-clear interrupt bits */
+dev->wmask[MRDMODE] = 0x0;
+dev->w1cmask[MRDMODE] = MRDMODE_INTR_CH0 | MRDMODE_INTR_CH1;
+
 setup_cmd646_bar(d, 0);
 setup_cmd646_bar(d, 1);
 pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_IO, 
&d->cmd646_bar[0].data);
@@ -371,6 +397,8 @@ static void cmd646_ide_class_init(ObjectClass *klass, void 
*data)
 k->device_id = PCI_DEVICE_ID_CMD_646;
 k->revision = 0x07;
 k->class_id = PCI_CLASS_STORAGE_IDE;
+k->config_read = cmd646_pci_config_read;
+k->config_write = cmd646_pci_config_write;
 dc->props = cmd646_ide_properties;
 }
 
-- 
1.9.3

[Qemu-devel] [PULL 33/55] dataplane: fail notifier setting gracefully

[Stefan Hajnoczi]

From: Cornelia Huck 

The dataplane code is currently doing a hard exit if it fails to set
up either guest or host notifiers. In practice, this may mean that a
guest suddenly dies after a dataplane device failed to come up (e.g.,
when a file descriptor limit is hit for tne nth device).

Let's just try to unwind the setup instead and return.

Acked-by: Christian Borntraeger 
Signed-off-by: Cornelia Huck 
Signed-off-by: Stefan Hajnoczi 
---
 hw/block/dataplane/virtio-blk.c | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index 527a53c..94e1a29 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -232,8 +232,7 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
 
 vq = virtio_get_queue(s->vdev, 0);
 if (!vring_setup(&s->vring, s->vdev, 0)) {
-s->starting = false;
-return;
+goto fail_vring;
 }
 
 /* Set up guest notifier (irq) */
@@ -241,7 +240,7 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
 if (r != 0) {
 fprintf(stderr, "virtio-blk failed to set guest notifier (%d), "
 "ensure -enable-kvm is set\n", r);
-exit(1);
+goto fail_guest_notifiers;
 }
 s->guest_notifier = virtio_queue_get_guest_notifier(vq);
 
@@ -249,7 +248,7 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
 r = k->set_host_notifier(qbus->parent, 0, true);
 if (r != 0) {
 fprintf(stderr, "virtio-blk failed to set host notifier (%d)\n", r);
-exit(1);
+goto fail_host_notifier;
 }
 s->host_notifier = *virtio_queue_get_host_notifier(vq);
 
@@ -269,6 +268,14 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
 aio_context_acquire(s->ctx);
 aio_set_event_notifier(s->ctx, &s->host_notifier, handle_notify);
 aio_context_release(s->ctx);
+return;
+
+  fail_host_notifier:
+k->set_guest_notifiers(qbus->parent, 1, false);
+  fail_guest_notifiers:
+vring_teardown(&s->vring, s->vdev, 0);
+  fail_vring:
+s->starting = false;
 }
 
 /* Context: QEMU global mutex held */
-- 
1.9.3

[Qemu-devel] [PULL 41/55] libqtest: add QTEST_LOG for debugging qtest testcases

[Stefan Hajnoczi]

From: Marc Marí 

Signed-off-by: Paolo Bonzini 
Signed-off-by: Marc Marí 
Signed-off-by: Stefan Hajnoczi 
---
 tests/libqtest.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/tests/libqtest.c b/tests/libqtest.c
index 0bf17aa..ed55686 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -402,10 +402,14 @@ QDict *qtest_qmpv(QTestState *s, const char *fmt, va_list 
ap)
 
 /* No need to send anything for an empty QObject.  */
 if (qobj) {
+int log = getenv("QTEST_LOG") != NULL;
 QString *qstr = qobject_to_json(qobj);
 const char *str = qstring_get_str(qstr);
 size_t size = qstring_get_length(qstr);
 
+if (log) {
+fprintf(stderr, "%s", str);
+}
 /* Send QMP request */
 socket_send(s->qmp_fd, str, size);
 
-- 
1.9.3

[Qemu-devel] [PULL 24/55] qtest/ide: Fix small memory leak

[Stefan Hajnoczi]

From: John Snow 

For libqos debugging purposes, it's nice to
be able to assert that tests and associated libraries
have no memory leaks. To that end, free up the
trivial cmdline leak.

The remaining leaks caused by pc_alloc_init are fixed
instead by my first-fit pc_alloc implementation already
on the qemu-devel mailing list.

Signed-off-by: John Snow 
Signed-off-by: Stefan Hajnoczi 
---
 tests/ide-test.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/ide-test.c b/tests/ide-test.c
index e2b4efc..a77a037 100644
--- a/tests/ide-test.c
+++ b/tests/ide-test.c
@@ -120,6 +120,8 @@ static void ide_test_start(const char *cmdline_fmt, ...)
 qtest_start(cmdline);
 qtest_irq_intercept_in(global_qtest, "ioapic");
 guest_malloc = pc_alloc_init();
+
+g_free(cmdline);
 }
 
 static void ide_test_quit(void)
-- 
1.9.3

[Qemu-devel] [PULL 32/55] dataplane: print why starting failed

[Stefan Hajnoczi]

From: Cornelia Huck 

Setting up guest or host notifiers may fail, but the user will have
no idea why: Let's print the error returned by the callback.

Acked-by: Christian Borntraeger 
Signed-off-by: Cornelia Huck 
Signed-off-by: Stefan Hajnoczi 
---
 hw/block/dataplane/virtio-blk.c | 13 -
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index d6ba65c..527a53c 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -218,6 +218,7 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
 VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
 VirtIOBlock *vblk = VIRTIO_BLK(s->vdev);
 VirtQueue *vq;
+int r;
 
 if (s->started) {
 return;
@@ -236,16 +237,18 @@ void virtio_blk_data_plane_start(VirtIOBlockDataPlane *s)
 }
 
 /* Set up guest notifier (irq) */
-if (k->set_guest_notifiers(qbus->parent, 1, true) != 0) {
-fprintf(stderr, "virtio-blk failed to set guest notifier, "
-"ensure -enable-kvm is set\n");
+r = k->set_guest_notifiers(qbus->parent, 1, true);
+if (r != 0) {
+fprintf(stderr, "virtio-blk failed to set guest notifier (%d), "
+"ensure -enable-kvm is set\n", r);
 exit(1);
 }
 s->guest_notifier = virtio_queue_get_guest_notifier(vq);
 
 /* Set up virtqueue notify */
-if (k->set_host_notifier(qbus->parent, 0, true) != 0) {
-fprintf(stderr, "virtio-blk failed to set host notifier\n");
+r = k->set_host_notifier(qbus->parent, 0, true);
+if (r != 0) {
+fprintf(stderr, "virtio-blk failed to set host notifier (%d)\n", r);
 exit(1);
 }
 s->host_notifier = *virtio_queue_get_host_notifier(vq);
-- 
1.9.3

[Qemu-devel] [PULL 40/55] ide: Fix segfault when flushing a device that doesn't exist

[Stefan Hajnoczi]

From: Kevin Wolf 

Signed-off-by: Kevin Wolf 
Signed-off-by: Stefan Hajnoczi 
---
 hw/ide/core.c|  4 +++-
 tests/ide-test.c | 14 ++
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index bdb0a80..82dd4af 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -848,7 +848,9 @@ static void ide_flush_cb(void *opaque, int ret)
 }
 }
 
-bdrv_acct_done(s->bs, &s->acct);
+if (s->bs) {
+bdrv_acct_done(s->bs, &s->acct);
+}
 s->status = READY_STAT | SEEK_STAT;
 ide_cmd_done(s);
 ide_set_irq(s->bus);
diff --git a/tests/ide-test.c b/tests/ide-test.c
index a77a037..ffce6ed 100644
--- a/tests/ide-test.c
+++ b/tests/ide-test.c
@@ -564,6 +564,19 @@ static void test_retry_flush(void)
 ide_test_quit();
 }
 
+static void test_flush_nodev(void)
+{
+ide_test_start("");
+
+/* FLUSH CACHE command on device 0*/
+outb(IDE_BASE + reg_device, 0);
+outb(IDE_BASE + reg_command, CMD_FLUSH_CACHE);
+
+/* Just testing that qemu doesn't crash... */
+
+ide_test_quit();
+}
+
 int main(int argc, char **argv)
 {
 const char *arch = qtest_get_arch();
@@ -601,6 +614,7 @@ int main(int argc, char **argv)
 qtest_add_func("/ide/bmdma/teardown", test_bmdma_teardown);
 
 qtest_add_func("/ide/flush", test_flush);
+qtest_add_func("/ide/flush_nodev", test_flush_nodev);
 
 qtest_add_func("/ide/retry/flush", test_retry_flush);
 
-- 
1.9.3

[Qemu-devel] [PULL 31/55] channel-posix: using qemu_set_nonblock() instead of fcntl(O_NONBLOCK)

[Stefan Hajnoczi]

From: Gonglei 

Technically, fcntl(soc, F_SETFL, O_NONBLOCK)
is incorrect since it clobbers all other file flags.
We can use F_GETFL to get the current flags, set or
clear the O_NONBLOCK flag, then use F_SETFL to set the flags.

Using the qemu_set_nonblock() wrapper.

Signed-off-by: Gonglei 
Signed-off-by: Wangxin 
Reviewed-by: Eric Blake 
Signed-off-by: Stefan Hajnoczi 
---
 qga/channel-posix.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qga/channel-posix.c b/qga/channel-posix.c
index e65dda3..8aad4fe 100644
--- a/qga/channel-posix.c
+++ b/qga/channel-posix.c
@@ -42,7 +42,7 @@ static gboolean ga_channel_listen_accept(GIOChannel *channel,
 g_warning("error converting fd to gsocket: %s", strerror(errno));
 goto out;
 }
-fcntl(client_fd, F_SETFL, O_NONBLOCK);
+qemu_set_nonblock(client_fd);
 ret = ga_channel_client_add(c, client_fd);
 if (ret) {
 g_warning("error setting up connection");
-- 
1.9.3

[Qemu-devel] [PULL 22/55] libqos: Fixes a small memory leak.

[Stefan Hajnoczi]

From: John Snow 

Allow users the chance to clean up the QPCIBusPC structure
by adding a small cleanup routine. Helps clear up small
memory leaks during setup/teardown, to allow for cleaner
debug output messages.

Signed-off-by: John Snow 
Signed-off-by: Stefan Hajnoczi 
---
 tests/libqos/pci-pc.c | 7 +++
 tests/libqos/pci-pc.h | 1 +
 2 files changed, 8 insertions(+)

diff --git a/tests/libqos/pci-pc.c b/tests/libqos/pci-pc.c
index 4adf400..f5d6469 100644
--- a/tests/libqos/pci-pc.c
+++ b/tests/libqos/pci-pc.c
@@ -237,3 +237,10 @@ QPCIBus *qpci_init_pc(void)
 
 return &ret->bus;
 }
+
+void qpci_free_pc(QPCIBus *bus)
+{
+QPCIBusPC *s = container_of(bus, QPCIBusPC, bus);
+
+g_free(s);
+}
diff --git a/tests/libqos/pci-pc.h b/tests/libqos/pci-pc.h
index 4f7475f..2621179 100644
--- a/tests/libqos/pci-pc.h
+++ b/tests/libqos/pci-pc.h
@@ -16,5 +16,6 @@
 #include "libqos/pci.h"
 
 QPCIBus *qpci_init_pc(void);
+void qpci_free_pc(QPCIBus *bus);
 
 #endif
-- 
1.9.3

[Qemu-devel] [PULL 23/55] libqos: allow qpci_iomap to return BAR mapping size

[Stefan Hajnoczi]

From: John Snow 

This patch allows qpci_iomap to return the size of the
BAR mapping that it created, to allow driver applications
(e.g, ahci-test) to make determinations about the suitability
or the mapping size, or in the specific case of AHCI, how
many ports are supported by the HBA.

Signed-off-by: John Snow 
Signed-off-by: Stefan Hajnoczi 
---
 tests/ide-test.c  | 2 +-
 tests/libqos/pci-pc.c | 5 -
 tests/libqos/pci.c| 4 ++--
 tests/libqos/pci.h| 4 ++--
 tests/usb-hcd-ehci-test.c | 2 +-
 5 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/tests/ide-test.c b/tests/ide-test.c
index 151ef30..e2b4efc 100644
--- a/tests/ide-test.c
+++ b/tests/ide-test.c
@@ -146,7 +146,7 @@ static QPCIDevice *get_pci_device(uint16_t *bmdma_base)
 g_assert(device_id == PCI_DEVICE_ID_INTEL_82371SB_1);
 
 /* Map bmdma BAR */
-*bmdma_base = (uint16_t)(uintptr_t) qpci_iomap(dev, 4);
+*bmdma_base = (uint16_t)(uintptr_t) qpci_iomap(dev, 4, NULL);
 
 qpci_device_enable(dev);
 
diff --git a/tests/libqos/pci-pc.c b/tests/libqos/pci-pc.c
index f5d6469..0609294 100644
--- a/tests/libqos/pci-pc.c
+++ b/tests/libqos/pci-pc.c
@@ -144,7 +144,7 @@ static void qpci_pc_config_writel(QPCIBus *bus, int devfn, 
uint8_t offset, uint3
 outl(0xcfc, value);
 }
 
-static void *qpci_pc_iomap(QPCIBus *bus, QPCIDevice *dev, int barno)
+static void *qpci_pc_iomap(QPCIBus *bus, QPCIDevice *dev, int barno, uint64_t 
*sizeptr)
 {
 QPCIBusPC *s = container_of(bus, QPCIBusPC, bus);
 static const int bar_reg_map[] = {
@@ -173,6 +173,9 @@ static void *qpci_pc_iomap(QPCIBus *bus, QPCIDevice *dev, 
int barno)
 if (size == 0) {
 return NULL;
 }
+if (sizeptr) {
+*sizeptr = size;
+}
 
 if (io_type == PCI_BASE_ADDRESS_SPACE_IO) {
 uint16_t loc;
diff --git a/tests/libqos/pci.c b/tests/libqos/pci.c
index c9a0b91..ce0b308 100644
--- a/tests/libqos/pci.c
+++ b/tests/libqos/pci.c
@@ -138,9 +138,9 @@ void qpci_io_writel(QPCIDevice *dev, void *data, uint32_t 
value)
 dev->bus->io_writel(dev->bus, data, value);
 }
 
-void *qpci_iomap(QPCIDevice *dev, int barno)
+void *qpci_iomap(QPCIDevice *dev, int barno, uint64_t *sizeptr)
 {
-return dev->bus->iomap(dev->bus, dev, barno);
+return dev->bus->iomap(dev->bus, dev, barno, sizeptr);
 }
 
 void qpci_iounmap(QPCIDevice *dev, void *data)
diff --git a/tests/libqos/pci.h b/tests/libqos/pci.h
index 3439431..9ee048b 100644
--- a/tests/libqos/pci.h
+++ b/tests/libqos/pci.h
@@ -41,7 +41,7 @@ struct QPCIBus
 void (*config_writel)(QPCIBus *bus, int devfn,
   uint8_t offset, uint32_t value);
 
-void *(*iomap)(QPCIBus *bus, QPCIDevice *dev, int barno);
+void *(*iomap)(QPCIBus *bus, QPCIDevice *dev, int barno, uint64_t 
*sizeptr);
 void (*iounmap)(QPCIBus *bus, void *data);
 };
 
@@ -74,7 +74,7 @@ void qpci_io_writeb(QPCIDevice *dev, void *data, uint8_t 
value);
 void qpci_io_writew(QPCIDevice *dev, void *data, uint16_t value);
 void qpci_io_writel(QPCIDevice *dev, void *data, uint32_t value);
 
-void *qpci_iomap(QPCIDevice *dev, int barno);
+void *qpci_iomap(QPCIDevice *dev, int barno, uint64_t *sizeptr);
 void qpci_iounmap(QPCIDevice *dev, void *data);
 
 #endif
diff --git a/tests/usb-hcd-ehci-test.c b/tests/usb-hcd-ehci-test.c
index bcdf62f..c990492 100644
--- a/tests/usb-hcd-ehci-test.c
+++ b/tests/usb-hcd-ehci-test.c
@@ -34,7 +34,7 @@ static void pci_init_one(struct qhc *hc, uint32_t devfn, int 
bar)
 hc->dev = qpci_device_find(pcibus, devfn);
 g_assert(hc->dev != NULL);
 qpci_device_enable(hc->dev);
-hc->base = qpci_iomap(hc->dev, bar);
+hc->base = qpci_iomap(hc->dev, bar, NULL);
 g_assert(hc->base != NULL);
 }
 
-- 
1.9.3

[Qemu-devel] [PULL 25/55] cmd646: add constants for CNTRL register access

[Stefan Hajnoczi]

From: Mark Cave-Ayland 

Signed-off-by: Mark Cave-Ayland 
Signed-off-by: Stefan Hajnoczi 
---
 hw/ide/cmd646.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
index a8e35fe..d8395ef 100644
--- a/hw/ide/cmd646.c
+++ b/hw/ide/cmd646.c
@@ -33,6 +33,9 @@
 #include 
 
 /* CMD646 specific */
+#define CNTRL  0x51
+#define   CNTRL_EN_CH0 0x04
+#define   CNTRL_EN_CH1 0x08
 #define MRDMODE0x71
 #define   MRDMODE_INTR_CH0 0x04
 #define   MRDMODE_INTR_CH1 0x08
@@ -269,10 +272,10 @@ static int pci_cmd646_ide_initfn(PCIDevice *dev)
 
 pci_conf[PCI_CLASS_PROG] = 0x8f;
 
-pci_conf[0x51] = 0x04; // enable IDE0
+pci_conf[CNTRL] = CNTRL_EN_CH0; // enable IDE0
 if (d->secondary) {
 /* XXX: if not enabled, really disable the seconday IDE controller */
-pci_conf[0x51] |= 0x08; /* enable IDE1 */
+pci_conf[CNTRL] |= CNTRL_EN_CH1; /* enable IDE1 */
 }
 
 setup_cmd646_bar(d, 0);
-- 
1.9.3

[Qemu-devel] [PULL 29/55] cmd646: synchronise UDMA interrupt status with DMA interrupt status

[Stefan Hajnoczi]

From: Mark Cave-Ayland 

Make sure that both registers are synchronised when being accessed through
PCI configuration space.

Signed-off-by: Mark Cave-Ayland 
Signed-off-by: Stefan Hajnoczi 
---
 hw/ide/cmd646.c | 24 
 1 file changed, 24 insertions(+)

diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
index b8dc4ab..74d0deb 100644
--- a/hw/ide/cmd646.c
+++ b/hw/ide/cmd646.c
@@ -146,6 +146,22 @@ static void cmd646_update_dma_interrupts(PCIDevice *pd)
 }
 }
 
+static void cmd646_update_udma_interrupts(PCIDevice *pd)
+{
+/* Sync UDMA interrupt status from DMA interrupt status */
+if (pd->config[CFR] & CFR_INTR_CH0) {
+pd->config[MRDMODE] |= MRDMODE_INTR_CH0;
+} else {
+pd->config[MRDMODE] &= ~MRDMODE_INTR_CH0;
+}
+
+if (pd->config[ARTTIM23] & ARTTIM23_INTR_CH1) {
+pd->config[MRDMODE] |= MRDMODE_INTR_CH1;
+} else {
+pd->config[MRDMODE] &= ~MRDMODE_INTR_CH1;
+}
+}
+
 static uint64_t bmdma_read(void *opaque, hwaddr addr,
unsigned size)
 {
@@ -296,6 +312,10 @@ static void cmd646_pci_config_write(PCIDevice *d, uint32_t 
addr, uint32_t val,
 
 for (i = addr; i < addr + l; i++) {
 switch (i) {
+case CFR:
+case ARTTIM23:
+cmd646_update_udma_interrupts(d);
+break;
 case MRDMODE:
 cmd646_update_dma_interrupts(d);
 break;
@@ -322,6 +342,10 @@ static int pci_cmd646_ide_initfn(PCIDevice *dev)
 }
 
 /* Set write-to-clear interrupt bits */
+dev->wmask[CFR] = 0x0;
+dev->w1cmask[CFR] = CFR_INTR_CH0;
+dev->wmask[ARTTIM23] = 0x0;
+dev->w1cmask[ARTTIM23] = ARTTIM23_INTR_CH1;
 dev->wmask[MRDMODE] = 0x0;
 dev->w1cmask[MRDMODE] = MRDMODE_INTR_CH0 | MRDMODE_INTR_CH1;
 
-- 
1.9.3

[Qemu-devel] [PULL 20/55] libqos: Correct memory leak

[Stefan Hajnoczi]

From: John Snow 

Fix a small memory leak inside of libqos, in the pc_alloc_init routine.

Signed-off-by: John Snow 
Signed-off-by: Stefan Hajnoczi 
---
 tests/libqos/malloc-pc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tests/libqos/malloc-pc.c b/tests/libqos/malloc-pc.c
index db1496c..63af60a 100644
--- a/tests/libqos/malloc-pc.c
+++ b/tests/libqos/malloc-pc.c
@@ -67,5 +67,8 @@ QGuestAllocator *pc_alloc_init(void)
 /* Respect PCI hole */
 s->end = MIN(ram_size, 0xE000);
 
+/* clean-up */
+g_free(fw_cfg);
+
 return &s->alloc;
 }
-- 
1.9.3

[Qemu-devel] [PULL 21/55] libqtest: Correct small memory leak.

[Stefan Hajnoczi]

From: John Snow 

Fixes a small memory leak inside of libqtest.
After we produce a test path and glib copies the string
for itself, we should clean up our temporary copy.

Signed-off-by: John Snow 
Signed-off-by: Stefan Hajnoczi 
---
 tests/libqtest.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/libqtest.c b/tests/libqtest.c
index e525e6f..0bf17aa 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -644,6 +644,7 @@ void qtest_add_func(const char *str, void (*fn))
 {
 gchar *path = g_strdup_printf("/%s/%s", qtest_get_arch(), str);
 g_test_add_func(path, fn);
+g_free(path);
 }
 
 void qtest_memwrite(QTestState *s, uint64_t addr, const void *data, size_t 
size)
-- 
1.9.3

[Qemu-devel] [PULL 26/55] cmd646: synchronise DMA interrupt status with UDMA interrupt status

[Stefan Hajnoczi]

From: Mark Cave-Ayland 

Make sure that the standard DMA interrupt status bits reflect any changes made
to the UDMA interrupt status bits. The CMD646U2 datasheet claims that these
bits are equivalent, and they must be synchronised for guests that manipulate
both registers.

Signed-off-by: Mark Cave-Ayland 
Signed-off-by: Stefan Hajnoczi 
---
 hw/ide/cmd646.c | 22 ++
 1 file changed, 22 insertions(+)

diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
index d8395ef..c3c6c53 100644
--- a/hw/ide/cmd646.c
+++ b/hw/ide/cmd646.c
@@ -33,9 +33,13 @@
 #include 
 
 /* CMD646 specific */
+#define CFR0x50
+#define   CFR_INTR_CH0 0x04
 #define CNTRL  0x51
 #define   CNTRL_EN_CH0 0x04
 #define   CNTRL_EN_CH1 0x08
+#define ARTTIM23   0x57
+#defineARTTIM23_INTR_CH1   0x10
 #define MRDMODE0x71
 #define   MRDMODE_INTR_CH0 0x04
 #define   MRDMODE_INTR_CH1 0x08
@@ -126,6 +130,22 @@ static void setup_cmd646_bar(PCIIDEState *d, int bus_num)
   "cmd646-data", 8);
 }
 
+static void cmd646_update_dma_interrupts(PCIDevice *pd)
+{
+/* Sync DMA interrupt status from UDMA interrupt status */
+if (pd->config[MRDMODE] & MRDMODE_INTR_CH0) {
+pd->config[CFR] |= CFR_INTR_CH0;
+} else {
+pd->config[CFR] &= ~CFR_INTR_CH0;
+}
+
+if (pd->config[MRDMODE] & MRDMODE_INTR_CH1) {
+pd->config[ARTTIM23] |= ARTTIM23_INTR_CH1;
+} else {
+pd->config[ARTTIM23] &= ~ARTTIM23_INTR_CH1;
+}
+}
+
 static uint64_t bmdma_read(void *opaque, hwaddr addr,
unsigned size)
 {
@@ -184,6 +204,7 @@ static void bmdma_write(void *opaque, hwaddr addr,
 case 1:
 pci_dev->config[MRDMODE] =
 (pci_dev->config[MRDMODE] & ~0x30) | (val & 0x30);
+cmd646_update_dma_interrupts(pci_dev);
 cmd646_update_irq(bm->pci_dev);
 break;
 case 2:
@@ -249,6 +270,7 @@ static void cmd646_set_irq(void *opaque, int channel, int 
level)
 } else {
 pd->config[MRDMODE] &= ~irq_mask;
 }
+cmd646_update_dma_interrupts(pd);
 cmd646_update_irq(d);
 }
 
-- 
1.9.3

[Qemu-devel] [PULL 36/55] parallels: replace tabs with spaces in block/parallels.c

[Stefan Hajnoczi]

From: "Denis V. Lunev" 

Signed-off-by: Denis V. Lunev 
Reviewed-by: Jeff Cody 
CC: Kevin Wolf 
CC: Stefan Hajnoczi 
Signed-off-by: Stefan Hajnoczi 
---
 block/parallels.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/block/parallels.c b/block/parallels.c
index 59cfad6..e0cf1d9 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -61,11 +61,11 @@ static int parallels_probe(const uint8_t *buf, int 
buf_size, const char *filenam
 const struct parallels_header *ph = (const void *)buf;
 
 if (buf_size < HEADER_SIZE)
-   return 0;
+return 0;
 
 if (!memcmp(ph->magic, HEADER_MAGIC, 16) &&
-   (le32_to_cpu(ph->version) == HEADER_VERSION))
-   return 100;
+(le32_to_cpu(ph->version) == HEADER_VERSION))
+return 100;
 
 return 0;
 }
@@ -119,7 +119,7 @@ static int parallels_open(BlockDriverState *bs, QDict 
*options, int flags,
 }
 
 for (i = 0; i < s->catalog_size; i++)
-   le32_to_cpus(&s->catalog_bitmap[i]);
+le32_to_cpus(&s->catalog_bitmap[i]);
 
 qemu_co_mutex_init(&s->lock);
 return 0;
@@ -139,7 +139,7 @@ static int64_t seek_to_sector(BlockDriverState *bs, int64_t 
sector_num)
 
 /* not allocated */
 if ((index > s->catalog_size) || (s->catalog_bitmap[index] == 0))
-   return -1;
+return -1;
 return (uint64_t)(s->catalog_bitmap[index] + offset) * 512;
 }
 
-- 
1.9.3

[Qemu-devel] [PULL 19/55] qtest: Adding qtest_memset and qmemset.

[Stefan Hajnoczi]

From: John Snow 

Currently, libqtest allows for memread and memwrite, but
does not offer a simple way to zero out regions of memory.
This patch adds a simple function to do so.

Signed-off-by: John Snow 
Signed-off-by: Stefan Hajnoczi 
---
 tests/libqtest.c | 12 
 tests/libqtest.h | 24 
 2 files changed, 36 insertions(+)

diff --git a/tests/libqtest.c b/tests/libqtest.c
index 4a75cd3..e525e6f 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -659,6 +659,18 @@ void qtest_memwrite(QTestState *s, uint64_t addr, const 
void *data, size_t size)
 qtest_rsp(s, 0);
 }
 
+void qtest_memset(QTestState *s, uint64_t addr, uint8_t pattern, size_t size)
+{
+size_t i;
+
+qtest_sendf(s, "write 0x%" PRIx64 " 0x%zx 0x", addr, size);
+for (i = 0; i < size; i++) {
+qtest_sendf(s, "%02x", pattern);
+}
+qtest_sendf(s, "\n");
+qtest_rsp(s, 0);
+}
+
 QDict *qmp(const char *fmt, ...)
 {
 va_list ap;
diff --git a/tests/libqtest.h b/tests/libqtest.h
index 8f323c7..1be0934 100644
--- a/tests/libqtest.h
+++ b/tests/libqtest.h
@@ -283,6 +283,17 @@ void qtest_memread(QTestState *s, uint64_t addr, void 
*data, size_t size);
 void qtest_memwrite(QTestState *s, uint64_t addr, const void *data, size_t 
size);
 
 /**
+ * qtest_memset:
+ * @s: #QTestState instance to operate on.
+ * @addr: Guest address to write to.
+ * @patt: Byte pattern to fill the guest memory region with.
+ * @size: Number of bytes to write.
+ *
+ * Write a pattern to guest memory.
+ */
+void qtest_memset(QTestState *s, uint64_t addr, uint8_t patt, size_t size);
+
+/**
  * qtest_clock_step_next:
  * @s: #QTestState instance to operate on.
  *
@@ -621,6 +632,19 @@ static inline void memwrite(uint64_t addr, const void 
*data, size_t size)
 }
 
 /**
+ * qmemset:
+ * @addr: Guest address to write to.
+ * @patt: Byte pattern to fill the guest memory region with.
+ * @size: Number of bytes to write.
+ *
+ * Write a pattern to guest memory.
+ */
+static inline void qmemset(uint64_t addr, uint8_t patt, size_t size)
+{
+qtest_memset(global_qtest, addr, patt, size);
+}
+
+/**
  * clock_step_next:
  *
  * Advance the QEMU_CLOCK_VIRTUAL to the next deadline.
-- 
1.9.3

[Qemu-devel] [PULL 15/55] ide: stop PIO transfer on errors

[Stefan Hajnoczi]

From: Paolo Bonzini 

This will provide a hook for sending the result of the command via the
FIS receive area.

Signed-off-by: Paolo Bonzini 
Signed-off-by: John Snow 
Signed-off-by: Stefan Hajnoczi 
---
 hw/ide/core.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 00fe043..91a17e6 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -420,6 +420,7 @@ BlockDriverAIOCB *ide_issue_trim(BlockDriverState *bs,
 
 static inline void ide_abort_command(IDEState *s)
 {
+ide_transfer_stop(s);
 s->status = READY_STAT | ERR_STAT;
 s->error = ABRT_ERR;
 }
@@ -605,9 +606,7 @@ void ide_set_inactive(IDEState *s, bool more)
 
 void ide_dma_error(IDEState *s)
 {
-ide_transfer_stop(s);
-s->error = ABRT_ERR;
-s->status = READY_STAT | ERR_STAT;
+ide_abort_command(s);
 ide_set_inactive(s, false);
 ide_set_irq(s->bus);
 }
-- 
1.9.3

[Qemu-devel] [PULL 17/55] ahci: construct PIO Setup FIS for PIO commands

[Stefan Hajnoczi]

From: Paolo Bonzini 

PIO commands should put a PIO Setup FIS in the receive area when data
transfer ends.  Currently QEMU does not do this and only places the
D2H FIS at the end of the operation.

Signed-off-by: Paolo Bonzini 
Signed-off-by: John Snow 
Signed-off-by: Stefan Hajnoczi 
---
 hw/ide/ahci.c | 70 +++
 1 file changed, 70 insertions(+)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index b40ec06..4cda0d0 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -587,6 +587,71 @@ static void ahci_write_fis_sdb(AHCIState *s, int port, 
uint32_t finished)
 ahci_trigger_irq(s, &s->dev[port], PORT_IRQ_SDB_FIS);
 }
 
+static void ahci_write_fis_pio(AHCIDevice *ad, uint16_t len)
+{
+AHCIPortRegs *pr = &ad->port_regs;
+uint8_t *pio_fis, *cmd_fis;
+uint64_t tbl_addr;
+dma_addr_t cmd_len = 0x80;
+
+if (!ad->res_fis || !(pr->cmd & PORT_CMD_FIS_RX)) {
+return;
+}
+
+/* map cmd_fis */
+tbl_addr = le64_to_cpu(ad->cur_cmd->tbl_addr);
+cmd_fis = dma_memory_map(ad->hba->as, tbl_addr, &cmd_len,
+ DMA_DIRECTION_TO_DEVICE);
+
+if (cmd_fis == NULL) {
+DPRINTF(ad->port_no, "dma_memory_map failed in ahci_write_fis_pio");
+ahci_trigger_irq(ad->hba, ad, PORT_IRQ_HBUS_ERR);
+return;
+}
+
+if (cmd_len != 0x80) {
+DPRINTF(ad->port_no,
+"dma_memory_map mapped too few bytes in ahci_write_fis_pio");
+dma_memory_unmap(ad->hba->as, cmd_fis, cmd_len,
+ DMA_DIRECTION_TO_DEVICE, cmd_len);
+ahci_trigger_irq(ad->hba, ad, PORT_IRQ_HBUS_ERR);
+return;
+}
+
+pio_fis = &ad->res_fis[RES_FIS_PSFIS];
+
+pio_fis[0] = 0x5f;
+pio_fis[1] = (ad->hba->control_regs.irqstatus ? (1 << 6) : 0);
+pio_fis[2] = ad->port.ifs[0].status;
+pio_fis[3] = ad->port.ifs[0].error;
+
+pio_fis[4] = cmd_fis[4];
+pio_fis[5] = cmd_fis[5];
+pio_fis[6] = cmd_fis[6];
+pio_fis[7] = cmd_fis[7];
+pio_fis[8] = cmd_fis[8];
+pio_fis[9] = cmd_fis[9];
+pio_fis[10] = cmd_fis[10];
+pio_fis[11] = cmd_fis[11];
+pio_fis[12] = cmd_fis[12];
+pio_fis[13] = cmd_fis[13];
+pio_fis[14] = 0;
+pio_fis[15] = ad->port.ifs[0].status;
+pio_fis[16] = len & 255;
+pio_fis[17] = len >> 8;
+pio_fis[18] = 0;
+pio_fis[19] = 0;
+
+if (pio_fis[2] & ERR_STAT) {
+ahci_trigger_irq(ad->hba, ad, PORT_IRQ_TF_ERR);
+}
+
+ahci_trigger_irq(ad->hba, ad, PORT_IRQ_PIOS_FIS);
+
+dma_memory_unmap(ad->hba->as, cmd_fis, cmd_len,
+ DMA_DIRECTION_TO_DEVICE, cmd_len);
+}
+
 static void ahci_write_fis_d2h(AHCIDevice *ad, uint8_t *cmd_fis)
 {
 AHCIPortRegs *pr = &ad->port_regs;
@@ -1031,6 +1096,11 @@ out:
 }
 
 s->end_transfer_func(s);
+
+if (!(s->status & DRQ_STAT)) {
+/* done with PIO send/receive */
+ahci_write_fis_pio(ad, le32_to_cpu(ad->cur_cmd->status));
+}
 }
 
 static void ahci_start_dma(IDEDMA *dma, IDEState *s,
-- 
1.9.3

[Qemu-devel] [PULL 10/55] ide: remove wrong setting of BM_STATUS_INT

[Stefan Hajnoczi]

From: Paolo Bonzini 

Similar to the case removed in commit 69c38b8 (ide/core: Remove explicit
setting of BM_STATUS_INT, 2011-05-19), the only remaining use of
add_status(..., BM_STATUS_INT) is for short PRDs.  The flag should
not be raised in this case.

Signed-off-by: Paolo Bonzini 
Signed-off-by: John Snow 
Signed-off-by: Stefan Hajnoczi 
---
 hw/ide/ahci.c  | 4 
 hw/ide/atapi.c | 1 -
 2 files changed, 5 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index adbac3d..14677ec 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -1107,10 +1107,6 @@ static int ahci_dma_add_status(IDEDMA *dma, int status)
 AHCIDevice *ad = DO_UPCAST(AHCIDevice, dma, dma);
 DPRINTF(ad->port_no, "set status: %x\n", status);
 
-if (status & BM_STATUS_INT) {
-ahci_trigger_irq(ad->hba, ad, PORT_IRQ_STAT_DSS);
-}
-
 return 0;
 }
 
diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index d13395e..46ed3f5 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -355,7 +355,6 @@ static void ide_atapi_cmd_read_dma_cb(void *opaque, int ret)
 
 eot:
 bdrv_acct_done(s->bs, &s->acct);
-s->bus->dma->ops->add_status(s->bus->dma, BM_STATUS_INT);
 ide_set_inactive(s);
 }
 
-- 
1.9.3

[Qemu-devel] [PULL 12/55] ide: move BM_STATUS bits to pci.[ch]

[Stefan Hajnoczi]

From: Paolo Bonzini 

They are not used by AHCI, and should not be even available there.

Signed-off-by: Paolo Bonzini 
Signed-off-by: John Snow 
Signed-off-by: Stefan Hajnoczi 
---
 hw/ide/internal.h | 11 ---
 hw/ide/pci.c  |  4 
 hw/ide/pci.h  |  7 +++
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index b35e52c..f369b0b 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -484,10 +484,6 @@ struct IDEDevice {
 uint64_t wwn;
 };
 
-#define BM_STATUS_DMAING 0x01
-#define BM_STATUS_ERROR  0x02
-#define BM_STATUS_INT0x04
-
 /* FIXME These are not status register bits */
 #define BM_STATUS_DMA_RETRY  0x08
 #define BM_STATUS_PIO_RETRY  0x10
@@ -495,13 +491,6 @@ struct IDEDevice {
 #define BM_STATUS_RETRY_FLUSH 0x40
 #define BM_STATUS_RETRY_TRIM 0x80
 
-#define BM_MIGRATION_COMPAT_STATUS_BITS \
-(BM_STATUS_DMA_RETRY | BM_STATUS_PIO_RETRY | \
-BM_STATUS_RETRY_READ | BM_STATUS_RETRY_FLUSH)
-
-#define BM_CMD_START 0x01
-#define BM_CMD_READ  0x08
-
 static inline IDEState *idebus_active_if(IDEBus *bus)
 {
 return bus->ifs + bus->unit;
diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index 73267a4..0e82cab 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -33,6 +33,10 @@
 
 #define BMDMA_PAGE_SIZE 4096
 
+#define BM_MIGRATION_COMPAT_STATUS_BITS \
+(BM_STATUS_DMA_RETRY | BM_STATUS_PIO_RETRY | \
+BM_STATUS_RETRY_READ | BM_STATUS_RETRY_FLUSH)
+
 static void bmdma_start_dma(IDEDMA *dma, IDEState *s,
 BlockDriverCompletionFunc *dma_cb)
 {
diff --git a/hw/ide/pci.h b/hw/ide/pci.h
index 2428275..517711f 100644
--- a/hw/ide/pci.h
+++ b/hw/ide/pci.h
@@ -3,6 +3,13 @@
 
 #include 
 
+#define BM_STATUS_DMAING 0x01
+#define BM_STATUS_ERROR  0x02
+#define BM_STATUS_INT0x04
+
+#define BM_CMD_START 0x01
+#define BM_CMD_READ  0x08
+
 typedef struct BMDMAState {
 IDEDMA dma;
 uint8_t cmd;
-- 
1.9.3

[Qemu-devel] [PULL 18/55] q35: Enable the ioapic device to be seen by qtest.

[Stefan Hajnoczi]

From: John Snow 

Currently, the ioapic device can not be found in a qtest environment
when requesting "irq_interrupt_in ioapic" via the qtest socket.

By mirroring how the ioapic is added in i44ofx (hw/i440/pc_piix.c),
as a child of "q35," the device is able to be seen by qtest.

Signed-off-by: John Snow 
Signed-off-by: Stefan Hajnoczi 
---
 hw/i386/pc_q35.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index c39ee98..0dd7e02 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -236,7 +236,7 @@ static void pc_q35_init(MachineState *machine)
 gsi_state->i8259_irq[i] = i8259[i];
 }
 if (pci_enabled) {
-ioapic_init_gsi(gsi_state, NULL);
+ioapic_init_gsi(gsi_state, "q35");
 }
 qdev_init_nofail(icc_bridge);
 
-- 
1.9.3

[Qemu-devel] [PULL 14/55] ahci: remove duplicate PORT_IRQ_* constants

[Stefan Hajnoczi]

From: Paolo Bonzini 

These are defined twice, just use one set consistently.

Signed-off-by: Paolo Bonzini 
Signed-off-by: John Snow 
Signed-off-by: Stefan Hajnoczi 
---
 hw/ide/ahci.c |  6 +++---
 hw/ide/ahci.h | 21 -
 2 files changed, 3 insertions(+), 24 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 0ec5627..e1f27bd 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -584,7 +584,7 @@ static void ahci_write_fis_sdb(AHCIState *s, int port, 
uint32_t finished)
 s->dev[port].finished |= finished;
 *(uint32_t*)(sdb_fis + 4) = cpu_to_le32(s->dev[port].finished);
 
-ahci_trigger_irq(s, &s->dev[port], PORT_IRQ_STAT_SDBS);
+ahci_trigger_irq(s, &s->dev[port], PORT_IRQ_SDB_FIS);
 }
 
 static void ahci_write_fis_d2h(AHCIDevice *ad, uint8_t *cmd_fis)
@@ -629,7 +629,7 @@ static void ahci_write_fis_d2h(AHCIDevice *ad, uint8_t 
*cmd_fis)
 }
 
 if (d2h_fis[2] & ERR_STAT) {
-ahci_trigger_irq(ad->hba, ad, PORT_IRQ_STAT_TFES);
+ahci_trigger_irq(ad->hba, ad, PORT_IRQ_TF_ERR);
 }
 
 ahci_trigger_irq(ad->hba, ad, PORT_IRQ_D2H_REG_FIS);
@@ -1039,7 +1039,7 @@ out:
 
 if (!(s->status & DRQ_STAT)) {
 /* done with DMA */
-ahci_trigger_irq(ad->hba, ad, PORT_IRQ_STAT_DSS);
+ahci_trigger_irq(ad->hba, ad, PORT_IRQ_D2H_REG_FIS);
 }
 }
 
diff --git a/hw/ide/ahci.h b/hw/ide/ahci.h
index f418b30..1543df7 100644
--- a/hw/ide/ahci.h
+++ b/hw/ide/ahci.h
@@ -132,27 +132,6 @@
 #define PORT_CMD_ICC_PARTIAL  (0x2 << 28) /* Put i/f in partial state */
 #define PORT_CMD_ICC_SLUMBER  (0x6 << 28) /* Put i/f in slumber state */
 
-#define PORT_IRQ_STAT_DHRS(1 << 0) /* Device to Host Register FIS */
-#define PORT_IRQ_STAT_PSS (1 << 1) /* PIO Setup FIS */
-#define PORT_IRQ_STAT_DSS (1 << 2) /* DMA Setup FIS */
-#define PORT_IRQ_STAT_SDBS(1 << 3) /* Set Device Bits */
-#define PORT_IRQ_STAT_UFS (1 << 4) /* Unknown FIS */
-#define PORT_IRQ_STAT_DPS (1 << 5) /* Descriptor Processed */
-#define PORT_IRQ_STAT_PCS (1 << 6) /* Port Connect Change Status */
-#define PORT_IRQ_STAT_DMPS(1 << 7) /* Device Mechanical Presence
-  Status */
-#define PORT_IRQ_STAT_PRCS(1 << 22) /* File Ready Status */
-#define PORT_IRQ_STAT_IPMS(1 << 23) /* Incorrect Port Multiplier
-   Status */
-#define PORT_IRQ_STAT_OFS (1 << 24) /* Overflow Status */
-#define PORT_IRQ_STAT_INFS(1 << 26) /* Interface Non-Fatal Error
-   Status */
-#define PORT_IRQ_STAT_IFS (1 << 27) /* Interface Fatal Error */
-#define PORT_IRQ_STAT_HBDS(1 << 28) /* Host Bus Data Error Status */
-#define PORT_IRQ_STAT_HBFS(1 << 29) /* Host Bus Fatal Error Status */
-#define PORT_IRQ_STAT_TFES(1 << 30) /* Task File Error Status */
-#define PORT_IRQ_STAT_CPDS(1U << 31) /* Code Port Detect Status */
-
 /* ap->flags bits */
 #define AHCI_FLAG_NO_NCQ  (1 << 24)
 #define AHCI_FLAG_IGN_IRQ_IF_ERR  (1 << 25) /* ignore IRQ_IF_ERR */
-- 
1.9.3

[Qemu-devel] [PULL 11/55] ide: fold add_status callback into set_inactive

[Stefan Hajnoczi]

From: Paolo Bonzini 

It is now called only after the set_inactive callback.  Put the two together.

Signed-off-by: Paolo Bonzini 
Signed-off-by: John Snow 
Signed-off-by: Stefan Hajnoczi 
---
 hw/ide/ahci.c |  9 -
 hw/ide/atapi.c|  2 +-
 hw/ide/core.c | 12 
 hw/ide/internal.h |  6 +++---
 hw/ide/macio.c|  1 -
 hw/ide/pci.c  | 19 +++
 6 files changed, 15 insertions(+), 34 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 14677ec..0ec5627 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -1102,14 +1102,6 @@ static int ahci_dma_set_unit(IDEDMA *dma, int unit)
 return 0;
 }
 
-static int ahci_dma_add_status(IDEDMA *dma, int status)
-{
-AHCIDevice *ad = DO_UPCAST(AHCIDevice, dma, dma);
-DPRINTF(ad->port_no, "set status: %x\n", status);
-
-return 0;
-}
-
 static void ahci_async_cmd_done(IDEDMA *dma)
 {
 AHCIDevice *ad = DO_UPCAST(AHCIDevice, dma, dma);
@@ -1140,7 +1132,6 @@ static const IDEDMAOps ahci_dma_ops = {
 .prepare_buf = ahci_dma_prepare_buf,
 .rw_buf = ahci_dma_rw_buf,
 .set_unit = ahci_dma_set_unit,
-.add_status = ahci_dma_add_status,
 .async_cmd_done = ahci_async_cmd_done,
 .restart_cb = ahci_dma_restart_cb,
 };
diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index 46ed3f5..3b419b3 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -355,7 +355,7 @@ static void ide_atapi_cmd_read_dma_cb(void *opaque, int ret)
 
 eot:
 bdrv_acct_done(s->bs, &s->acct);
-ide_set_inactive(s);
+ide_set_inactive(s, false);
 }
 
 /* start a CD-CDROM read command with DMA */
diff --git a/hw/ide/core.c b/hw/ide/core.c
index aa561ae..24f24ce 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -594,11 +594,11 @@ static void ide_async_cmd_done(IDEState *s)
 }
 }
 
-void ide_set_inactive(IDEState *s)
+void ide_set_inactive(IDEState *s, bool more)
 {
 s->bus->dma->aiocb = NULL;
 if (s->bus->dma->ops->set_inactive) {
-s->bus->dma->ops->set_inactive(s->bus->dma);
+s->bus->dma->ops->set_inactive(s->bus->dma, more);
 }
 ide_async_cmd_done(s);
 }
@@ -608,7 +608,7 @@ void ide_dma_error(IDEState *s)
 ide_transfer_stop(s);
 s->error = ABRT_ERR;
 s->status = READY_STAT | ERR_STAT;
-ide_set_inactive(s);
+ide_set_inactive(s, false);
 ide_set_irq(s->bus);
 }
 
@@ -719,10 +719,7 @@ eot:
 if (s->dma_cmd == IDE_DMA_READ || s->dma_cmd == IDE_DMA_WRITE) {
 bdrv_acct_done(s->bs, &s->acct);
 }
-ide_set_inactive(s);
-if (stay_active) {
-s->bus->dma->ops->add_status(s->bus->dma, BM_STATUS_DMAING);
-}
+ide_set_inactive(s, stay_active);
 }
 
 static void ide_sector_start_dma(IDEState *s, enum ide_dma_cmd dma_cmd)
@@ -2224,7 +2221,6 @@ static const IDEDMAOps ide_dma_nop_ops = {
 .prepare_buf= ide_nop_int,
 .rw_buf = ide_nop_int,
 .set_unit   = ide_nop_int,
-.add_status = ide_nop_int,
 .restart_cb = ide_nop_restart,
 };
 
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 2fe1f0a..b35e52c 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -322,6 +322,7 @@ typedef void EndTransferFunc(IDEState *);
 typedef void DMAStartFunc(IDEDMA *, IDEState *, BlockDriverCompletionFunc *);
 typedef void DMAVoidFunc(IDEDMA *);
 typedef int DMAIntFunc(IDEDMA *, int);
+typedef void DMAStopFunc(IDEDMA *, bool);
 typedef void DMARestartFunc(void *, int, RunState);
 
 struct unreported_events {
@@ -431,8 +432,7 @@ struct IDEDMAOps {
 DMAIntFunc *prepare_buf;
 DMAIntFunc *rw_buf;
 DMAIntFunc *set_unit;
-DMAIntFunc *add_status;
-DMAVoidFunc *set_inactive;
+DMAStopFunc *set_inactive;
 DMAVoidFunc *async_cmd_done;
 DMARestartFunc *restart_cb;
 DMAVoidFunc *reset;
@@ -565,7 +565,7 @@ void ide_flush_cache(IDEState *s);
 void ide_transfer_start(IDEState *s, uint8_t *buf, int size,
 EndTransferFunc *end_transfer_func);
 void ide_transfer_stop(IDEState *s);
-void ide_set_inactive(IDEState *s);
+void ide_set_inactive(IDEState *s, bool more);
 BlockDriverAIOCB *ide_issue_trim(BlockDriverState *bs,
 int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
 BlockDriverCompletionFunc *cb, void *opaque);
diff --git a/hw/ide/macio.c b/hw/ide/macio.c
index b7cedb6..b0c0d40 100644
--- a/hw/ide/macio.c
+++ b/hw/ide/macio.c
@@ -569,7 +569,6 @@ static const IDEDMAOps dbdma_ops = {
 .prepare_buf= ide_nop_int,
 .rw_buf = ide_nop_int,
 .set_unit   = ide_nop_int,
-.add_status = ide_nop_int,
 .restart_cb = ide_nop_restart,
 };
 
diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index 1ee8c0a..73267a4 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -152,21 +152,17 @@ static int bmdma_set_unit(IDEDMA *dma, int unit)
 return 0;
 }
 
-static int bmdma_add_status(IDEDMA *dma, int status)
+static void bmdma_set_inactive(IDEDMA *dma, bool more)
 {
 BMDMAState *bm = DO_UPCAST(BMDMAState, dma, dma);
-bm->status |= statu

[Qemu-devel] [PULL 07/55] ide: simplify async_cmd_done callbacks

[Stefan Hajnoczi]

From: Paolo Bonzini 

Drop the unused return value.

Signed-off-by: Paolo Bonzini 
Signed-off-by: John Snow 
Signed-off-by: Stefan Hajnoczi 
---
 hw/ide/ahci.c | 4 +---
 hw/ide/internal.h | 2 +-
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 679357f..e494503 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -1116,7 +1116,7 @@ static int ahci_dma_add_status(IDEDMA *dma, int status)
 return 0;
 }
 
-static int ahci_async_cmd_done(IDEDMA *dma)
+static void ahci_async_cmd_done(IDEDMA *dma)
 {
 AHCIDevice *ad = DO_UPCAST(AHCIDevice, dma, dma);
 
@@ -1130,8 +1130,6 @@ static int ahci_async_cmd_done(IDEDMA *dma)
 ad->check_bh = qemu_bh_new(ahci_check_cmd_bh, ad);
 qemu_bh_schedule(ad->check_bh);
 }
-
-return 0;
 }
 
 static void ahci_irq_set(void *opaque, int n, int level)
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 34064cf..940dd45 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -434,7 +434,7 @@ struct IDEDMAOps {
 DMAIntFunc *set_unit;
 DMAIntFunc *add_status;
 DMAVoidFunc *set_inactive;
-DMAFunc *async_cmd_done;
+DMAVoidFunc *async_cmd_done;
 DMARestartFunc *restart_cb;
 DMAVoidFunc *reset;
 };
-- 
1.9.3

[Qemu-devel] [PULL 08/55] ide: simplify start_transfer callbacks

[Stefan Hajnoczi]

From: Paolo Bonzini 

Drop the unused return value and make the callback optional.

Signed-off-by: Paolo Bonzini 
Signed-off-by: John Snow 
Signed-off-by: Stefan Hajnoczi 
---
 hw/ide/ahci.c |  4 +---
 hw/ide/core.c | 10 +++---
 hw/ide/internal.h |  3 +--
 hw/ide/macio.c|  6 --
 hw/ide/pci.c  |  6 --
 5 files changed, 5 insertions(+), 24 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index e494503..adbac3d 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -991,7 +991,7 @@ out:
 }
 
 /* DMA dev <-> ram */
-static int ahci_start_transfer(IDEDMA *dma)
+static void ahci_start_transfer(IDEDMA *dma)
 {
 AHCIDevice *ad = DO_UPCAST(AHCIDevice, dma, dma);
 IDEState *s = &ad->port.ifs[0];
@@ -1041,8 +1041,6 @@ out:
 /* done with DMA */
 ahci_trigger_irq(ad->hba, ad, PORT_IRQ_STAT_DSS);
 }
-
-return 0;
 }
 
 static void ahci_start_dma(IDEDMA *dma, IDEState *s,
diff --git a/hw/ide/core.c b/hw/ide/core.c
index d52a6f6..e5ddecb 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -434,7 +434,9 @@ void ide_transfer_start(IDEState *s, uint8_t *buf, int size,
 if (!(s->status & ERR_STAT)) {
 s->status |= DRQ_STAT;
 }
-s->bus->dma->ops->start_transfer(s->bus->dma);
+if (s->bus->dma->ops->start_transfer) {
+s->bus->dma->ops->start_transfer(s->bus->dma);
+}
 }
 
 void ide_transfer_stop(IDEState *s)
@@ -2207,11 +2209,6 @@ static void ide_nop_start(IDEDMA *dma, IDEState *s,
 {
 }
 
-static int ide_nop(IDEDMA *dma)
-{
-return 0;
-}
-
 static int ide_nop_int(IDEDMA *dma, int x)
 {
 return 0;
@@ -2223,7 +2220,6 @@ static void ide_nop_restart(void *opaque, int x, RunState 
y)
 
 static const IDEDMAOps ide_dma_nop_ops = {
 .start_dma  = ide_nop_start,
-.start_transfer = ide_nop,
 .prepare_buf= ide_nop_int,
 .rw_buf = ide_nop_int,
 .set_unit   = ide_nop_int,
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 940dd45..64fbf2f 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -321,7 +321,6 @@ typedef void EndTransferFunc(IDEState *);
 
 typedef void DMAStartFunc(IDEDMA *, IDEState *, BlockDriverCompletionFunc *);
 typedef void DMAVoidFunc(IDEDMA *);
-typedef int DMAFunc(IDEDMA *);
 typedef int DMAIntFunc(IDEDMA *, int);
 typedef void DMARestartFunc(void *, int, RunState);
 
@@ -428,7 +427,7 @@ struct IDEState {
 
 struct IDEDMAOps {
 DMAStartFunc *start_dma;
-DMAFunc *start_transfer;
+DMAVoidFunc *start_transfer;
 DMAIntFunc *prepare_buf;
 DMAIntFunc *rw_buf;
 DMAIntFunc *set_unit;
diff --git a/hw/ide/macio.c b/hw/ide/macio.c
index 0e26bac..b7cedb6 100644
--- a/hw/ide/macio.c
+++ b/hw/ide/macio.c
@@ -545,11 +545,6 @@ static void macio_ide_reset(DeviceState *dev)
 ide_bus_reset(&d->bus);
 }
 
-static int ide_nop(IDEDMA *dma)
-{
-return 0;
-}
-
 static int ide_nop_int(IDEDMA *dma, int x)
 {
 return 0;
@@ -571,7 +566,6 @@ static void ide_dbdma_start(IDEDMA *dma, IDEState *s,
 
 static const IDEDMAOps dbdma_ops = {
 .start_dma  = ide_dbdma_start,
-.start_transfer = ide_nop,
 .prepare_buf= ide_nop_int,
 .rw_buf = ide_nop_int,
 .set_unit   = ide_nop_int,
diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index 9dc3cc5..1ee8c0a 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -264,11 +264,6 @@ static void bmdma_reset(IDEDMA *dma)
 bm->nsector = 0;
 }
 
-static int bmdma_start_transfer(IDEDMA *dma)
-{
-return 0;
-}
-
 static void bmdma_irq(void *opaque, int n, int level)
 {
 BMDMAState *bm = opaque;
@@ -500,7 +495,6 @@ void pci_ide_create_devs(PCIDevice *dev, DriveInfo 
**hd_table)
 
 static const struct IDEDMAOps bmdma_ops = {
 .start_dma = bmdma_start_dma,
-.start_transfer = bmdma_start_transfer,
 .prepare_buf = bmdma_prepare_buf,
 .rw_buf = bmdma_rw_buf,
 .set_unit = bmdma_set_unit,
-- 
1.9.3

[Qemu-devel] [PULL 09/55] ide: wrap start_dma callback

[Stefan Hajnoczi]

From: Paolo Bonzini 

Make it optional and prepare for the next patches.

Signed-off-by: Paolo Bonzini 
Signed-off-by: John Snow 
Signed-off-by: Stefan Hajnoczi 
---
 hw/ide/atapi.c|  6 ++
 hw/ide/core.c | 15 ---
 hw/ide/internal.h |  1 +
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index f7d2009..d13395e 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -255,8 +255,7 @@ static void ide_atapi_cmd_reply(IDEState *s, int size, int 
max_size)
 if (s->atapi_dma) {
 bdrv_acct_start(s->bs, &s->acct, size, BDRV_ACCT_READ);
 s->status = READY_STAT | SEEK_STAT | DRQ_STAT;
-s->bus->dma->ops->start_dma(s->bus->dma, s,
-   ide_atapi_cmd_read_dma_cb);
+ide_start_dma(s, ide_atapi_cmd_read_dma_cb);
 } else {
 s->status = READY_STAT | SEEK_STAT;
 ide_atapi_cmd_reply_end(s);
@@ -375,8 +374,7 @@ static void ide_atapi_cmd_read_dma(IDEState *s, int lba, 
int nb_sectors,
 
 /* XXX: check if BUSY_STAT should be set */
 s->status = READY_STAT | SEEK_STAT | DRQ_STAT | BUSY_STAT;
-s->bus->dma->ops->start_dma(s->bus->dma, s,
-   ide_atapi_cmd_read_dma_cb);
+ide_start_dma(s, ide_atapi_cmd_read_dma_cb);
 }
 
 static void ide_atapi_cmd_read(IDEState *s, int lba, int nb_sectors,
diff --git a/hw/ide/core.c b/hw/ide/core.c
index e5ddecb..aa561ae 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -745,7 +745,14 @@ static void ide_sector_start_dma(IDEState *s, enum 
ide_dma_cmd dma_cmd)
 break;
 }
 
-s->bus->dma->ops->start_dma(s->bus->dma, s, ide_dma_cb);
+ide_start_dma(s, ide_dma_cb);
+}
+
+void ide_start_dma(IDEState *s, BlockDriverCompletionFunc *cb)
+{
+if (s->bus->dma->ops->start_dma) {
+s->bus->dma->ops->start_dma(s->bus->dma, s, cb);
+}
 }
 
 static void ide_sector_write_timer_cb(void *opaque)
@@ -2204,11 +2211,6 @@ static void ide_init1(IDEBus *bus, int unit)
ide_sector_write_timer_cb, s);
 }
 
-static void ide_nop_start(IDEDMA *dma, IDEState *s,
-  BlockDriverCompletionFunc *cb)
-{
-}
-
 static int ide_nop_int(IDEDMA *dma, int x)
 {
 return 0;
@@ -2219,7 +2221,6 @@ static void ide_nop_restart(void *opaque, int x, RunState 
y)
 }
 
 static const IDEDMAOps ide_dma_nop_ops = {
-.start_dma  = ide_nop_start,
 .prepare_buf= ide_nop_int,
 .rw_buf = ide_nop_int,
 .set_unit   = ide_nop_int,
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 64fbf2f..2fe1f0a 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -532,6 +532,7 @@ void ide_bus_reset(IDEBus *bus);
 int64_t ide_get_sector(IDEState *s);
 void ide_set_sector(IDEState *s, int64_t sector_num);
 
+void ide_start_dma(IDEState *s, BlockDriverCompletionFunc *cb);
 void ide_dma_error(IDEState *s);
 
 void ide_atapi_cmd_ok(IDEState *s);
-- 
1.9.3

[Qemu-devel] [PULL 16/55] ide: make all commands go through cmd_done

[Stefan Hajnoczi]

From: Paolo Bonzini 

AHCI has code to fill in the D2H FIS trigger the IRQ all over the place.
Centralize this in a single cmd_done callback by generalizing the existing
async_cmd_done callback.

Signed-off-by: Paolo Bonzini 
Signed-off-by: John Snow 
Signed-off-by: Stefan Hajnoczi 
---
 hw/ide/ahci.c | 16 +++-
 hw/ide/atapi.c|  2 +-
 hw/ide/core.c | 20 +++-
 hw/ide/internal.h |  2 +-
 4 files changed, 16 insertions(+), 24 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index e1f27bd..b40ec06 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -969,11 +969,6 @@ static int handle_cmd(AHCIState *s, int port, int slot)
 
 /* We're ready to process the command in FIS byte 2. */
 ide_exec_cmd(&s->dev[port].port, cmd_fis[2]);
-
-if ((s->dev[port].port.ifs[0].status & 
(READY_STAT|DRQ_STAT|BUSY_STAT)) ==
-READY_STAT) {
-ahci_write_fis_d2h(&s->dev[port], cmd_fis);
-}
 }
 
 out:
@@ -1036,11 +1031,6 @@ out:
 }
 
 s->end_transfer_func(s);
-
-if (!(s->status & DRQ_STAT)) {
-/* done with DMA */
-ahci_trigger_irq(ad->hba, ad, PORT_IRQ_D2H_REG_FIS);
-}
 }
 
 static void ahci_start_dma(IDEDMA *dma, IDEState *s,
@@ -1102,11 +1092,11 @@ static int ahci_dma_set_unit(IDEDMA *dma, int unit)
 return 0;
 }
 
-static void ahci_async_cmd_done(IDEDMA *dma)
+static void ahci_cmd_done(IDEDMA *dma)
 {
 AHCIDevice *ad = DO_UPCAST(AHCIDevice, dma, dma);
 
-DPRINTF(ad->port_no, "async cmd done\n");
+DPRINTF(ad->port_no, "cmd done\n");
 
 /* update d2h status */
 ahci_write_fis_d2h(ad, NULL);
@@ -1132,7 +1122,7 @@ static const IDEDMAOps ahci_dma_ops = {
 .prepare_buf = ahci_dma_prepare_buf,
 .rw_buf = ahci_dma_rw_buf,
 .set_unit = ahci_dma_set_unit,
-.async_cmd_done = ahci_async_cmd_done,
+.cmd_done = ahci_cmd_done,
 .restart_cb = ahci_dma_restart_cb,
 };
 
diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index 3b419b3..3d92b52 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -174,9 +174,9 @@ void ide_atapi_cmd_reply_end(IDEState *s)
 #endif
 if (s->packet_transfer_size <= 0) {
 /* end of transfer */
-ide_transfer_stop(s);
 s->status = READY_STAT | SEEK_STAT;
 s->nsector = (s->nsector & ~7) | ATAPI_INT_REASON_IO | 
ATAPI_INT_REASON_CD;
+ide_transfer_stop(s);
 ide_set_irq(s->bus);
 #ifdef DEBUG_IDE_ATAPI
 printf("status=0x%x\n", s->status);
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 91a17e6..bdb0a80 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -440,12 +440,20 @@ void ide_transfer_start(IDEState *s, uint8_t *buf, int 
size,
 }
 }
 
+static void ide_cmd_done(IDEState *s)
+{
+if (s->bus->dma->ops->cmd_done) {
+s->bus->dma->ops->cmd_done(s->bus->dma);
+}
+}
+
 void ide_transfer_stop(IDEState *s)
 {
 s->end_transfer_func = ide_transfer_stop;
 s->data_ptr = s->io_buffer;
 s->data_end = s->io_buffer;
 s->status &= ~DRQ_STAT;
+ide_cmd_done(s);
 }
 
 int64_t ide_get_sector(IDEState *s)
@@ -588,20 +596,13 @@ static void dma_buf_commit(IDEState *s)
 qemu_sglist_destroy(&s->sg);
 }
 
-static void ide_async_cmd_done(IDEState *s)
-{
-if (s->bus->dma->ops->async_cmd_done) {
-s->bus->dma->ops->async_cmd_done(s->bus->dma);
-}
-}
-
 void ide_set_inactive(IDEState *s, bool more)
 {
 s->bus->dma->aiocb = NULL;
 if (s->bus->dma->ops->set_inactive) {
 s->bus->dma->ops->set_inactive(s->bus->dma, more);
 }
-ide_async_cmd_done(s);
+ide_cmd_done(s);
 }
 
 void ide_dma_error(IDEState *s)
@@ -849,7 +850,7 @@ static void ide_flush_cb(void *opaque, int ret)
 
 bdrv_acct_done(s->bs, &s->acct);
 s->status = READY_STAT | SEEK_STAT;
-ide_async_cmd_done(s);
+ide_cmd_done(s);
 ide_set_irq(s->bus);
 }
 
@@ -1773,6 +1774,7 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
 s->status |= SEEK_STAT;
 }
 
+ide_cmd_done(s);
 ide_set_irq(s->bus);
 }
 }
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index b919e96..5c19f79 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -433,7 +433,7 @@ struct IDEDMAOps {
 DMAIntFunc *rw_buf;
 DMAIntFunc *set_unit;
 DMAStopFunc *set_inactive;
-DMAVoidFunc *async_cmd_done;
+DMAVoidFunc *cmd_done;
 DMARestartFunc *restart_cb;
 DMAVoidFunc *reset;
 };
-- 
1.9.3

[Qemu-devel] [PULL 13/55] ide: move retry constants out of BM_STATUS_* namespace

[Stefan Hajnoczi]

From: Paolo Bonzini 

Signed-off-by: Paolo Bonzini 
Signed-off-by: John Snow 
Signed-off-by: Stefan Hajnoczi 
---
 hw/ide/core.c | 20 ++--
 hw/ide/internal.h | 12 ++--
 hw/ide/pci.c  | 14 +++---
 3 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 24f24ce..00fe043 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -523,8 +523,8 @@ static void ide_sector_read_cb(void *opaque, int ret)
 
 bdrv_acct_done(s->bs, &s->acct);
 if (ret != 0) {
-if (ide_handle_rw_error(s, -ret, BM_STATUS_PIO_RETRY |
-BM_STATUS_RETRY_READ)) {
+if (ide_handle_rw_error(s, -ret, IDE_RETRY_PIO |
+IDE_RETRY_READ)) {
 return;
 }
 }
@@ -614,14 +614,14 @@ void ide_dma_error(IDEState *s)
 
 static int ide_handle_rw_error(IDEState *s, int error, int op)
 {
-bool is_read = (op & BM_STATUS_RETRY_READ) != 0;
+bool is_read = (op & IDE_RETRY_READ) != 0;
 BlockErrorAction action = bdrv_get_error_action(s->bs, is_read, error);
 
 if (action == BLOCK_ERROR_ACTION_STOP) {
 s->bus->dma->ops->set_unit(s->bus->dma, s->unit);
 s->bus->error_status = op;
 } else if (action == BLOCK_ERROR_ACTION_REPORT) {
-if (op & BM_STATUS_DMA_RETRY) {
+if (op & IDE_RETRY_DMA) {
 dma_buf_commit(s);
 ide_dma_error(s);
 } else {
@@ -640,12 +640,12 @@ void ide_dma_cb(void *opaque, int ret)
 bool stay_active = false;
 
 if (ret < 0) {
-int op = BM_STATUS_DMA_RETRY;
+int op = IDE_RETRY_DMA;
 
 if (s->dma_cmd == IDE_DMA_READ)
-op |= BM_STATUS_RETRY_READ;
+op |= IDE_RETRY_READ;
 else if (s->dma_cmd == IDE_DMA_TRIM)
-op |= BM_STATUS_RETRY_TRIM;
+op |= IDE_RETRY_TRIM;
 
 if (ide_handle_rw_error(s, -ret, op)) {
 return;
@@ -769,7 +769,7 @@ static void ide_sector_write_cb(void *opaque, int ret)
 s->status &= ~BUSY_STAT;
 
 if (ret != 0) {
-if (ide_handle_rw_error(s, -ret, BM_STATUS_PIO_RETRY)) {
+if (ide_handle_rw_error(s, -ret, IDE_RETRY_PIO)) {
 return;
 }
 }
@@ -843,7 +843,7 @@ static void ide_flush_cb(void *opaque, int ret)
 
 if (ret < 0) {
 /* XXX: What sector number to set here? */
-if (ide_handle_rw_error(s, -ret, BM_STATUS_RETRY_FLUSH)) {
+if (ide_handle_rw_error(s, -ret, IDE_RETRY_FLUSH)) {
 return;
 }
 }
@@ -2338,7 +2338,7 @@ static bool ide_drive_pio_state_needed(void *opaque)
 IDEState *s = opaque;
 
 return ((s->status & DRQ_STAT) != 0)
-|| (s->bus->error_status & BM_STATUS_PIO_RETRY);
+|| (s->bus->error_status & IDE_RETRY_PIO);
 }
 
 static bool ide_tray_state_needed(void *opaque)
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index f369b0b..b919e96 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -484,12 +484,12 @@ struct IDEDevice {
 uint64_t wwn;
 };
 
-/* FIXME These are not status register bits */
-#define BM_STATUS_DMA_RETRY  0x08
-#define BM_STATUS_PIO_RETRY  0x10
-#define BM_STATUS_RETRY_READ  0x20
-#define BM_STATUS_RETRY_FLUSH 0x40
-#define BM_STATUS_RETRY_TRIM 0x80
+/* These are used for the error_status field of IDEBus */
+#define IDE_RETRY_DMA  0x08
+#define IDE_RETRY_PIO  0x10
+#define IDE_RETRY_READ  0x20
+#define IDE_RETRY_FLUSH 0x40
+#define IDE_RETRY_TRIM 0x80
 
 static inline IDEState *idebus_active_if(IDEBus *bus)
 {
diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index 0e82cab..2397f35 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -34,8 +34,8 @@
 #define BMDMA_PAGE_SIZE 4096
 
 #define BM_MIGRATION_COMPAT_STATUS_BITS \
-(BM_STATUS_DMA_RETRY | BM_STATUS_PIO_RETRY | \
-BM_STATUS_RETRY_READ | BM_STATUS_RETRY_FLUSH)
+(IDE_RETRY_DMA | IDE_RETRY_PIO | \
+IDE_RETRY_READ | IDE_RETRY_FLUSH)
 
 static void bmdma_start_dma(IDEDMA *dma, IDEState *s,
 BlockDriverCompletionFunc *dma_cb)
@@ -198,7 +198,7 @@ static void bmdma_restart_bh(void *opaque)
 return;
 }
 
-is_read = (bus->error_status & BM_STATUS_RETRY_READ) != 0;
+is_read = (bus->error_status & IDE_RETRY_READ) != 0;
 
 /* The error status must be cleared before resubmitting the request: The
  * request may fail again, and this case can only be distinguished if the
@@ -206,19 +206,19 @@ static void bmdma_restart_bh(void *opaque)
 error_status = bus->error_status;
 bus->error_status = 0;
 
-if (error_status & BM_STATUS_DMA_RETRY) {
-if (error_status & BM_STATUS_RETRY_TRIM) {
+if (error_status & IDE_RETRY_DMA) {
+if (error_status & IDE_RETRY_TRIM) {
 bmdma_restart_dma(bm, IDE_DMA_TRIM);
 } else {
 bmdma_restart_dma(bm, is_read ? IDE_DMA_READ : IDE_DMA_WRITE);
 }
-} else if (error_status & BM_STATUS_PIO_RETRY) {
+} else if (

[Qemu-devel] [PULL 05/55] ide: simplify reset callbacks

[Stefan Hajnoczi]

From: Paolo Bonzini 

Drop the unused return value and make the callback optional.

Signed-off-by: Paolo Bonzini 
Signed-off-by: John Snow 
Signed-off-by: Stefan Hajnoczi 
---
 hw/ide/ahci.c | 6 --
 hw/ide/core.c | 5 +++--
 hw/ide/internal.h | 3 ++-
 hw/ide/macio.c| 1 -
 hw/ide/pci.c  | 4 +---
 5 files changed, 6 insertions(+), 13 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 604152a..273712f 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -1147,11 +1147,6 @@ static void ahci_dma_restart_cb(void *opaque, int 
running, RunState state)
 {
 }
 
-static int ahci_dma_reset(IDEDMA *dma)
-{
-return 0;
-}
-
 static const IDEDMAOps ahci_dma_ops = {
 .start_dma = ahci_start_dma,
 .start_transfer = ahci_start_transfer,
@@ -1162,7 +1157,6 @@ static const IDEDMAOps ahci_dma_ops = {
 .set_inactive = ahci_dma_set_inactive,
 .async_cmd_done = ahci_async_cmd_done,
 .restart_cb = ahci_dma_restart_cb,
-.reset = ahci_dma_reset,
 };
 
 void ahci_init(AHCIState *s, DeviceState *qdev, AddressSpace *as, int ports)
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 79985f9..4183a3a 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -2088,7 +2088,9 @@ void ide_bus_reset(IDEBus *bus)
 }
 
 /* reset dma provider too */
-bus->dma->ops->reset(bus->dma);
+if (bus->dma->ops->reset) {
+bus->dma->ops->reset(bus->dma);
+}
 }
 
 static bool ide_cd_is_tray_open(void *opaque)
@@ -2226,7 +2228,6 @@ static const IDEDMAOps ide_dma_nop_ops = {
 .add_status = ide_nop_int,
 .set_inactive   = ide_nop,
 .restart_cb = ide_nop_restart,
-.reset  = ide_nop,
 };
 
 static IDEDMA ide_dma_nop = {
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 0567a52..6cca46f 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -320,6 +320,7 @@ typedef enum { IDE_HD, IDE_CD, IDE_CFATA } IDEDriveKind;
 typedef void EndTransferFunc(IDEState *);
 
 typedef void DMAStartFunc(IDEDMA *, IDEState *, BlockDriverCompletionFunc *);
+typedef void DMAVoidFunc(IDEDMA *);
 typedef int DMAFunc(IDEDMA *);
 typedef int DMAIntFunc(IDEDMA *, int);
 typedef void DMARestartFunc(void *, int, RunState);
@@ -435,7 +436,7 @@ struct IDEDMAOps {
 DMAFunc *set_inactive;
 DMAFunc *async_cmd_done;
 DMARestartFunc *restart_cb;
-DMAFunc *reset;
+DMAVoidFunc *reset;
 };
 
 struct IDEDMA {
diff --git a/hw/ide/macio.c b/hw/ide/macio.c
index c14a1dd..ca39e3f 100644
--- a/hw/ide/macio.c
+++ b/hw/ide/macio.c
@@ -578,7 +578,6 @@ static const IDEDMAOps dbdma_ops = {
 .add_status = ide_nop_int,
 .set_inactive   = ide_nop,
 .restart_cb = ide_nop_restart,
-.reset  = ide_nop,
 };
 
 static void macio_ide_realizefn(DeviceState *dev, Error **errp)
diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index 6257a21..c24496a 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -247,7 +247,7 @@ static void bmdma_cancel(BMDMAState *bm)
 }
 }
 
-static int bmdma_reset(IDEDMA *dma)
+static void bmdma_reset(IDEDMA *dma)
 {
 BMDMAState *bm = DO_UPCAST(BMDMAState, dma, dma);
 
@@ -264,8 +264,6 @@ static int bmdma_reset(IDEDMA *dma)
 bm->cur_prd_len = 0;
 bm->sector_num = 0;
 bm->nsector = 0;
-
-return 0;
 }
 
 static int bmdma_start_transfer(IDEDMA *dma)
-- 
1.9.3

[Qemu-devel] [PULL 02/55] libqtest: add QTEST_LOG for debugging qtest testcases

[Stefan Hajnoczi]

From: Paolo Bonzini 

Signed-off-by: Paolo Bonzini 
Signed-off-by: John Snow 
Signed-off-by: Stefan Hajnoczi 
---
 tests/libqtest.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/tests/libqtest.c b/tests/libqtest.c
index 98e8f4b..4a75cd3 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -167,11 +167,12 @@ QTestState *qtest_init(const char *extra_args)
 if (s->qemu_pid == 0) {
 command = g_strdup_printf("exec %s "
   "-qtest unix:%s,nowait "
-  "-qtest-log /dev/null "
+  "-qtest-log %s "
   "-qmp unix:%s,nowait "
   "-machine accel=qtest "
   "-display none "
   "%s", qemu_binary, socket_path,
+  getenv("QTEST_LOG") ? "/dev/fd/2" : 
"/dev/null",
   qmp_socket_path,
   extra_args ?: "");
 execlp("/bin/sh", "sh", "-c", command, NULL);
@@ -358,6 +359,7 @@ static void qmp_response(JSONMessageParser *parser, QList 
*tokens)
 QDict *qtest_qmp_receive(QTestState *s)
 {
 QMPResponseParser qmp;
+bool log = getenv("QTEST_LOG") != NULL;
 
 qmp.response = NULL;
 json_message_parser_init(&qmp.parser, qmp_response);
@@ -375,6 +377,9 @@ QDict *qtest_qmp_receive(QTestState *s)
 exit(1);
 }
 
+if (log) {
+len = write(2, &c, 1);
+}
 json_message_parser_feed(&qmp.parser, &c, 1);
 }
 json_message_parser_destroy(&qmp.parser);
-- 
1.9.3

[Qemu-devel] [PULL 06/55] ide: simplify set_inactive callbacks

[Stefan Hajnoczi]

From: Paolo Bonzini 

Drop the unused return value and make the callback optional.

Signed-off-by: Paolo Bonzini 
Signed-off-by: John Snow 
Signed-off-by: Stefan Hajnoczi 
---
 hw/ide/ahci.c | 6 --
 hw/ide/core.c | 5 +++--
 hw/ide/internal.h | 2 +-
 hw/ide/macio.c| 1 -
 hw/ide/pci.c  | 4 +---
 5 files changed, 5 insertions(+), 13 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 273712f..679357f 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -1116,11 +1116,6 @@ static int ahci_dma_add_status(IDEDMA *dma, int status)
 return 0;
 }
 
-static int ahci_dma_set_inactive(IDEDMA *dma)
-{
-return 0;
-}
-
 static int ahci_async_cmd_done(IDEDMA *dma)
 {
 AHCIDevice *ad = DO_UPCAST(AHCIDevice, dma, dma);
@@ -1154,7 +1149,6 @@ static const IDEDMAOps ahci_dma_ops = {
 .rw_buf = ahci_dma_rw_buf,
 .set_unit = ahci_dma_set_unit,
 .add_status = ahci_dma_add_status,
-.set_inactive = ahci_dma_set_inactive,
 .async_cmd_done = ahci_async_cmd_done,
 .restart_cb = ahci_dma_restart_cb,
 };
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 4183a3a..d52a6f6 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -595,7 +595,9 @@ static void ide_async_cmd_done(IDEState *s)
 void ide_set_inactive(IDEState *s)
 {
 s->bus->dma->aiocb = NULL;
-s->bus->dma->ops->set_inactive(s->bus->dma);
+if (s->bus->dma->ops->set_inactive) {
+s->bus->dma->ops->set_inactive(s->bus->dma);
+}
 ide_async_cmd_done(s);
 }
 
@@ -2226,7 +2228,6 @@ static const IDEDMAOps ide_dma_nop_ops = {
 .rw_buf = ide_nop_int,
 .set_unit   = ide_nop_int,
 .add_status = ide_nop_int,
-.set_inactive   = ide_nop,
 .restart_cb = ide_nop_restart,
 };
 
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 6cca46f..34064cf 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -433,7 +433,7 @@ struct IDEDMAOps {
 DMAIntFunc *rw_buf;
 DMAIntFunc *set_unit;
 DMAIntFunc *add_status;
-DMAFunc *set_inactive;
+DMAVoidFunc *set_inactive;
 DMAFunc *async_cmd_done;
 DMARestartFunc *restart_cb;
 DMAVoidFunc *reset;
diff --git a/hw/ide/macio.c b/hw/ide/macio.c
index ca39e3f..0e26bac 100644
--- a/hw/ide/macio.c
+++ b/hw/ide/macio.c
@@ -576,7 +576,6 @@ static const IDEDMAOps dbdma_ops = {
 .rw_buf = ide_nop_int,
 .set_unit   = ide_nop_int,
 .add_status = ide_nop_int,
-.set_inactive   = ide_nop,
 .restart_cb = ide_nop_restart,
 };
 
diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index c24496a..9dc3cc5 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -160,15 +160,13 @@ static int bmdma_add_status(IDEDMA *dma, int status)
 return 0;
 }
 
-static int bmdma_set_inactive(IDEDMA *dma)
+static void bmdma_set_inactive(IDEDMA *dma)
 {
 BMDMAState *bm = DO_UPCAST(BMDMAState, dma, dma);
 
 bm->status &= ~BM_STATUS_DMAING;
 bm->dma_cb = NULL;
 bm->unit = -1;
-
-return 0;
 }
 
 static void bmdma_restart_dma(BMDMAState *bm, enum ide_dma_cmd dma_cmd)
-- 
1.9.3

[Qemu-devel] [PULL 04/55] ide: stash aiocb for flushes

[Stefan Hajnoczi]

From: Paolo Bonzini 

This ensures that operations are completed after a reset

Signed-off-by: Paolo Bonzini 
Signed-off-by: John Snow 
Signed-off-by: Stefan Hajnoczi 
---
 hw/ide/core.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index db191a6..79985f9 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -831,6 +831,8 @@ static void ide_flush_cb(void *opaque, int ret)
 {
 IDEState *s = opaque;
 
+s->pio_aiocb = NULL;
+
 if (ret < 0) {
 /* XXX: What sector number to set here? */
 if (ide_handle_rw_error(s, -ret, BM_STATUS_RETRY_FLUSH)) {
@@ -853,7 +855,7 @@ void ide_flush_cache(IDEState *s)
 
 s->status |= BUSY_STAT;
 bdrv_acct_start(s->bs, &s->acct, 0, BDRV_ACCT_FLUSH);
-bdrv_aio_flush(s->bs, ide_flush_cb, s);
+s->pio_aiocb = bdrv_aio_flush(s->bs, ide_flush_cb, s);
 }
 
 static void ide_cfata_metadata_inquiry(IDEState *s)
-- 
1.9.3

[Qemu-devel] [PULL 03/55] ide-test: add test for werror=stop

[Stefan Hajnoczi]

From: Paolo Bonzini 

Signed-off-by: Paolo Bonzini 
Signed-off-by: John Snow 
Signed-off-by: Stefan Hajnoczi 
---
 tests/ide-test.c | 81 
 1 file changed, 81 insertions(+)

diff --git a/tests/ide-test.c b/tests/ide-test.c
index 4a0d97f..151ef30 100644
--- a/tests/ide-test.c
+++ b/tests/ide-test.c
@@ -106,6 +106,7 @@ static QPCIBus *pcibus = NULL;
 static QGuestAllocator *guest_malloc;
 
 static char tmp_path[] = "/tmp/qtest.XX";
+static char debug_path[] = "/tmp/qtest-blkdebug.XX";
 
 static void ide_test_start(const char *cmdline_fmt, ...)
 {
@@ -489,6 +490,78 @@ static void test_flush(void)
 ide_test_quit();
 }
 
+static void prepare_blkdebug_script(const char *debug_fn, const char *event)
+{
+FILE *debug_file = fopen(debug_fn, "w");
+int ret;
+
+fprintf(debug_file, "[inject-error]\n");
+fprintf(debug_file, "event = \"%s\"\n", event);
+fprintf(debug_file, "errno = \"5\"\n");
+fprintf(debug_file, "state = \"1\"\n");
+fprintf(debug_file, "immediately = \"off\"\n");
+fprintf(debug_file, "once = \"on\"\n");
+
+fprintf(debug_file, "[set-state]\n");
+fprintf(debug_file, "event = \"%s\"\n", event);
+fprintf(debug_file, "new_state = \"2\"\n");
+fflush(debug_file);
+g_assert(!ferror(debug_file));
+
+ret = fclose(debug_file);
+g_assert(ret == 0);
+}
+
+static void test_retry_flush(void)
+{
+uint8_t data;
+const char *s;
+QDict *response;
+
+prepare_blkdebug_script(debug_path, "flush_to_disk");
+
+ide_test_start(
+"-vnc none "
+"-drive 
file=blkdebug:%s:%s,if=ide,cache=writeback,rerror=stop,werror=stop",
+debug_path, tmp_path);
+
+/* FLUSH CACHE command on device 0*/
+outb(IDE_BASE + reg_device, 0);
+outb(IDE_BASE + reg_command, CMD_FLUSH_CACHE);
+
+/* Check status while request is in flight*/
+data = inb(IDE_BASE + reg_status);
+assert_bit_set(data, BSY | DRDY);
+assert_bit_clear(data, DF | ERR | DRQ);
+
+for (;; response = NULL) {
+response = qmp_receive();
+if ((qdict_haskey(response, "event")) &&
+(strcmp(qdict_get_str(response, "event"), "STOP") == 0)) {
+QDECREF(response);
+break;
+}
+QDECREF(response);
+}
+
+/* Complete the command */
+s = "{'execute':'cont' }";
+qmp_discard_response(s);
+
+/* Check registers */
+data = inb(IDE_BASE + reg_device);
+g_assert_cmpint(data & DEV, ==, 0);
+
+do {
+data = inb(IDE_BASE + reg_status);
+} while (data & BSY);
+
+assert_bit_set(data, DRDY);
+assert_bit_clear(data, BSY | DF | ERR | DRQ);
+
+ide_test_quit();
+}
+
 int main(int argc, char **argv)
 {
 const char *arch = qtest_get_arch();
@@ -501,6 +574,11 @@ int main(int argc, char **argv)
 return 0;
 }
 
+/* Create temporary blkdebug instructions */
+fd = mkstemp(debug_path);
+g_assert(fd >= 0);
+close(fd);
+
 /* Create a temporary raw image */
 fd = mkstemp(tmp_path);
 g_assert(fd >= 0);
@@ -522,10 +600,13 @@ int main(int argc, char **argv)
 
 qtest_add_func("/ide/flush", test_flush);
 
+qtest_add_func("/ide/retry/flush", test_retry_flush);
+
 ret = g_test_run();
 
 /* Cleanup */
 unlink(tmp_path);
+unlink(debug_path);
 
 return ret;
 }
-- 
1.9.3

[Qemu-devel] [PULL 01/55] blkdebug: report errors on flush too

[Stefan Hajnoczi]

From: Paolo Bonzini 

Signed-off-by: Paolo Bonzini 
Signed-off-by: John Snow 
Signed-off-by: Stefan Hajnoczi 
---
 block/blkdebug.c | 20 
 1 file changed, 20 insertions(+)

diff --git a/block/blkdebug.c b/block/blkdebug.c
index f51407d..1586ed9 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -522,6 +522,25 @@ static BlockDriverAIOCB 
*blkdebug_aio_writev(BlockDriverState *bs,
 return bdrv_aio_writev(bs->file, sector_num, qiov, nb_sectors, cb, opaque);
 }
 
+static BlockDriverAIOCB *blkdebug_aio_flush(BlockDriverState *bs,
+BlockDriverCompletionFunc *cb, void *opaque)
+{
+BDRVBlkdebugState *s = bs->opaque;
+BlkdebugRule *rule = NULL;
+
+QSIMPLEQ_FOREACH(rule, &s->active_rules, active_next) {
+if (rule->options.inject.sector == -1) {
+break;
+}
+}
+
+if (rule && rule->options.inject.error) {
+return inject_error(bs, cb, opaque, rule);
+}
+
+return bdrv_aio_flush(bs->file, cb, opaque);
+}
+
 
 static void blkdebug_close(BlockDriverState *bs)
 {
@@ -699,6 +718,7 @@ static BlockDriver bdrv_blkdebug = {
 
 .bdrv_aio_readv = blkdebug_aio_readv,
 .bdrv_aio_writev= blkdebug_aio_writev,
+.bdrv_aio_flush = blkdebug_aio_flush,
 
 .bdrv_debug_event   = blkdebug_debug_event,
 .bdrv_debug_breakpoint  = blkdebug_debug_breakpoint,
-- 
1.9.3

[Qemu-devel] [PULL 00/55] Block patches

[Stefan Hajnoczi]

The following changes since commit 5c6b3c50cca2106e5fbcbc6efa94c2f8b9d29fd8:

  Merge remote-tracking branch 'remotes/stefanha/tags/tracing-pull-request' 
into staging (2014-08-15 16:37:17 +0100)

are available in the git repository at:


  git://github.com/stefanha/qemu.git tags/block-pull-request

for you to fetch changes up to 39ba3bf69c4ef4d8a8b683ee7282efd25b3f01ff:

  qcow2: fix new_blocks double-free in alloc_refcount_block() (2014-08-15 
18:03:26 +0100)


Block pull request


Cornelia Huck (3):
  dataplane: print why starting failed
  dataplane: fail notifier setting gracefully
  dataplane: stop trying on notifier error

Denis V. Lunev (4):
  parallels: extend parallels format header with actual data values
  parallels: replace tabs with spaces in block/parallels.c
  parallels: split check for parallels format in parallels_open
  parallels: 2TB+ parallels images support

Gonglei (2):
  qemu-char: using qemu_set_nonblock() instead of fcntl(O_NONBLOCK)
  channel-posix: using qemu_set_nonblock() instead of fcntl(O_NONBLOCK)

John Snow (7):
  q35: Enable the ioapic device to be seen by qtest.
  qtest: Adding qtest_memset and qmemset.
  libqos: Correct memory leak
  libqtest: Correct small memory leak.
  libqos: Fixes a small memory leak.
  libqos: allow qpci_iomap to return BAR mapping size
  qtest/ide: Fix small memory leak

Kevin Wolf (1):
  ide: Fix segfault when flushing a device that doesn't exist

Marc Marí (4):
  libqtest: add QTEST_LOG for debugging qtest testcases
  libqos: Correct mask to align size to PAGE_SIZE in malloc-pc
  libqos: Change free function called in malloc
  virtio-blk: Correct bug in support for flexible descriptor layout

Maria Kustova (9):
  docs: Specification for the image fuzzer
  image-fuzzer: Tool for fuzz tests execution
  image-fuzzer: Fuzzing functions for qcow2 images
  image-fuzzer: Generator of fuzzed qcow2 images
  image-fuzzer: Public API for image-fuzzer/runner/runner.py
  docs: Expand the list of supported image elements with L1/L2 tables
  image-fuzzer: Add fuzzing functions for L1/L2 table entries
  image-fuzzer: Add generators of L1/L2 tables
  image-fuzzer: Reduce number of generator functions in __init__

Mark Cave-Ayland (5):
  cmd646: add constants for CNTRL register access
  cmd646: synchronise DMA interrupt status with UDMA interrupt status
  cmd646: switch cmd646_update_irq() to accept PCIDevice instead of 
PCIIDEState
  cmd646: allow MRDMODE interrupt status bits clearing from PCI config space
  cmd646: synchronise UDMA interrupt status with DMA interrupt status

Michael Tokarev (1):
  ide: only constrain read/write requests to drive size, not other types

Paolo Bonzini (17):
  blkdebug: report errors on flush too
  libqtest: add QTEST_LOG for debugging qtest testcases
  ide-test: add test for werror=stop
  ide: stash aiocb for flushes
  ide: simplify reset callbacks
  ide: simplify set_inactive callbacks
  ide: simplify async_cmd_done callbacks
  ide: simplify start_transfer callbacks
  ide: wrap start_dma callback
  ide: remove wrong setting of BM_STATUS_INT
  ide: fold add_status callback into set_inactive
  ide: move BM_STATUS bits to pci.[ch]
  ide: move retry constants out of BM_STATUS_* namespace
  ahci: remove duplicate PORT_IRQ_* constants
  ide: stop PIO transfer on errors
  ide: make all commands go through cmd_done
  ahci: construct PIO Setup FIS for PIO commands

Peter Lieven (1):
  qemu-options: add missing -drive discard option to cmdline help

Stefan Hajnoczi (1):
  qcow2: fix new_blocks double-free in alloc_refcount_block()

 block/blkdebug.c |  20 ++
 block/parallels.c|  52 ++--
 block/qcow2-refcount.c   |   1 +
 docs/image-fuzzer.txt| 238 ++
 hw/block/dataplane/virtio-blk.c  |  39 ++-
 hw/block/virtio-blk.c|  14 +-
 hw/i386/pc_q35.c |   2 +-
 hw/ide/ahci.c| 115 +
 hw/ide/ahci.h|  21 --
 hw/ide/atapi.c   |  11 +-
 hw/ide/cmd646.c  |  94 ++-
 hw/ide/core.c| 101 
 hw/ide/internal.h|  38 ++-
 hw/ide/macio.c   |   9 -
 hw/ide/pci.c |  45 ++--
 hw/ide/pci.h |   7 +
 qemu-char.c  |   4 +-
 qemu-options.hx  |   2 +-
 qga/channel-posix.c  |   2 +-
 tests/ide-test.c |  99 +++-
 tests/image-fuzzer/qcow2/__init__.py |   1 +
 tests/image-fuzzer/qcow2/fuzz.py | 355 +