Re: [PATCH 4/6] hw/pci-host/sabre: Report unimplemented accesses via UNIMP log_mask

[Mark Cave-Ayland]

On 12/10/2020 18:09, Philippe Mathieu-Daudé wrote:

> Report unimplemented register accesses using qemu_log_mask(UNIMP).
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>   hw/pci-host/sabre.c | 12 ++++++++----
>   1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/hw/pci-host/sabre.c b/hw/pci-host/sabre.c
> index 3645bc962cb..4412e23131c 100644
> --- a/hw/pci-host/sabre.c
> +++ b/hw/pci-host/sabre.c
> @@ -121,8 +121,10 @@ static void sabre_config_write(void *opaque, hwaddr addr,
>       trace_sabre_config_write(addr, val);
>   
>       switch (addr) {
> -    case 0x30 ... 0x4f: /* DMA error registers */
> -        /* XXX: not implemented yet */
> +    case  0x30 ...  0x4f: /* DMA error registers */
> +        qemu_log_mask(LOG_UNIMP,
> +                      "%s: Register 0x%02" HWADDR_PRIX " not implemented\n",
> +                      __func__, addr);
>           break;
>       case 0xc00 ... 0xc3f: /* PCI interrupt control */
>           if (addr & 4) {
> @@ -198,8 +200,10 @@ static uint64_t sabre_config_read(void *opaque,
>       uint32_t val = 0;
>   
>       switch (addr) {
> -    case 0x30 ... 0x4f: /* DMA error registers */
> -        /* XXX: not implemented yet */
> +    case  0x30 ...  0x4f: /* DMA error registers */
> +        qemu_log_mask(LOG_UNIMP,
> +                      "%s: Register 0x%02" HWADDR_PRIX " not implemented\n",
> +                      __func__, addr);
>           break;
>       case 0xc00 ... 0xc3f: /* PCI interrupt control */
>           if (addr & 4) {

It seems as if there are quite a few other registers that haven't been implemented 
here which aren't mentioned in the comments. My preference would be to rework this 
patch so that the comments for the unimplemented registers are all at the end of the 
switch() with the fallthrough to default, and then update patch 6 to use LOG_UNIMP so 
everything is logged in one place.


ATB,

Mark.