Re: [Qemu-devel] [PATCH 1/2] hw/sd: implement CMD23 (SET_BLOCK_COUNT) for MMC compatibility

[Andrew Baumann]

> From: Peter Crosthwaite [mailto:address@hidden
> Sent: Saturday, 5 December 2015 22:27
> On Fri, Dec 4, 2015 at 1:16 PM, Andrew Baumann
> <address@hidden> wrote:
> > @@ -1563,6 +1592,11 @@ void sd_write_data(SDState *sd, uint8_t value)
> >              sd->data_offset = 0;
> >              sd->csd[14] |= 0x40;
> >
> > +            if (sd->multi_blk_active) {
> > +                assert(sd->multi_blk_cnt > 0);
> > +                sd->multi_blk_cnt--;
> 
> So reading the spec, it says that this command is an alternative to a
> host issued CMD12. Does this mean completion of the length-specified
> read should move back to sd_transfer_state the same way CMD12 does?

That was my initial assumption (and implementation) too, but no: table 4-34 in 
the spec shows that you stay in transfer mode, and UEFI issues CMD12 after each 
multiple-block (CMD23) I/O, which doesn't work if you've already left the 
transfer state.

> >      case 18:   /* CMD18:  READ_MULTIPLE_BLOCK */
> > +        if (sd->multi_blk_active && sd->multi_blk_cnt == 0) {
> > +            /* we've overrun the block count */
> > +            fprintf(stderr, "sd_read_data: overrun of multi-block 
> > transfer\n");
> 
> So if the card stays in-state and the intention is to discard overrun,
> this message is likely to be flood prone. From my understanding (which
> is pretty fresh) CMD23 may be designed to gracefully handle this
> condition from the guest, which would suggest this is not an error at
> all and no message is needed. If you do want to keep a message it
> should be GUEST_ERROR and have a squelching mechanism so reading one
> block past the end of CMD23 doesn't cause a large number of messages.

Point taken. These printfs were more to assure myself that they never happened; 
they can be removed. (It did seem weird to use a bare printf, but all the code 
around here seemed to be doing the same thing.)

I'll send a revised patch and respond to the other feedback probably later next 
week.

Thanks,
Andrew