Re: [Qemu-devel] [PATCH] ahci.c: mask unused flags when reading size PRDT DBC

2014-06-30 Thread Alexander Graf


On 29.06.14 20:21, reza.jel...@gmail.com wrote:

This requires a custom ovmf image with sata controller for testing [0]

[0]: http://reza.jelveh.me/assets/OVMF.fd.bz2



I guess this is supposed to be a cover letter? A few rules for cover 
letters:


  1) Cover letters only make sense for patch sets. This is a single 
patch, so you don't need one
  2) Because they come with patch sets and you number patch sets, cover 
letters are [PATCH 0/n].
  3) Usually cover letters contain git statistics. Try git format-patch 
--cover-letter. It will give you a nice template.


The usual way to add the comment you have here to a patch you're trying 
to submit it is to put volatile information (things that shouldn't end 
up in the git log) behind a --- line in the commit message. Everything 
after that line gets ignored by git when the patch gets applied.



Alex




Re: [Qemu-devel] [PATCH] ahci.c: mask unused flags when reading size PRDT DBC

2014-06-30 Thread Alexander Graf


On 29.06.14 20:21, reza.jel...@gmail.com wrote:

From: Reza Jelveh reza.jel...@tuhh.de


This is a hint that your git configuration isn't fully correct. If the 
email address git thinks you want to use is the same as the From: email 
address, it will not print this line. I suppose the problem is with the 
difference in @gmail.com and @tuhh.de?



The data byte count(DBC) read from the description information is defined for


bits


21:00.


Bits


30:22 are reserved and


bit


31 is the Interrupt on Completion (I) flag.

Interrupt is not implemented in QEMU.


They are implemented in QEMU, but incorrectly. We trigger a completion 
interrupt after every transaction, not every time we see the I bit in 
the PRDT.



tbl_entry_size is a signed integer and
improperly reading the DBC leads to a negative offset that causes sglist
allocation to fail.

Signed-off-by: Reza Jelveh reza.jel...@tuhh.de
---
  hw/ide/ahci.c | 12 +---
  hw/ide/ahci.h |  2 ++
  2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 9bae22e..93aa981 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -639,6 +639,12 @@ static void ahci_write_fis_d2h(AHCIDevice *ad, uint8_t 
*cmd_fis)
  }
  }
  
+static int prdt_tbl_entry_size(const AHCI_SG tbl)


The argument should still be a pointer.


+{
+return (le32_to_cpu(tbl.flags_size)  AHCI_PRDT_SIZE_MASK) + 1;
+}
+
+


There is still one whitespace line too much :).


Alex


  static int ahci_populate_sglist(AHCIDevice *ad, QEMUSGList *sglist, int 
offset)
  {
  AHCICmdHdr *cmd = ad-cur_cmd;
@@ -681,7 +687,7 @@ static int ahci_populate_sglist(AHCIDevice *ad, QEMUSGList 
*sglist, int offset)
  sum = 0;
  for (i = 0; i  sglist_alloc_hint; i++) {
  /* flags_size is zero-based */
-tbl_entry_size = (le32_to_cpu(tbl[i].flags_size) + 1);
+tbl_entry_size = prdt_tbl_entry_size(tbl[i]);
  if (offset = (sum + tbl_entry_size)) {
  off_idx = i;
  off_pos = offset - sum;
@@ -700,12 +706,12 @@ static int ahci_populate_sglist(AHCIDevice *ad, 
QEMUSGList *sglist, int offset)
  qemu_sglist_init(sglist, qbus-parent, (sglist_alloc_hint - off_idx),
   ad-hba-as);
  qemu_sglist_add(sglist, le64_to_cpu(tbl[off_idx].addr + off_pos),
-le32_to_cpu(tbl[off_idx].flags_size) + 1 - off_pos);
+prdt_tbl_entry_size(tbl[off_idx]) - off_pos);
  
  for (i = off_idx + 1; i  sglist_alloc_hint; i++) {

  /* flags_size is zero-based */
  qemu_sglist_add(sglist, le64_to_cpu(tbl[i].addr),
-le32_to_cpu(tbl[i].flags_size) + 1);
+prdt_tbl_entry_size(tbl[i]));
  }
  }
  
diff --git a/hw/ide/ahci.h b/hw/ide/ahci.h

index 9a4064f..f418b30 100644
--- a/hw/ide/ahci.h
+++ b/hw/ide/ahci.h
@@ -201,6 +201,8 @@
  
  #define AHCI_COMMAND_TABLE_ACMD0x40
  
+#define AHCI_PRDT_SIZE_MASK0x3f

+
  #define IDE_FEATURE_DMA1
  
  #define READ_FPDMA_QUEUED  0x60





Re: [Qemu-devel] [PATCH] ahci.c: mask unused flags when reading size PRDT DBC

2014-06-30 Thread Eric Blake
On 06/30/2014 06:09 AM, Alexander Graf wrote:
 
 On 29.06.14 20:21, reza.jel...@gmail.com wrote:
 This requires a custom ovmf image with sata controller for testing [0]

 [0]: http://reza.jelveh.me/assets/OVMF.fd.bz2

 
 I guess this is supposed to be a cover letter? A few rules for cover
 letters:
 
   1) Cover letters only make sense for patch sets. This is a single
 patch, so you don't need one
   2) Because they come with patch sets and you number patch sets, cover
 letters are [PATCH 0/n].
   3) Usually cover letters contain git statistics. Try git format-patch
 --cover-letter. It will give you a nice template.
 
 The usual way to add the comment you have here to a patch you're trying
 to submit it is to put volatile information (things that shouldn't end
 up in the git log) behind a --- line in the commit message. Everything
 after that line gets ignored by git when the patch gets applied.

For more hints: http://wiki.qemu.org/Contribute/SubmitAPatch

(if anything said here isn't on that page, well, it's a wiki and we
should add it :)

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



signature.asc
Description: OpenPGP digital signature