[6/6] hw/pc.c: Fix converting of ioport_register* to MemoryRegion

From: Julien Grall <julien.grall@citrix.com>

The commit 258711 introduced MemoryRegion to replace ioport_region*
for ioport 80h and F0h.
A MemoryRegion needs to have both read and write callback otherwise a segfault
will occur when an access is made.

The previous behaviour of this both ioport is to return 0xffffffffffffffff.
So keep this behaviour.

Reported-by: Adam Lackorzynski <adam@os.inf.tu-dresden.de>
Signed-off-by: Julien Grall <julien.grall@citrix.com>
Tested-by: Adam Lackorzynski <adam@os.inf.tu-dresden.de>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/pc.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

Am 11.01.2013 10:18, schrieb Stefan Hajnoczi:
> From: Julien Grall <julien.grall@citrix.com>
> 
> The commit 258711 introduced MemoryRegion to replace ioport_region*
> for ioport 80h and F0h.
> A MemoryRegion needs to have both read and write callback otherwise a segfault
> will occur when an access is made.
> 
> The previous behaviour of this both ioport is to return 0xffffffffffffffff.
> So keep this behaviour.
> 
> Reported-by: Adam Lackorzynski <adam@os.inf.tu-dresden.de>
> Signed-off-by: Julien Grall <julien.grall@citrix.com>
> Tested-by: Adam Lackorzynski <adam@os.inf.tu-dresden.de>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  hw/pc.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/hw/pc.c b/hw/pc.c
> index df0c48e..90b1bf7 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -103,6 +103,11 @@ static void ioport80_write(void *opaque, hwaddr addr, uint64_t data,
>  {
>  }
>  
> +static uint64_t ioport80_read(void *opaque, hwaddr addr, unsigned size)
> +{
> +    return 0xffffffffffffffff;

Might these require ULL for i386?

Andreas

> +}
> +
>  /* MSDOS compatibility mode FPU exception support */
>  static qemu_irq ferr_irq;
>  
> @@ -123,6 +128,11 @@ static void ioportF0_write(void *opaque, hwaddr addr, uint64_t data,
>      qemu_irq_lower(ferr_irq);
>  }
>  
> +static uint64_t ioportF0_read(void *opaque, hwaddr addr, unsigned size)
> +{
> +    return 0xffffffffffffffff;
> +}
> +
>  /* TSC handling */
>  uint64_t cpu_get_tsc(CPUX86State *env)
>  {
> @@ -960,6 +970,7 @@ static void cpu_request_exit(void *opaque, int irq, int level)
>  
>  static const MemoryRegionOps ioport80_io_ops = {
>      .write = ioport80_write,
> +    .read = ioport80_read,
>      .endianness = DEVICE_NATIVE_ENDIAN,
>      .impl = {
>          .min_access_size = 1,
> @@ -969,6 +980,7 @@ static const MemoryRegionOps ioport80_io_ops = {
>  
>  static const MemoryRegionOps ioportF0_io_ops = {
>      .write = ioportF0_write,
> +    .read = ioportF0_read,
>      .endianness = DEVICE_NATIVE_ENDIAN,
>      .impl = {
>          .min_access_size = 1,

On 01/11/2013 11:50 AM, Andreas Färber wrote:

> Am 11.01.2013 10:18, schrieb Stefan Hajnoczi:
>> From: Julien Grall <julien.grall@citrix.com>
>>
>> The commit 258711 introduced MemoryRegion to replace ioport_region*
>> for ioport 80h and F0h.
>> A MemoryRegion needs to have both read and write callback otherwise a segfault
>> will occur when an access is made.
>>
>> The previous behaviour of this both ioport is to return 0xffffffffffffffff.
>> So keep this behaviour.
>>
>> Reported-by: Adam Lackorzynski <adam@os.inf.tu-dresden.de>
>> Signed-off-by: Julien Grall <julien.grall@citrix.com>
>> Tested-by: Adam Lackorzynski <adam@os.inf.tu-dresden.de>
>> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
>> ---
>>  hw/pc.c | 12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/hw/pc.c b/hw/pc.c
>> index df0c48e..90b1bf7 100644
>> --- a/hw/pc.c
>> +++ b/hw/pc.c
>> @@ -103,6 +103,11 @@ static void ioport80_write(void *opaque, hwaddr addr, uint64_t data,
>>  {
>>  }
>>  
>> +static uint64_t ioport80_read(void *opaque, hwaddr addr, unsigned size)
>> +{
>> +    return 0xffffffffffffffff;
> 
> Might these require ULL for i386?

Indeed. I will resend a patch with ULL for ioport80_read and ioportF0_read.

>

>> +}
>> +
>>  /* MSDOS compatibility mode FPU exception support */
>>  static qemu_irq ferr_irq;
>>  
>> @@ -123,6 +128,11 @@ static void ioportF0_write(void *opaque, hwaddr addr, uint64_t data,
>>      qemu_irq_lower(ferr_irq);
>>  }
>>  
>> +static uint64_t ioportF0_read(void *opaque, hwaddr addr, unsigned size)
>> +{
>> +    return 0xffffffffffffffff;
>> +}
>> +
>>  /* TSC handling */
>>  uint64_t cpu_get_tsc(CPUX86State *env)
>>  {
>> @@ -960,6 +970,7 @@ static void cpu_request_exit(void *opaque, int irq, int level)
>>  
>>  static const MemoryRegionOps ioport80_io_ops = {
>>      .write = ioport80_write,
>> +    .read = ioport80_read,
>>      .endianness = DEVICE_NATIVE_ENDIAN,
>>      .impl = {
>>          .min_access_size = 1,
>> @@ -969,6 +980,7 @@ static const MemoryRegionOps ioport80_io_ops = {
>>  
>>  static const MemoryRegionOps ioportF0_io_ops = {
>>      .write = ioportF0_write,
>> +    .read = ioportF0_read,
>>      .endianness = DEVICE_NATIVE_ENDIAN,
>>      .impl = {
>>          .min_access_size = 1,
>

Am 11.01.2013 15:53, schrieb Julien Grall:
> On 01/11/2013 11:50 AM, Andreas Färber wrote:
>> Am 11.01.2013 10:18, schrieb Stefan Hajnoczi:
>>> @@ -103,6 +103,11 @@ static void ioport80_write(void *opaque, hwaddr addr, uint64_t data,
>>>  {
>>>  }
>>>  
>>> +static uint64_t ioport80_read(void *opaque, hwaddr addr, unsigned size)
>>> +{
>>> +    return 0xffffffffffffffff;
>>
>> Might these require ULL for i386?
> 
> Indeed. I will resend a patch with ULL for ioport80_read and ioportF0_read.

Since this patch is already in a pull and Anthony indicated it is being
processed, can you fix this as a follow-up patch instead?

Thanks,
Andreas