[PATCH 0/4] Emulator fixes to enable running NetBSD/alpha

[PATCH 0/4] Emulator fixes to enable running NetBSD/alpha

[Jason Thorpe]

The following are a set of fixes to device and system emulation that
allow NetBSD/alpha to run in qemu-system-alpha.

The first change fixes behavior in the mc146818rtc emulation to more
accurately reflect how the real hardware vis a vis the PF status bit
(specifically, that it is independent of the PIE control bit).  The
behavior of PF now matches the data sheet for the part.  This documented
behavior is relied upon by NetBSD/alpha to calibrate some timing loops.

The next two fix up a couple of aspects of the emulated device topology
for the "Clipper" system emulation.

The fourth passes console configuration information to the PALcode
at start-up, which in turn will reflect this information in the
Console Terminal Block to the guest operating system, and relies
on a corresponding PALcode change, although older PALcode will still
work if the "-nographics" option is not specified.

Jason Thorpe (4):
  mc146818rtc: Make PF independent of PIE
  alpha: Set minimum PCI device ID to 1 to match Clipper IRQ mappings.
  alpha: Provide a PCI-ISA bridge device node for guest OS's that expect
    it
  alpha: Provide console information to the PALcode at start-up.

 hw/alpha/alpha_sys.h |   2 +-
 hw/alpha/dp264.c     |  16 +++++-
 hw/alpha/typhoon.c   | 116 +++++++++++++++++++++++++++++++++++++++++--
 hw/rtc/mc146818rtc.c |   4 --
 4 files changed, 127 insertions(+), 11 deletions(-)

-- 
2.30.2

[PATCH 1/4] mc146818rtc: Make PF independent of PIE

[Jason Thorpe]

Make the PF flag behave like real hardware by always running the
periodic timer without regard to the setting of the PIE bit, so
that the PF will be set when the period expires even if an interrupt
will not be raised.  This behavior is documented on page 16 of the
MC146818A advance information datasheet.

Signed-off-by: Jason Thorpe <thorpej@me.com>
---
 hw/rtc/mc146818rtc.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/hw/rtc/mc146818rtc.c b/hw/rtc/mc146818rtc.c
index 4fbafddb22..366b8f13de 100644
--- a/hw/rtc/mc146818rtc.c
+++ b/hw/rtc/mc146818rtc.c
@@ -155,10 +155,6 @@ static uint32_t rtc_periodic_clock_ticks(RTCState *s)
 {
     int period_code;
 
-    if (!(s->cmos_data[RTC_REG_B] & REG_B_PIE)) {
-        return 0;
-     }
-
     period_code = s->cmos_data[RTC_REG_A] & 0x0f;
 
     return periodic_period_to_clock(period_code);
-- 
2.30.2

[PATCH 2/4] alpha: Set minimum PCI device ID to 1 to match Clipper IRQ mappings.

[Jason Thorpe]

Since we are emulating a Clipper device topology, we need to set the
minimum PCI device ID to 1, as there is no IRQ mapping for a device
at ID 0 (see sys_dp264.c:clipper_map_irq()).

- Add a 'devfn_min' argument to typhoon_init().  Pass that argument
  along to pci_register_root_bus().
- In clipper_init(), pass PCI_DEVFN(1, 0) as the minimum PCI device
  ID/function.

Signed-off-by: Jason Thorpe <thorpej@me.com>
---
 hw/alpha/alpha_sys.h | 2 +-
 hw/alpha/dp264.c     | 5 +++--
 hw/alpha/typhoon.c   | 5 +++--
 3 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/hw/alpha/alpha_sys.h b/hw/alpha/alpha_sys.h
index e2c02e2bbe..4835b3d5ee 100644
--- a/hw/alpha/alpha_sys.h
+++ b/hw/alpha/alpha_sys.h
@@ -11,7 +11,7 @@
 
 
 PCIBus *typhoon_init(MemoryRegion *, ISABus **, qemu_irq *, AlphaCPU *[4],
-                     pci_map_irq_fn);
+                     pci_map_irq_fn, uint8_t devfn_min);
 
 /* alpha_pci.c.  */
 extern const MemoryRegionOps alpha_pci_ignore_ops;
diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
index 1017ecf330..ac97104464 100644
--- a/hw/alpha/dp264.c
+++ b/hw/alpha/dp264.c
@@ -76,9 +76,10 @@ static void clipper_init(MachineState *machine)
     cpus[0]->env.trap_arg1 = 0;
     cpus[0]->env.trap_arg2 = smp_cpus;
 
-    /* Init the chipset.  */
+    /* Init the chipset.  Because we're using CLIPPER IRQ mappings,
+       the minimum PCI device IdSel is 1.  */
     pci_bus = typhoon_init(machine->ram, &isa_bus, &rtc_irq, cpus,
-                           clipper_pci_map_irq);
+                           clipper_pci_map_irq, PCI_DEVFN(1, 0));
 
     /* Since we have an SRM-compatible PALcode, use the SRM epoch.  */
     mc146818_rtc_init(isa_bus, 1900, rtc_irq);
diff --git a/hw/alpha/typhoon.c b/hw/alpha/typhoon.c
index 87020cbe0d..fa31a2f286 100644
--- a/hw/alpha/typhoon.c
+++ b/hw/alpha/typhoon.c
@@ -815,7 +815,8 @@ static void typhoon_alarm_timer(void *opaque)
 }
 
 PCIBus *typhoon_init(MemoryRegion *ram, ISABus **isa_bus, qemu_irq *p_rtc_irq,
-                     AlphaCPU *cpus[4], pci_map_irq_fn sys_map_irq)
+                     AlphaCPU *cpus[4], pci_map_irq_fn sys_map_irq,
+                     uint8_t devfn_min)
 {
     MemoryRegion *addr_space = get_system_memory();
     DeviceState *dev;
@@ -885,7 +886,7 @@ PCIBus *typhoon_init(MemoryRegion *ram, ISABus **isa_bus, qemu_irq *p_rtc_irq,
     b = pci_register_root_bus(dev, "pci",
                               typhoon_set_irq, sys_map_irq, s,
                               &s->pchip.reg_mem, &s->pchip.reg_io,
-                              0, 64, TYPE_PCI_BUS);
+                              devfn_min, 64, TYPE_PCI_BUS);
     phb->bus = b;
     sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
 
-- 
2.30.2

[PATCH 3/4] alpha: Provide a PCI-ISA bridge device node for guest OS's that expect it

[Jason Thorpe]

Provide a PCI device node at ID 7 for the PCI-ISA bridge.  Even though
Tsunami/Typhoon systems would have used a different chip (Cypress or ALI),
for simplicity we model the Intel i82378, which was also used on several
Alpha models.  This is needed for some operating systems that only probe
ISA devices if a PCI-ISA or PCI-EISA bridge is found.

Signed-off-by: Jason Thorpe <thorpej@me.com>
---
 hw/alpha/typhoon.c | 111 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 109 insertions(+), 2 deletions(-)

diff --git a/hw/alpha/typhoon.c b/hw/alpha/typhoon.c
index fa31a2f286..de01828d23 100644
--- a/hw/alpha/typhoon.c
+++ b/hw/alpha/typhoon.c
@@ -919,11 +919,32 @@ PCIBus *typhoon_init(MemoryRegion *ram, ISABus **isa_bus, qemu_irq *p_rtc_irq,
     /* Pchip1 PCI I/O, 0x802.FC00.0000, 32MB.  */
     /* Pchip1 PCI configuration, 0x802.FE00.0000, 16MB.  */
 
-    /* Init the ISA bus.  */
-    /* ??? Technically there should be a cy82c693ub pci-isa bridge.  */
+    /* Init the PCI-ISA bridge.  Technically, PCI-based Alphas shipped
+       with one of three different PCI-ISA bridges:
+
+       - Intel i82378 SIO
+       - Cypress CY82c693UB
+       - ALI M1533
+
+       (An Intel i82375 PCI-EISA bridge was also used on some models.)
+
+       For simplicity, we model an i82378 here, even though it wouldn't
+       have been on any Tsunami/Typhoon systems; it's close enough, and
+       we don't want to deal with modelling the CY82c693UB (which has
+       incompatible edge/level control registers, plus other peripherals
+       like IDE and USB) or the M1533 (which also has IDE and USB).
+
+       Importantly, we need to provide a PCI device node for it, otherwise
+       some operating systems won't notice there's an ISA bus to configure.
+
+       ??? We are using a private, stripped-down implementation of i82378
+       so that we can handle the way the ISA interrupts are wired up on
+       Tsunami/Typhoon systems.  */
     {
         qemu_irq *isa_irqs;
 
+        pci_create_simple(b, PCI_DEVFN(7, 0), "i82378-typhoon");
+
         *isa_bus = isa_bus_new(NULL, get_system_memory(), &s->pchip.reg_io,
                                &error_abort);
         isa_irqs = i8259_init(*isa_bus,
@@ -954,10 +975,96 @@ static const TypeInfo typhoon_iommu_memory_region_info = {
     .class_init = typhoon_iommu_memory_region_class_init,
 };
 
+/* The following was copied from hw/isa/i82378.c and modified to provide
+   only the minimal PCI device node.  */
+
+/*
+ * QEMU Intel i82378 emulation (PCI to ISA bridge)
+ *
+ * Copyright (c) 2010-2011 Herv\xc3\xa9 Poussineau
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "migration/vmstate.h"
+
+#define TYPE_I82378 "i82378-typhoon"
+#define I82378(obj) \
+    OBJECT_CHECK(I82378State, (obj), TYPE_I82378)
+
+typedef struct I82378State {
+    PCIDevice parent_obj;
+} I82378State;
+
+static const VMStateDescription vmstate_i82378 = {
+    .name = "pci-i82378-typhoon",
+    .version_id = 0,
+    .minimum_version_id = 0,
+    .fields = (VMStateField[]) {
+        VMSTATE_PCI_DEVICE(parent_obj, I82378State),
+        VMSTATE_END_OF_LIST()
+    },
+};
+
+static void i82378_realize(PCIDevice *pci, Error **errp)
+{
+    uint8_t *pci_conf;
+
+    pci_conf = pci->config;
+    pci_set_word(pci_conf + PCI_COMMAND,
+                 PCI_COMMAND_IO | PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER);
+    pci_set_word(pci_conf + PCI_STATUS,
+                 PCI_STATUS_DEVSEL_MEDIUM);
+
+    pci_config_set_interrupt_pin(pci_conf, 1); /* interrupt pin 0 */
+}
+
+static void i82378_init(Object *obj)
+{
+}
+
+static void i82378_class_init(ObjectClass *klass, void *data)
+{
+    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    k->realize = i82378_realize;
+    k->vendor_id = PCI_VENDOR_ID_INTEL;
+    k->device_id = PCI_DEVICE_ID_INTEL_82378;
+    k->revision = 0x03;
+    k->class_id = PCI_CLASS_BRIDGE_ISA;
+    dc->vmsd = &vmstate_i82378;
+    set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
+}
+
+static const TypeInfo i82378_typhoon_type_info = {
+    .name = TYPE_I82378,
+    .parent = TYPE_PCI_DEVICE,
+    .instance_size = sizeof(I82378State),
+    .instance_init = i82378_init,
+    .class_init = i82378_class_init,
+    .interfaces = (InterfaceInfo[]) {
+        { INTERFACE_CONVENTIONAL_PCI_DEVICE },
+        { },
+    },
+};
+
 static void typhoon_register_types(void)
 {
     type_register_static(&typhoon_pcihost_info);
     type_register_static(&typhoon_iommu_memory_region_info);
+    type_register_static(&i82378_typhoon_type_info);
 }
 
 type_init(typhoon_register_types)
-- 
2.30.2

[PATCH 4/4] alpha: Provide console information to the PALcode at start-up.

[Jason Thorpe]

Redefine the a2 register passed by Qemu at start-up to also include
some configuration flags, in addition to the CPU count, and define
a flag to mirror the "-nographics" option.

Signed-off-by: Jason Thorpe <thorpej@me.com>
---
 hw/alpha/dp264.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
index ac97104464..d86dcb1d81 100644
--- a/hw/alpha/dp264.c
+++ b/hw/alpha/dp264.c
@@ -72,9 +72,20 @@ static void clipper_init(MachineState *machine)
         cpus[i] = ALPHA_CPU(cpu_create(machine->cpu_type));
     }
 
+    /* arg0 -> memory size
+       arg1 -> kernel entry point
+       arg2 -> config word
+
+       Config word: bits 0-5 -> ncpus
+                    bit  6   -> nographics option (for HWRPB CTB)
+
+       See init_hwrpb() in the PALcode.  */
+
     cpus[0]->env.trap_arg0 = ram_size;
     cpus[0]->env.trap_arg1 = 0;
     cpus[0]->env.trap_arg2 = smp_cpus;
+    if (!machine->enable_graphics)
+      cpus[0]->env.trap_arg2 |= (1 << 6);
 
     /* Init the chipset.  Because we're using CLIPPER IRQ mappings,
        the minimum PCI device IdSel is 1.  */
-- 
2.30.2

Re: [PATCH 0/4] Emulator fixes to enable running NetBSD/alpha

[no-reply]

Patchew URL: https://patchew.org/QEMU/20210613211549.18094-1-thorpej@me.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20210613211549.18094-1-thorpej@me.com
Subject: [PATCH 0/4] Emulator fixes to enable running NetBSD/alpha

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]         patchew/20210610001424.209158-1-jiang.wang@bytedance.com -> patchew/20210610001424.209158-1-jiang.wang@bytedance.com
 * [new tag]         patchew/20210612085101.61304-1-sw@weilnetz.de -> patchew/20210612085101.61304-1-sw@weilnetz.de
Switched to a new branch 'test'
ae5541d alpha: Provide console information to the PALcode at start-up.
ed14ccc alpha: Provide a PCI-ISA bridge device node for guest OS's that expect it
4ab8e91 alpha: Set minimum PCI device ID to 1 to match Clipper IRQ mappings.
8e9fae5 mc146818rtc: Make PF independent of PIE

=== OUTPUT BEGIN ===
1/4 Checking commit 8e9fae55ef15 (mc146818rtc: Make PF independent of PIE)
2/4 Checking commit 4ab8e91bd35e (alpha: Set minimum PCI device ID to 1 to match Clipper IRQ mappings.)
WARNING: Block comments use a leading /* on a separate line
#46: FILE: hw/alpha/dp264.c:79:
+    /* Init the chipset.  Because we're using CLIPPER IRQ mappings,

WARNING: Block comments use * on subsequent lines
#47: FILE: hw/alpha/dp264.c:80:
+    /* Init the chipset.  Because we're using CLIPPER IRQ mappings,
+       the minimum PCI device IdSel is 1.  */

WARNING: Block comments use a trailing */ on a separate line
#47: FILE: hw/alpha/dp264.c:80:
+       the minimum PCI device IdSel is 1.  */

total: 0 errors, 3 warnings, 37 lines checked

Patch 2/4 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
3/4 Checking commit ed14ccc6e1b5 (alpha: Provide a PCI-ISA bridge device node for guest OS's that expect it)
WARNING: Block comments use a leading /* on a separate line
#29: FILE: hw/alpha/typhoon.c:922:
+    /* Init the PCI-ISA bridge.  Technically, PCI-based Alphas shipped

WARNING: Block comments use * on subsequent lines
#30: FILE: hw/alpha/typhoon.c:923:
+    /* Init the PCI-ISA bridge.  Technically, PCI-based Alphas shipped
+       with one of three different PCI-ISA bridges:

WARNING: Block comments use a trailing */ on a separate line
#49: FILE: hw/alpha/typhoon.c:942:
+       Tsunami/Typhoon systems.  */

WARNING: Block comments use a leading /* on a separate line
#62: FILE: hw/alpha/typhoon.c:978:
+/* The following was copied from hw/isa/i82378.c and modified to provide

WARNING: Block comments use * on subsequent lines
#63: FILE: hw/alpha/typhoon.c:979:
+/* The following was copied from hw/isa/i82378.c and modified to provide
+   only the minimal PCI device node.  */

WARNING: Block comments use a trailing */ on a separate line
#63: FILE: hw/alpha/typhoon.c:979:
+   only the minimal PCI device node.  */

total: 0 errors, 6 warnings, 130 lines checked

Patch 3/4 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
4/4 Checking commit ae5541d5739e (alpha: Provide console information to the PALcode at start-up.)
WARNING: Block comments use a leading /* on a separate line
#25: FILE: hw/alpha/dp264.c:75:
+    /* arg0 -> memory size

WARNING: Block comments use * on subsequent lines
#26: FILE: hw/alpha/dp264.c:76:
+    /* arg0 -> memory size
+       arg1 -> kernel entry point

WARNING: Block comments use a trailing */ on a separate line
#32: FILE: hw/alpha/dp264.c:82:
+       See init_hwrpb() in the PALcode.  */

ERROR: suspect code indent for conditional statements (4, 6)
#37: FILE: hw/alpha/dp264.c:87:
+    if (!machine->enable_graphics)
+      cpus[0]->env.trap_arg2 |= (1 << 6);

ERROR: braces {} are necessary for all arms of this statement
#37: FILE: hw/alpha/dp264.c:87:
+    if (!machine->enable_graphics)
[...]

total: 2 errors, 3 warnings, 20 lines checked

Patch 4/4 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20210613211549.18094-1-thorpej@me.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [PATCH 2/4] alpha: Set minimum PCI device ID to 1 to match Clipper IRQ mappings.

[Richard Henderson]

On 6/13/21 2:15 PM, Jason Thorpe wrote:
> Since we are emulating a Clipper device topology, we need to set the
> minimum PCI device ID to 1, as there is no IRQ mapping for a device
> at ID 0 (see sys_dp264.c:clipper_map_irq()).
> 
> - Add a 'devfn_min' argument to typhoon_init().  Pass that argument
>    along to pci_register_root_bus().
> - In clipper_init(), pass PCI_DEVFN(1, 0) as the minimum PCI device
>    ID/function.
> 
> Signed-off-by: Jason Thorpe<thorpej@me.com>
> ---
>   hw/alpha/alpha_sys.h | 2 +-
>   hw/alpha/dp264.c     | 5 +++--
>   hw/alpha/typhoon.c   | 5 +++--
>   3 files changed, 7 insertions(+), 5 deletions(-)

Thanks, queued.

r~

Re: [PATCH 4/4] alpha: Provide console information to the PALcode at start-up.

[Richard Henderson]

On 6/13/21 2:15 PM, Jason Thorpe wrote:
> Redefine the a2 register passed by Qemu at start-up to also include
> some configuration flags, in addition to the CPU count, and define
> a flag to mirror the "-nographics" option.
> 
> Signed-off-by: Jason Thorpe <thorpej@me.com>
> ---
>   hw/alpha/dp264.c | 11 +++++++++++
>   1 file changed, 11 insertions(+)
> 
> diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
> index ac97104464..d86dcb1d81 100644
> --- a/hw/alpha/dp264.c
> +++ b/hw/alpha/dp264.c
> @@ -72,9 +72,20 @@ static void clipper_init(MachineState *machine)
>           cpus[i] = ALPHA_CPU(cpu_create(machine->cpu_type));
>       }
>   
> +    /* arg0 -> memory size
> +       arg1 -> kernel entry point
> +       arg2 -> config word
> +
> +       Config word: bits 0-5 -> ncpus
> +                    bit  6   -> nographics option (for HWRPB CTB)
> +
> +       See init_hwrpb() in the PALcode.  */
> +
>       cpus[0]->env.trap_arg0 = ram_size;
>       cpus[0]->env.trap_arg1 = 0;
>       cpus[0]->env.trap_arg2 = smp_cpus;
> +    if (!machine->enable_graphics)
> +      cpus[0]->env.trap_arg2 |= (1 << 6);

Thanks, I have fixed up the style problems and queued.


r~

Re: [PATCH 1/4] mc146818rtc: Make PF independent of PIE

[Richard Henderson]

Cc: paolo and mst

On 6/13/21 2:15 PM, Jason Thorpe wrote:
> Make the PF flag behave like real hardware by always running the
> periodic timer without regard to the setting of the PIE bit, so
> that the PF will be set when the period expires even if an interrupt
> will not be raised.  This behavior is documented on page 16 of the
> MC146818A advance information datasheet.
> 
> Signed-off-by: Jason Thorpe <thorpej@me.com>
> ---
>   hw/rtc/mc146818rtc.c | 4 ----
>   1 file changed, 4 deletions(-)
> 
> diff --git a/hw/rtc/mc146818rtc.c b/hw/rtc/mc146818rtc.c
> index 4fbafddb22..366b8f13de 100644
> --- a/hw/rtc/mc146818rtc.c
> +++ b/hw/rtc/mc146818rtc.c
> @@ -155,10 +155,6 @@ static uint32_t rtc_periodic_clock_ticks(RTCState *s)
>   {
>       int period_code;
>   
> -    if (!(s->cmos_data[RTC_REG_B] & REG_B_PIE)) {
> -        return 0;
> -     }

This looks correct, but I don't know enough about this device.


r~

Re: [PATCH 3/4] alpha: Provide a PCI-ISA bridge device node for guest OS's that expect it

[Richard Henderson]

On 6/13/21 2:15 PM, Jason Thorpe wrote:
> +/* The following was copied from hw/isa/i82378.c and modified to provide
> +   only the minimal PCI device node.  */
> +
> +/*
> + * QEMU Intel i82378 emulation (PCI to ISA bridge)
> + *

Why can't we just use the existing device model?
Certainly duplicating code like this isn't the best way.


r~

Re: [PATCH 3/4] alpha: Provide a PCI-ISA bridge device node for guest OS's that expect it

[Jason Thorpe]

> On Jun 14, 2021, at 9:20 PM, Richard Henderson <richard.henderson@linaro.org> wrote:
> 
> On 6/13/21 2:15 PM, Jason Thorpe wrote:
>> +/* The following was copied from hw/isa/i82378.c and modified to provide
>> +   only the minimal PCI device node.  */
>> +
>> +/*
>> + * QEMU Intel i82378 emulation (PCI to ISA bridge)
>> + *
> 
> Why can't we just use the existing device model?
> Certainly duplicating code like this isn't the best way.

Yah, I’m not super happy with that, either, tbh.  When I first started working on this several months ago, I it looked like it would be invasive to wire it up in the way the Alpha platform expects, but I can’t remember exactly what the issue was.

Anyways, I’ll look at it again.  Stay tuned.

-- thorpej

Re: [PATCH 3/4] alpha: Provide a PCI-ISA bridge device node for guest OS's that expect it

[Jason Thorpe]

> On Jun 14, 2021, at 9:24 PM, Jason Thorpe <thorpej@me.com> wrote:

>> Why can't we just use the existing device model?
>> Certainly duplicating code like this isn't the best way.
> 
> Yah, I’m not super happy with that, either, tbh.  When I first started working on this several months ago, I it looked like it would be invasive to wire it up in the way the Alpha platform expects, but I can’t remember exactly what the issue was.
> 
> Anyways, I’ll look at it again.  Stay tuned.

I spent some time on this last night and figured out the bit I was missing before, and just booted NetBSD in the emulator using the existing SIO model.  Since you’ve already queued up the others, I’ll post a v2 of just that patch in a few minutes (hopefully with the correct block comment styling this time :-).

-- thorpej

Re: [PATCH 3/4] alpha: Provide a PCI-ISA bridge device node for guest OS's that expect it

[Richard Henderson]

On 6/16/21 6:53 AM, Jason Thorpe wrote:
> Since you’ve already queued up the others...

To be clear, I have not queued patch 1, re the rtc.
I was hoping to get some additional feedback on that,
since it doesn't affect only alpha.


r~

Re: [PATCH 1/4] mc146818rtc: Make PF independent of PIE

[Jason Thorpe]

> On Jun 14, 2021, at 9:17 PM, Richard Henderson <richard.henderson@linaro.org> wrote:
> 
> Cc: paolo and mst
> 
> On 6/13/21 2:15 PM, Jason Thorpe wrote:
>> Make the PF flag behave like real hardware by always running the
>> periodic timer without regard to the setting of the PIE bit, so
>> that the PF will be set when the period expires even if an interrupt
>> will not be raised.  This behavior is documented on page 16 of the
>> MC146818A advance information datasheet.
>> Signed-off-by: Jason Thorpe <thorpej@me.com>
>> ---
>>  hw/rtc/mc146818rtc.c | 4 ----
>>  1 file changed, 4 deletions(-)
>> diff --git a/hw/rtc/mc146818rtc.c b/hw/rtc/mc146818rtc.c
>> index 4fbafddb22..366b8f13de 100644
>> --- a/hw/rtc/mc146818rtc.c
>> +++ b/hw/rtc/mc146818rtc.c
>> @@ -155,10 +155,6 @@ static uint32_t rtc_periodic_clock_ticks(RTCState *s)
>>  {
>>      int period_code;
>>  -    if (!(s->cmos_data[RTC_REG_B] & REG_B_PIE)) {
>> -        return 0;
>> -     }
> 
> This looks correct, but I don't know enough about this device.

Quoting the data sheet here, for context:

<quote>
PF - The periodic interrupt flag (PF) is a read-only bit which is set to a "1" when a particular edge is detected on the selected tap of the divider chain.  The RS3 to RS0 bits establish the periodic rate.  PF is set to "1" independent of the state of the PIE bit.  PF initiates an ~IRQ signal and sets the IRQF bit when PIE is also a "1".  The PF bit is cleared by a ~RESET or a software read of Register C.
</quote>

This is why my patch always runs the timer, and does not suppress it if PF is already set; real hardware will always latch PF at regular intervals irrespective of when software reads Register C.

-- thorpej

Re: [PATCH 1/4] mc146818rtc: Make PF independent of PIE

[Philippe Mathieu-Daudé]

Hi Jason,

On 6/16/21 7:34 PM, Jason Thorpe wrote:
> 
>> On Jun 14, 2021, at 9:17 PM, Richard Henderson <richard.henderson@linaro.org> wrote:
>>
>> Cc: paolo and mst
>>
>> On 6/13/21 2:15 PM, Jason Thorpe wrote:
>>> Make the PF flag behave like real hardware by always running the
>>> periodic timer without regard to the setting of the PIE bit, so
>>> that the PF will be set when the period expires even if an interrupt
>>> will not be raised.  This behavior is documented on page 16 of the
>>> MC146818A advance information datasheet.
>>> Signed-off-by: Jason Thorpe <thorpej@me.com>
>>> ---
>>>  hw/rtc/mc146818rtc.c | 4 ----
>>>  1 file changed, 4 deletions(-)
>>> diff --git a/hw/rtc/mc146818rtc.c b/hw/rtc/mc146818rtc.c
>>> index 4fbafddb22..366b8f13de 100644
>>> --- a/hw/rtc/mc146818rtc.c
>>> +++ b/hw/rtc/mc146818rtc.c
>>> @@ -155,10 +155,6 @@ static uint32_t rtc_periodic_clock_ticks(RTCState *s)
>>>  {
>>>      int period_code;
>>>  -    if (!(s->cmos_data[RTC_REG_B] & REG_B_PIE)) {
>>> -        return 0;
>>> -     }
>>
>> This looks correct, but I don't know enough about this device.
> 
> Quoting the data sheet here, for context:
> 
> <quote>
> PF - The periodic interrupt flag (PF) is a read-only bit which is set to a "1" when a particular edge is detected on the selected tap of the divider chain.  The RS3 to RS0 bits establish the periodic rate.  PF is set to "1" independent of the state of the PIE bit.  PF initiates an ~IRQ signal and sets the IRQF bit when PIE is also a "1".  The PF bit is cleared by a ~RESET or a software read of Register C.
> </quote>
> 
> This is why my patch always runs the timer, and does not suppress it if PF is already set; real hardware will always latch PF at regular intervals irrespective of when software reads Register C.

Do you mind reposting this single patch including the DS quote
in the description?

Regards,

Phil.

Re: [PATCH 1/4] mc146818rtc: Make PF independent of PIE

[Jason Thorpe]

> On Jun 19, 2021, at 8:56 AM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> 
> Hi Jason,
> 
> Do you mind reposting this single patch including the DS quote
> in the description?

Sure, no problem.  Will do so shortly.

-- thorpej