Message ID | 1403825329-21942-1-git-send-email-reza.jelveh@tuhh.de |
---|---|
State | New |
Headers | show |
Il 27/06/2014 01:28, Reza Jelveh ha scritto: > +static int prdt_tbl_entry_size(const AHCI_SG tbl) { > + return (le32_to_cpu(tbl.flags_size) & AHCI_PRDT_SIZE_MASK) + 1; > +} Apart from the incorrect indentation/formatting here, the patch seems okay. How can this be reproduced? Paolo
On 06/26/2014 07:28 PM, Reza Jelveh wrote: > +#define AHCI_PRDT_SIZE_MASK 0x3fffff > out of rampant curiosity, is there ever a case where the lower bits might be set and the mask 0x3fffc is not desirable, or can we always trust those bits to simply be off anyway?
On 27.06.14 17:23, John Snow wrote: > > On 06/26/2014 07:28 PM, Reza Jelveh wrote: >> +#define AHCI_PRDT_SIZE_MASK 0x3fffff >> > out of rampant curiosity, is there ever a case where the lower bits > might be set and the mask 0x3fffc is not desirable, or can we always > trust those bits to simply be off anyway? We can't really trust anything from an OS :). But the reason for this patch is that PRDT.I was set on some entries to enable notification of the OS when the entry has been successfully processed. We currently don't emulate the I bit correctly, but get away without doing so. However, we have to make sure we mask it out when interpreting the values. Hence the mask. I do agree that this should have been in the patch description. Reza, could you please repost this with a proper patch description and as a checkpatch.pl compliant patch? Also please CC me on the next iteration :). Alex PS: Is your name really John Snow? That is so cool!
diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index 9bae22e..ee3613f 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -639,6 +639,11 @@ static void ahci_write_fis_d2h(AHCIDevice *ad, uint8_t *cmd_fis) } } +static int prdt_tbl_entry_size(const AHCI_SG tbl) { + return (le32_to_cpu(tbl.flags_size) & AHCI_PRDT_SIZE_MASK) + 1; +} + + static int ahci_populate_sglist(AHCIDevice *ad, QEMUSGList *sglist, int offset) { AHCICmdHdr *cmd = ad->cur_cmd; @@ -681,7 +686,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 +705,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_ACMD 0x40 +#define AHCI_PRDT_SIZE_MASK 0x3fffff + #define IDE_FEATURE_DMA 1 #define READ_FPDMA_QUEUED 0x60
Signed-off-by: Reza Jelveh <reza.jelveh@tuhh.de> --- hw/ide/ahci.c | 11 ++++++++--- hw/ide/ahci.h | 2 ++ 2 files changed, 10 insertions(+), 3 deletions(-)