* [PATCH 1/4] hw/sd/sdhci: Stop multiple transfers when block count is cleared
2020-09-03 17:28 [PATCH 0/4] hw/sd/sdhci: Strengthen multiple DMA transfers Philippe Mathieu-Daudé
@ 2020-09-03 17:28 ` Philippe Mathieu-Daudé
2020-09-03 17:28 ` [PATCH 2/4] hw/sd/sdhci: Resume pending DMA transfers on MMIO accesses Philippe Mathieu-Daudé
` (5 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-09-03 17:28 UTC (permalink / raw)
To: qemu-devel
Cc: Prasad J Pandit, qemu-block, Philippe Mathieu-Daudé,
Alexander Bulekov, qemu-arm, Ruhr-University
Clearing BlockCount stops multiple transfers.
See "SD Host Controller Simplified Specification Version 2.00":
- 2.2.3. Block Count Register (Offset 006h)
- Table 2-8 : Determination of Transfer Type
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/sd/sdhci.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index ecbf84e9d3f..703357e94a7 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -728,6 +728,12 @@ static void sdhci_do_adma(SDHCIState *s)
ADMADescr dscr = {};
int i;
+ if (s->trnmod & SDHC_TRNS_BLK_CNT_EN && !s->blkcnt) {
+ /* Stop Multiple Transfer */
+ sdhci_end_transfer(s);
+ return;
+ }
+
for (i = 0; i < SDHC_ADMA_DESCS_PER_DELAY; ++i) {
s->admaerr &= ~SDHC_ADMAERR_LENGTH_MISMATCH;
@@ -753,7 +759,6 @@ static void sdhci_do_adma(SDHCIState *s)
switch (dscr.attr & SDHC_ADMA_ATTR_ACT_MASK) {
case SDHC_ADMA_ATTR_ACT_TRAN: /* data transfer */
-
if (s->trnmod & SDHC_TRNS_READ) {
while (length) {
if (s->data_count == 0) {
--
2.26.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/4] hw/sd/sdhci: Resume pending DMA transfers on MMIO accesses
2020-09-03 17:28 [PATCH 0/4] hw/sd/sdhci: Strengthen multiple DMA transfers Philippe Mathieu-Daudé
2020-09-03 17:28 ` [PATCH 1/4] hw/sd/sdhci: Stop multiple transfers when block count is cleared Philippe Mathieu-Daudé
@ 2020-09-03 17:28 ` Philippe Mathieu-Daudé
2020-09-03 17:28 ` [PATCH 3/4] hw/sd/sdhci: Let sdhci_update_irq() return if IRQ was delivered Philippe Mathieu-Daudé
` (4 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-09-03 17:28 UTC (permalink / raw)
To: qemu-devel
Cc: Prasad J Pandit, qemu-block, Philippe Mathieu-Daudé,
Alexander Bulekov, qemu-arm, Ruhr-University
If we have pending DMA requests scheduled, process them first.
So far we don't need to implement a bottom half to process them.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/sd/sdhci.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 703357e94a7..2b197631870 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -945,11 +945,21 @@ sdhci_buff_access_is_sequential(SDHCIState *s, unsigned byte_num)
return true;
}
+static void sdhci_resume_pending_transfer(SDHCIState *s)
+{
+ timer_del(s->transfer_timer);
+ sdhci_data_transfer(s);
+}
+
static uint64_t sdhci_read(void *opaque, hwaddr offset, unsigned size)
{
SDHCIState *s = (SDHCIState *)opaque;
uint32_t ret = 0;
+ if (timer_pending(s->transfer_timer)) {
+ sdhci_resume_pending_transfer(s);
+ }
+
switch (offset & ~0x3) {
case SDHC_SYSAD:
ret = s->sdmasysad;
@@ -1093,6 +1103,10 @@ sdhci_write(void *opaque, hwaddr offset, uint64_t val, unsigned size)
uint32_t value = val;
value <<= shift;
+ if (timer_pending(s->transfer_timer)) {
+ sdhci_resume_pending_transfer(s);
+ }
+
switch (offset & ~0x3) {
case SDHC_SYSAD:
s->sdmasysad = (s->sdmasysad & mask) | value;
--
2.26.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/4] hw/sd/sdhci: Let sdhci_update_irq() return if IRQ was delivered
2020-09-03 17:28 [PATCH 0/4] hw/sd/sdhci: Strengthen multiple DMA transfers Philippe Mathieu-Daudé
2020-09-03 17:28 ` [PATCH 1/4] hw/sd/sdhci: Stop multiple transfers when block count is cleared Philippe Mathieu-Daudé
2020-09-03 17:28 ` [PATCH 2/4] hw/sd/sdhci: Resume pending DMA transfers on MMIO accesses Philippe Mathieu-Daudé
@ 2020-09-03 17:28 ` Philippe Mathieu-Daudé
2020-09-03 17:28 ` [PATCH 4/4] hw/sd/sdhci: Yield if interrupt delivered during multiple transfer Philippe Mathieu-Daudé
` (3 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-09-03 17:28 UTC (permalink / raw)
To: qemu-devel
Cc: Prasad J Pandit, qemu-block, Philippe Mathieu-Daudé,
Alexander Bulekov, qemu-arm, Ruhr-University
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/sd/sdhci.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 2b197631870..06cb098036c 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -215,9 +215,14 @@ static uint8_t sdhci_slotint(SDHCIState *s)
((s->norintsts & SDHC_NIS_REMOVE) && (s->wakcon & SDHC_WKUP_ON_RMV));
}
-static inline void sdhci_update_irq(SDHCIState *s)
+/* Return true if IRQ was pending and delivered */
+static bool sdhci_update_irq(SDHCIState *s)
{
- qemu_set_irq(s->irq, sdhci_slotint(s));
+ bool pending = sdhci_slotint(s);
+
+ qemu_set_irq(s->irq, pending);
+
+ return pending;
}
static void sdhci_raise_insertion_irq(void *opaque)
--
2.26.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 4/4] hw/sd/sdhci: Yield if interrupt delivered during multiple transfer
2020-09-03 17:28 [PATCH 0/4] hw/sd/sdhci: Strengthen multiple DMA transfers Philippe Mathieu-Daudé
` (2 preceding siblings ...)
2020-09-03 17:28 ` [PATCH 3/4] hw/sd/sdhci: Let sdhci_update_irq() return if IRQ was delivered Philippe Mathieu-Daudé
@ 2020-09-03 17:28 ` Philippe Mathieu-Daudé
2020-09-10 7:19 ` [PATCH 0/4] hw/sd/sdhci: Strengthen multiple DMA transfers Philippe Mathieu-Daudé
` (2 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-09-03 17:28 UTC (permalink / raw)
To: qemu-devel
Cc: Prasad J Pandit, qemu-block, Philippe Mathieu-Daudé,
Alexander Bulekov, qemu-arm, Ruhr-University
The Descriptor Table has a bit to allow the DMA to generates
Interrupt when the operation of the descriptor line is completed
(see "1.13.4. Descriptor Table" of 'SD Host Controller Simplified
Specification Version 2.00').
If we have pending interrupt and the descriptor requires it
to be generated as soon as it is completed, reschedule pending
transfers and yield to the CPU.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/sd/sdhci.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 06cb098036c..74b0bf77103 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -834,7 +834,10 @@ static void sdhci_do_adma(SDHCIState *s)
s->norintsts |= SDHC_NIS_DMA;
}
- sdhci_update_irq(s);
+ if (sdhci_update_irq(s) && !(dscr.attr & SDHC_ADMA_ATTR_END)) {
+ /* IRQ delivered, reschedule current transfer */
+ break;
+ }
}
/* ADMA transfer terminates if blkcnt == 0 or by END attribute */
--
2.26.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 0/4] hw/sd/sdhci: Strengthen multiple DMA transfers
2020-09-03 17:28 [PATCH 0/4] hw/sd/sdhci: Strengthen multiple DMA transfers Philippe Mathieu-Daudé
` (3 preceding siblings ...)
2020-09-03 17:28 ` [PATCH 4/4] hw/sd/sdhci: Yield if interrupt delivered during multiple transfer Philippe Mathieu-Daudé
@ 2020-09-10 7:19 ` Philippe Mathieu-Daudé
2020-09-10 14:47 ` Alexander Bulekov
2020-09-18 8:27 ` Philippe Mathieu-Daudé
6 siblings, 0 replies; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-09-10 7:19 UTC (permalink / raw)
To: qemu-devel
Cc: Alexander Bulekov, qemu-arm, Ruhr-University, Prasad J Pandit,
qemu-block
On 9/3/20 7:28 PM, Philippe Mathieu-Daudé wrote:
> Still trying to fix the bugs reported by Aleksander...
>
> - Do not send 0 block count
> - Reduce DMA to MMIO re-entrancy by yielding when pending IRQ
>
> Based-on: <20200901140411.112150-1-f4bug@amsat.org>
>
> Philippe Mathieu-Daudé (4):
> hw/sd/sdhci: Stop multiple transfers when block count is cleared
> hw/sd/sdhci: Resume pending DMA transfers on MMIO accesses
> hw/sd/sdhci: Let sdhci_update_irq() return if IRQ was delivered
> hw/sd/sdhci: Yield if interrupt delivered during multiple transfer
>
> hw/sd/sdhci.c | 35 +++++++++++++++++++++++++++++++----
> 1 file changed, 31 insertions(+), 4 deletions(-)
>
ping for review?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/4] hw/sd/sdhci: Strengthen multiple DMA transfers
2020-09-03 17:28 [PATCH 0/4] hw/sd/sdhci: Strengthen multiple DMA transfers Philippe Mathieu-Daudé
` (4 preceding siblings ...)
2020-09-10 7:19 ` [PATCH 0/4] hw/sd/sdhci: Strengthen multiple DMA transfers Philippe Mathieu-Daudé
@ 2020-09-10 14:47 ` Alexander Bulekov
2020-09-18 8:27 ` Philippe Mathieu-Daudé
6 siblings, 0 replies; 8+ messages in thread
From: Alexander Bulekov @ 2020-09-10 14:47 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-arm, Ruhr-University, qemu-devel, qemu-block, Prasad J Pandit
I fuzzed the SDHCI with this applied. There are still bugs in SDHCI, but
this fixes the ones triggered by my initial bug-reproducers, and doesn't
appear to create any new bugs.
In the interest of incrementally fixing the issues, for this series:
Tested-by: Alexander Bulekov <alxndr@bu.edu>
On 200903 1928, Philippe Mathieu-Daudé wrote:
> Still trying to fix the bugs reported by Aleksander...
>
> - Do not send 0 block count
> - Reduce DMA to MMIO re-entrancy by yielding when pending IRQ
>
> Based-on: <20200901140411.112150-1-f4bug@amsat.org>
>
> Philippe Mathieu-Daudé (4):
> hw/sd/sdhci: Stop multiple transfers when block count is cleared
> hw/sd/sdhci: Resume pending DMA transfers on MMIO accesses
> hw/sd/sdhci: Let sdhci_update_irq() return if IRQ was delivered
> hw/sd/sdhci: Yield if interrupt delivered during multiple transfer
>
> hw/sd/sdhci.c | 35 +++++++++++++++++++++++++++++++----
> 1 file changed, 31 insertions(+), 4 deletions(-)
>
> --
> 2.26.2
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/4] hw/sd/sdhci: Strengthen multiple DMA transfers
2020-09-03 17:28 [PATCH 0/4] hw/sd/sdhci: Strengthen multiple DMA transfers Philippe Mathieu-Daudé
` (5 preceding siblings ...)
2020-09-10 14:47 ` Alexander Bulekov
@ 2020-09-18 8:27 ` Philippe Mathieu-Daudé
6 siblings, 0 replies; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-09-18 8:27 UTC (permalink / raw)
To: qemu-devel
Cc: Alexander Bulekov, qemu-arm, Ruhr-University, Prasad J Pandit,
qemu-block
On 9/3/20 7:28 PM, Philippe Mathieu-Daudé wrote:
> Still trying to fix the bugs reported by Aleksander...
>
> - Do not send 0 block count
> - Reduce DMA to MMIO re-entrancy by yielding when pending IRQ
>
> Based-on: <20200901140411.112150-1-f4bug@amsat.org>
>
> Philippe Mathieu-Daudé (4):
> hw/sd/sdhci: Stop multiple transfers when block count is cleared
> hw/sd/sdhci: Resume pending DMA transfers on MMIO accesses
> hw/sd/sdhci: Let sdhci_update_irq() return if IRQ was delivered
> hw/sd/sdhci: Yield if interrupt delivered during multiple transfer
Thanks, series applied to my sd-next tree.
^ permalink raw reply [flat|nested] 8+ messages in thread