Re: [Qemu-devel] [PATCH v2] PIIX3: reset the VM when the Reset Control Register's RCPU bit gets set

[Laszlo Ersek]

On 01/23/13 09:36, Stefan Hajnoczi wrote:
> On Wed, Jan 16, 2013 at 07:40:19PM +0100, Laszlo Ersek wrote:
>>  static int piix3_post_load(void *opaque, int version_id)
>>  {
>>      PIIX3State *piix3 = opaque;
>>      piix3_update_irq_levels(piix3);
>> +    piix3->rcr &= 2; /* keep System Reset type only */
>>      return 0;
>>  }
> 
> Is this necessary?  I think only an evil migration source could set
> value not in {0x0, 0x2}.

I wanted to make sure in general that no write path to "piix3->rcr"
would let an invalid value through.

> And if so, it doesn't seem like our job to
> validate that.

Independently of the patch: why not?

>> +static void rcr_write(void *opaque, hwaddr addr, uint64_t val, unsigned len)
>> +{
>> +    PIIX3State *d = opaque;
>> +
>> +    if (val & 4) {
>> +        qemu_system_reset_request();
>> +        return;
>> +    }
>> +    d->rcr = val & 2; /* keep System Reset type only */
>> +}
> 
> We don't preserve d->rcr across reset:
> 
> (qemu) o /b 0xcf9 2
> rcr_write val 0x2 rcr 0x2
> (qemu) o /b 0xcf9 4
> piix3_reset rcr = 0
> piix3_reset rcr = 0
> 
> Is this okay?

Yes, that was my intent with modifying piix3_reset().

Thanks,
Laszlo