[v2,for-5.1] acpi-pm-tmr: allow any small-size reads

As found in LP#1886318, MacOS Catalina performs 2-byte reads
on the acpi timer address space while the spec says it should
be 4-byte. Allow any small reads.

Reported-By: Simon John <git@the-jedi.co.uk>
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
---
 hw/acpi/core.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

v2: fixed bug#, use the right form of S-o-b, and allow up to 1 byte reads.

I'm applying this to debian qemu package, need the fix
faster in order to release security updates for other
branches.

On Tue, Jul 14, 2020 at 01:51:13PM +0300, Michael Tokarev wrote:
> As found in LP#1886318, MacOS Catalina performs 2-byte reads
> on the acpi timer address space while the spec says it should
> be 4-byte. Allow any small reads.
> 
> Reported-By: Simon John <git@the-jedi.co.uk>
> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>


Do we need Simon to test this? Or did you already test
with MacOSX?



> ---
>  hw/acpi/core.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> v2: fixed bug#, use the right form of S-o-b, and allow up to 1 byte reads.
> 
> I'm applying this to debian qemu package, need the fix
> faster in order to release security updates for other
> branches.
> 
> diff --git a/hw/acpi/core.c b/hw/acpi/core.c
> --- a/hw/acpi/core.c
> +++ b/hw/acpi/core.c
> @@ -530,7 +530,10 @@ static void acpi_pm_tmr_write(void *opaque, hwaddr addr, uint64_t val,
>  static const MemoryRegionOps acpi_pm_tmr_ops = {
>      .read = acpi_pm_tmr_read,
>      .write = acpi_pm_tmr_write,
> -    .valid.min_access_size = 4,
> +    .impl.min_access_size = 4,
> +     /* at least MacOS Catalina reads 2 bytes and fails if it doesn't work */
> +     /* allow 1-byte reads too */
> +    .valid.min_access_size = 1,
>      .valid.max_access_size = 4,
>      .endianness = DEVICE_LITTLE_ENDIAN,
>  };
> -- 
> 2.20.1

On 7/14/20 12:51 PM, Michael Tokarev wrote:
> As found in LP#1886318, MacOS Catalina performs 2-byte reads
> on the acpi timer address space while the spec says it should
> be 4-byte. Allow any small reads.
> 

BugLink: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=964247

> Reported-By: Simon John <git@the-jedi.co.uk>
> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
> ---
>  hw/acpi/core.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> v2: fixed bug#, use the right form of S-o-b, and allow up to 1 byte reads.
> 
> I'm applying this to debian qemu package, need the fix
> faster in order to release security updates for other
> branches.
> 
> diff --git a/hw/acpi/core.c b/hw/acpi/core.c
> --- a/hw/acpi/core.c
> +++ b/hw/acpi/core.c
> @@ -530,7 +530,10 @@ static void acpi_pm_tmr_write(void *opaque, hwaddr addr, uint64_t val,
>  static const MemoryRegionOps acpi_pm_tmr_ops = {
>      .read = acpi_pm_tmr_read,
>      .write = acpi_pm_tmr_write,
> +    .impl.min_access_size = 4,

This is correct.

> +     /* at least MacOS Catalina reads 2 bytes and fails if it doesn't work */
> +     /* allow 1-byte reads too */
> -    .valid.min_access_size = 4,
> +    .valid.min_access_size = 1,

This is a quick fix for 5.1, but I don't think this is the correct
long-term fix. See:
https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg04639.html

>      .valid.max_access_size = 4,
>      .endianness = DEVICE_LITTLE_ENDIAN,
>  };
>

On Tue, Jul 14, 2020 at 01:51:13PM +0300, Michael Tokarev wrote:
> As found in LP#1886318, MacOS Catalina performs 2-byte reads
> on the acpi timer address space while the spec says it should
> be 4-byte. Allow any small reads.
> 
> Reported-By: Simon John <git@the-jedi.co.uk>
> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>

Simon's explanation about the history is good to have here,
and I guess Fixes tags (both what you found and what Simon found)
can't hurt either. I would CC stable too.

Simon do you have the time to iterate on this patch or would
you rather have Michael do it?


> ---
>  hw/acpi/core.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> v2: fixed bug#, use the right form of S-o-b, and allow up to 1 byte reads.
> 
> I'm applying this to debian qemu package, need the fix
> faster in order to release security updates for other
> branches.
> 
> diff --git a/hw/acpi/core.c b/hw/acpi/core.c
> --- a/hw/acpi/core.c
> +++ b/hw/acpi/core.c
> @@ -530,7 +530,10 @@ static void acpi_pm_tmr_write(void *opaque, hwaddr addr, uint64_t val,
>  static const MemoryRegionOps acpi_pm_tmr_ops = {
>      .read = acpi_pm_tmr_read,
>      .write = acpi_pm_tmr_write,
> -    .valid.min_access_size = 4,
> +    .impl.min_access_size = 4,
> +     /* at least MacOS Catalina reads 2 bytes and fails if it doesn't work */
> +     /* allow 1-byte reads too */

... since historically we did in the past.

> +    .valid.min_access_size = 1,
>      .valid.max_access_size = 4,
>      .endianness = DEVICE_LITTLE_ENDIAN,
>  };
> -- 
> 2.20.1