Re: [Qemu-devel] [PATCH 0/2] sd: sdhci: correct transfer mode register usage

[P J P]

+-- On Tue, 7 Feb 2017, Alistair Francis wrote --+
| > ===
| > diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
| > index d921423..7f3d547 100644
| > --- a/hw/sd/sdhci.c
| > +++ b/hw/sd/sdhci.c
| > @@ -1019,7 +1019,11 @@ sdhci_write(void *opaque, hwaddr offset, uint64_t 
val,
| > unsigned size)
| >  /* Writing to last byte of sdmasysad might trigger transfer */
| >  if (!(mask & 0xFF00) && TRANSFERRING_DATA(s->prnsts) && 
s->blkcnt
| > &&
| >  s->blksize && SDHC_DMA_TYPE(s->hostctl) == SDHC_CTRL_SDMA) 
{
| > -sdhci_sdma_transfer_multi_blocks(s);
| > +if (!(s->trnmod & SDHC_TRNS_MULTI)) {
| > +sdhci_sdma_transfer_single_block(s);
| > +} else {
| > +sdhci_sdma_transfer_multi_blocks(s);
| > +}
| >  }
| >  break;
| >  case SDHC_BLKSIZE:
| > ===
| 
| Should this be a third patch or is this in a different series?

  Yes, a third patch in the series; If it is required.

Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F

Re: [Qemu-devel] [PATCH 0/2] sd: sdhci: correct transfer mode register usage

[Alistair Francis]

On Tue, Feb 7, 2017 at 11:12 AM, P J P  wrote:
> +-- On Tue, 7 Feb 2017, Peter Maydell wrote --+
> | On 31 January 2017 at 12:24, P J P  wrote:
> | > In SDHCI emulation, the 'Block Count Enable' bit of the Transfer Mode
> | > register is used to control 's->blkcnt' value. One, this bit is not
> | > relevant in single block transfers. Second, Transfer Mode register
> | > value could be set such that 's->blkcnt' would not see an update
> | > during multi block transfers. Thus leading to an infinite loop.
> | >
> | > This patch set attempts to correct 'Block Count Enable' bit usage.
> |
> | Edgar, Alistair: the zynq models are our major SDHCI user -- would
> | you like to have a look at this patchset, please?

Yeah, I'll have a look.

>
> I suspect following patch would also be required along with the two in this
> series, not sure.
>
> ===
> diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
> index d921423..7f3d547 100644
> --- a/hw/sd/sdhci.c
> +++ b/hw/sd/sdhci.c
> @@ -1019,7 +1019,11 @@ sdhci_write(void *opaque, hwaddr offset, uint64_t val,
> unsigned size)
>  /* Writing to last byte of sdmasysad might trigger transfer */
>  if (!(mask & 0xFF00) && TRANSFERRING_DATA(s->prnsts) && s->blkcnt
> &&
>  s->blksize && SDHC_DMA_TYPE(s->hostctl) == SDHC_CTRL_SDMA) {
> -sdhci_sdma_transfer_multi_blocks(s);
> +if (!(s->trnmod & SDHC_TRNS_MULTI)) {
> +sdhci_sdma_transfer_single_block(s);
> +} else {
> +sdhci_sdma_transfer_multi_blocks(s);
> +}
>  }
>  break;
>  case SDHC_BLKSIZE:
> ===
>
> Could you please have a look this one too?

Sorry I'm confused. Should this be a third patch or is this in a
different series?

Thanks,

Alistair

>
> Thank you.
> --
> Prasad J Pandit / Red Hat Product Security Team
> 47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F
>

Re: [Qemu-devel] [PATCH 0/2] sd: sdhci: correct transfer mode register usage

[P J P]

+-- On Tue, 7 Feb 2017, Peter Maydell wrote --+
| On 31 January 2017 at 12:24, P J P  wrote:
| > In SDHCI emulation, the 'Block Count Enable' bit of the Transfer Mode
| > register is used to control 's->blkcnt' value. One, this bit is not
| > relevant in single block transfers. Second, Transfer Mode register
| > value could be set such that 's->blkcnt' would not see an update
| > during multi block transfers. Thus leading to an infinite loop.
| >
| > This patch set attempts to correct 'Block Count Enable' bit usage.
| 
| Edgar, Alistair: the zynq models are our major SDHCI user -- would
| you like to have a look at this patchset, please?

I suspect following patch would also be required along with the two in this 
series, not sure.

===
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index d921423..7f3d547 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -1019,7 +1019,11 @@ sdhci_write(void *opaque, hwaddr offset, uint64_t val, 
unsigned size)
 /* Writing to last byte of sdmasysad might trigger transfer */
 if (!(mask & 0xFF00) && TRANSFERRING_DATA(s->prnsts) && s->blkcnt 
&&
 s->blksize && SDHC_DMA_TYPE(s->hostctl) == SDHC_CTRL_SDMA) {
-sdhci_sdma_transfer_multi_blocks(s);
+if (!(s->trnmod & SDHC_TRNS_MULTI)) {
+sdhci_sdma_transfer_single_block(s);
+} else {
+sdhci_sdma_transfer_multi_blocks(s);
+}
 }
 break;
 case SDHC_BLKSIZE:
===

Could you please have a look this one too?

Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F

Re: [Qemu-devel] [PATCH 0/2] sd: sdhci: correct transfer mode register usage

[Peter Maydell]

On 31 January 2017 at 12:24, P J P  wrote:
> From: Prasad J Pandit 
>
> Hello,
>
> In SDHCI emulation, the 'Block Count Enable' bit of the Transfer Mode
> register is used to control 's->blkcnt' value. One, this bit is not
> relevant in single block transfers. Second, Transfer Mode register
> value could be set such that 's->blkcnt' would not see an update
> during multi block transfers. Thus leading to an infinite loop.
>
> This patch set attempts to correct 'Block Count Enable' bit usage.

Edgar, Alistair: the zynq models are our major SDHCI user -- would
you like to have a look at this patchset, please?

thanks
-- PMM

Re: [Qemu-devel] [PATCH 0/2] sd: sdhci: correct transfer mode register usage

[P J P]

+-- On Tue, 31 Jan 2017, P J P wrote --+
| In SDHCI emulation, the 'Block Count Enable' bit of the Transfer Mode
| register is used to control 's->blkcnt' value. One, this bit is not
| relevant in single block transfers. Second, Transfer Mode register
| value could be set such that 's->blkcnt' would not see an update
| during multi block transfers. Thus leading to an infinite loop.
| 
| This patch set attempts to correct 'Block Count Enable' bit usage.
| 
| Thank you.
| --
| Prasad J Pandit (2):
|   sd: sdhci: check transfer mode register in multi block transfer
|   sd: sdhci: block count enable not relevant in single block transfer

Ping...!
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F

[Qemu-devel] [PATCH 0/2] sd: sdhci: correct transfer mode register usage

[P J P]

From: Prasad J Pandit 

Hello,

In SDHCI emulation, the 'Block Count Enable' bit of the Transfer Mode
register is used to control 's->blkcnt' value. One, this bit is not
relevant in single block transfers. Second, Transfer Mode register
value could be set such that 's->blkcnt' would not see an update
during multi block transfers. Thus leading to an infinite loop.

This patch set attempts to correct 'Block Count Enable' bit usage.

Thank you.
--
Prasad J Pandit (2):
  sd: sdhci: check transfer mode register in multi block transfer
  sd: sdhci: block count enable not relevant in single block transfer

 hw/sd/sdhci.c | 19 ---
 1 file changed, 8 insertions(+), 11 deletions(-)

--
2.9.3