[1.2,6/7] ide: support enable/disable write cache

Enabling or disabling the write cache is done with the SET FEATURES
command.  The command can be issued with sg_sat_set_features from
sg3-utils.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/ide/core.c |   18 +++++++++++++++---
 1 files changed, 15 insertions(+), 3 deletions(-)

Am 22.05.2012 18:17, schrieb Paolo Bonzini:
> Enabling or disabling the write cache is done with the SET FEATURES
> command.  The command can be issued with sg_sat_set_features from
> sg3-utils.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/ide/core.c |   18 +++++++++++++++---
>  1 files changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/ide/core.c b/hw/ide/core.c
> index 9785d5f..7c50567 100644
> --- a/hw/ide/core.c
> +++ b/hw/ide/core.c
> @@ -1047,6 +1047,7 @@ static bool ide_cmd_permitted(IDEState *s, uint32_t cmd)
>  
>  void ide_exec_cmd(IDEBus *bus, uint32_t val)
>  {
> +    uint16_t *identify_data;
>      IDEState *s;
>      int n;
>      int lba48 = 0;
> @@ -1231,10 +1232,21 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
>              goto abort_cmd;
>          /* XXX: valid for CDROM ? */
>          switch(s->feature) {
> -        case 0xcc: /* reverting to power-on defaults enable */
> -        case 0x66: /* reverting to power-on defaults disable */
>          case 0x02: /* write cache enable */
> +            bdrv_set_enable_write_cache(s->bs, true);
> +            identify_data = (uint16_t *)s->identify_data;
> +            put_le16(identify_data + 85, (1 << 14) | (1 << 5) | 1);

How about just s->identify_set = 0; instead, so that all IDENTIFY logic
stays together in one place?

> +            s->status = READY_STAT | SEEK_STAT;
> +            ide_set_irq(s->bus);
> +            break;
>          case 0x82: /* write cache disable */
> +            bdrv_set_enable_write_cache(s->bs, false);
> +            identify_data = (uint16_t *)s->identify_data;
> +            put_le16(identify_data + 85, (1 << 14) | 1);
> +            ide_flush_cache(s);

Now I see, you're doing it this way because then the flush is AIO. Hm...
Making it bdrv_aio_set_enable_write_cache() looks a bit extreme.

Kevin

Il 31/05/2012 13:53, Kevin Wolf ha scritto:
>> >          case 0x02: /* write cache enable */
>> > +            bdrv_set_enable_write_cache(s->bs, true);
>> > +            identify_data = (uint16_t *)s->identify_data;
>> > +            put_le16(identify_data + 85, (1 << 14) | (1 << 5) | 1);
> How about just s->identify_set = 0; instead, so that all IDENTIFY logic
> stays together in one place?
> 

Good idea.

Paolo

Il 31/05/2012 14:33, Paolo Bonzini ha scritto:
>>>> >> >          case 0x02: /* write cache enable */
>>>> >> > +            bdrv_set_enable_write_cache(s->bs, true);
>>>> >> > +            identify_data = (uint16_t *)s->identify_data;
>>>> >> > +            put_le16(identify_data + 85, (1 << 14) | (1 << 5) | 1);
>> > How about just s->identify_set = 0; instead, so that all IDENTIFY logic
>> > stays together in one place?
>> > 
> Good idea.

Hmm, this is a rat's nest because of the other patching we do for "set
transfer mode".  I'd prefer to refactor it later.

Paolo

Am 31.05.2012 15:06, schrieb Paolo Bonzini:
> Il 31/05/2012 14:33, Paolo Bonzini ha scritto:
>>>>>>>>          case 0x02: /* write cache enable */
>>>>>>>> +            bdrv_set_enable_write_cache(s->bs, true);
>>>>>>>> +            identify_data = (uint16_t *)s->identify_data;
>>>>>>>> +            put_le16(identify_data + 85, (1 << 14) | (1 << 5) | 1);
>>>> How about just s->identify_set = 0; instead, so that all IDENTIFY logic
>>>> stays together in one place?
>>>>
>> Good idea.
> 
> Hmm, this is a rat's nest because of the other patching we do for "set
> transfer mode".  I'd prefer to refactor it later.

I wasn't aware of that... Okay, then whatever you like better, either
add another refactoring patch before this one for v2, or just keep it as
it is and we'll get to it later.

Kevin