Re: [PATCH v5 09/11] macio: Add dummy screamer register area - Mark Cave-Ayland

From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
To: BALATON Zoltan <balaton@eik.bme.hu>,
	qemu-devel@nongnu.org, qemu-ppc@nongnu.org
Cc: David Gibson <david@gibson.dropbear.id.au>,
	Howard Spoelstra <hsp.cat7@gmail.com>
Subject: Re: [PATCH v5 09/11] macio: Add dummy screamer register area
Date: Fri, 26 Jun 2020 14:12:34 +0100	[thread overview]
Message-ID: <3012998c-8345-f654-1de1-dd623ef375e8@ilande.co.uk> (raw)
In-Reply-To: <ecaaee4ba802b7a53de03e401c4d68e94de554f5.1592315226.git.balaton@eik.bme.hu>

On 16/06/2020 14:47, BALATON Zoltan wrote:

> The only thing this returns is an idle status so the firmware
> continues, otherwise just ignores and logs access for debugging. This
> is a stop gap until proper implementation of this device lands.
> 
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
>  hw/misc/macio/macio.c | 34 ++++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
> 
> diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
> index 8ba7af073c..c7e8556ca6 100644
> --- a/hw/misc/macio/macio.c
> +++ b/hw/misc/macio/macio.c
> @@ -26,6 +26,7 @@
>  #include "qemu/osdep.h"
>  #include "qapi/error.h"
>  #include "qemu/module.h"
> +#include "qemu/log.h"
>  #include "hw/ppc/mac.h"
>  #include "hw/misc/macio/cuda.h"
>  #include "hw/pci/pci.h"
> @@ -94,6 +95,33 @@ static void macio_bar_setup(MacIOState *s)
>      macio_escc_legacy_setup(s);
>  }
>  
> +#define AWAC_CODEC_STATUS_REG 0x20
> +
> +#define AWAC_MAKER_CRYSTAL 1
> +#define AWAC_REV_SCREAMER 3
> +#define AWAC_VALID_DATA 0x40
> +
> +static uint64_t screamer_read(void *opaque, hwaddr addr, unsigned size)
> +{
> +    qemu_log_mask(LOG_UNIMP,
> +                  "macio: screamer read %" HWADDR_PRIx "  %d\n", addr, size);
> +    return (addr == AWAC_CODEC_STATUS_REG ? AWAC_VALID_DATA << 8 |
> +            AWAC_MAKER_CRYSTAL << 16 | AWAC_REV_SCREAMER << 20 : 0);
> +}
> +
> +static void screamer_write(void *opaque, hwaddr addr,
> +                           uint64_t val, unsigned size)
> +{
> +    qemu_log_mask(LOG_UNIMP,
> +                  "macio: screamer write %" HWADDR_PRIx "  %d = %"PRIx64"\n",
> +                  addr, size, val);
> +}
> +
> +const MemoryRegionOps screamer_ops = {
> +    .read = screamer_read,
> +    .write = screamer_write,
> +};

static const.

>  static void macio_common_realize(PCIDevice *d, Error **errp)
>  {
>      MacIOState *s = MACIO(d);
> @@ -149,6 +177,7 @@ static void macio_oldworld_realize(PCIDevice *d, Error **errp)
>      DeviceState *pic_dev = DEVICE(os->pic);
>      Error *err = NULL;
>      SysBusDevice *sysbus_dev;
> +    MemoryRegion *screamer = g_new(MemoryRegion, 1);
>  
>      macio_common_realize(d, &err);
>      if (err) {
> @@ -208,6 +237,11 @@ static void macio_oldworld_realize(PCIDevice *d, Error **errp)
>          error_propagate(errp, err);
>          return;
>      }
> +
> +    /* Dummy screamer sound device */
> +    memory_region_init_io(screamer, OBJECT(d), &screamer_ops, NULL,
> +                          "screamer", 0x2000);
> +    memory_region_add_subregion(&s->bar, 0x14000, screamer);
>  }
>  
>  static void macio_init_ide(MacIOState *s, MACIOIDEState *ide, int index)

Again I'm wary of adding empty devices here as the main issue around the screamer
code (and why it has not been submitted upstream) is that it can cause random hangs
for MacOS on startup. Does it regress any MacOS 9 through 10.5 boot tests?

FWIW I've rebased the latest version of my screamer branch at
https://github.com/mcayland/qemu/commits/screamer to git master if you want to see if
any noise comes out.

ATB,

Mark.