Hi Laurent,
On Tue, Mar 23, 2021 at 11:14 PM Laurent Vivier <laurent@xxxxxxxxx> wrote:
This machine allows to have up to 3.2 GiB and 128 Virtio devices.
It is based on android goldfish devices.
Signed-off-by: Laurent Vivier <laurent@xxxxxxxxx>
Thanks for your patch!
--- a/arch/m68k/Kconfig.machine
+++ b/arch/m68k/Kconfig.machine
@@ -145,6 +145,23 @@ config SUN3
If you don't want to compile a kernel exclusively for a Sun 3, say N.
+config VIRT
+ bool "Virtual M68k Machine support"
+ depends on MMU
+ select MMU_MOTOROLA if MMU
+ select M68040
+ select LEGACY_TIMER_TICK
Can we avoid selecting this for a new platform?
+ select VIRTIO_MENU
VIRTIO_MENU defaults to y (should it?), so this can be dropped.
+ select VIRTIO_MMIO
+ select GOLDFISH
+ select TTY
+ select GOLDFISH_TTY
+ select RTC_CLASS
+ select RTC_DRV_GOLDFISH
Please sort the selects.
+ help
+ This options enable a pure virtual machine based on m68k,
+ VIRTIO MMIO devices and GOLDFISH interfaces (TTY, RTC, PIC)
+
config PILOT
bool
diff --git a/arch/m68k/configs/virt_defconfig b/arch/m68k/configs/virt_defconfig
new file mode 100644
index 000000000000..51842acd5434
--- /dev/null
+++ b/arch/m68k/configs/virt_defconfig
@@ -0,0 +1,93 @@
This is not a minimal config, please run "make savedefconfig"
and replace by the generated defconfig.
Please also add CONFIG_LOCALVERSION="-virt", for consistency with the
other configs.
--- a/arch/m68k/include/asm/irq.h
+++ b/arch/m68k/include/asm/irq.h
@@ -12,7 +12,8 @@
*/
#if defined(CONFIG_COLDFIRE)
#define NR_IRQS 256
-#elif defined(CONFIG_VME) || defined(CONFIG_SUN3) || defined(CONFIG_SUN3X)
+#elif defined(CONFIG_VME) || defined(CONFIG_SUN3) || \
+ defined(CONFIG_SUN3X) || defined(CONFIG_VIRT)
#define NR_IRQS 200
Is this related to NUM_VIRT_SOURCES?
Yes it is:
m68k_setup_irq_controller(&virt_irq_chip, handle_simple_irq, IRQ_USER,
NUM_VIRT_SOURCES - IRQ_USER);
#elif defined(CONFIG_ATARI)
#define NR_IRQS 141
--- a/arch/m68k/include/asm/setup.h
+++ b/arch/m68k/include/asm/setup.h
@@ -170,6 +180,20 @@ extern unsigned long m68k_machtype;
# define MACH_TYPE (MACH_SUN3X)
#endif
+#if !defined(CONFIG_VIRT)
+# define MACH_IS_VIRT (0)
+#elif defined(CONFIG_AMIGA) || defined(CONFIG_ATARI) || defined(CONFIG_APOLLO) \
+ || defined(CONFIG_MVME16x) || defined(CONFIG_BVME6000) \
+ || defined(CONFIG_HP300) || defined(CONFIG_Q40) \
+ || defined(CONFIG_SUN3X) || defined(CONFIG_MVME147) \
+ || defined(CONFIG_MAC)
Please move the MAC check between the ATARI and APOLLO checks.
+# define MACH_IS_VIRT (m68k_machtype == MACH_VIRT)
+#else
+# define MACH_VIRTONLY
MACH_VIRT_ONLY (albeit unused)
+# define MACH_IS_VIRT (1)
+# define MACH_TYPE (MACH_VIRT)
+#endif
+
#ifndef MACH_TYPE
# define MACH_TYPE (m68k_machtype)
#endif
--- /dev/null
+++ b/arch/m68k/include/asm/virt.h
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __ASM_VIRT_H
+#define __ASM_VIRT_H
+
+#define NUM_VIRT_SOURCES 200
+
+struct virt_booter_device_data {
+ unsigned long mmio;
+ unsigned long irq;
+};
+
+struct virt_booter_data {
+ unsigned long qemu_version;
+ struct virt_booter_device_data pic;
+ struct virt_booter_device_data rtc;
+ struct virt_booter_device_data tty;
+ struct virt_booter_device_data ctrl;
+ struct virt_booter_device_data virtio;
+};
+
+extern struct virt_booter_data virt_bi_data;
+
+extern void __init virt_init_IRQ(void);
+extern void __init virt_sched_init(void);
+
+#endif
diff --git a/arch/m68k/include/uapi/asm/bootinfo-virt.h b/arch/m68k/include/uapi/asm/bootinfo-virt.h
new file mode 100644
index 000000000000..ab17fd9d200d
--- /dev/null
+++ b/arch/m68k/include/uapi/asm/bootinfo-virt.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/*
+ * asm/bootinfo-virt.h -- Virtual-m68k-specific boot information definitions
+ */
+
+#ifndef _UAPI_ASM_M68K_BOOTINFO_VIRT_H
+#define _UAPI_ASM_M68K_BOOTINFO_VIRT_H
+
+#define BI_VIRT_QEMU_VERSION 0x8000
+#define BI_VIRT_GF_PIC_BASE 0x8001
+#define BI_VIRT_GF_RTC_BASE 0x8002
+#define BI_VIRT_GF_TTY_BASE 0x8003
+#define BI_VIRT_VIRTIO_BASE 0x8004
+#define BI_VIRT_CTRL_BASE 0x8005
+
+#define VIRT_BOOTI_VERSION MK_BI_VERSION(2, 0)
+
+#endif /* _UAPI_ASM_M68K_BOOTINFO_MAC_H */
diff --git a/arch/m68k/include/uapi/asm/bootinfo.h b/arch/m68k/include/uapi/asm/bootinfo.h
index 38d3140381fa..203d9cbf9630 100644
--- a/arch/m68k/include/uapi/asm/bootinfo.h
+++ b/arch/m68k/include/uapi/asm/bootinfo.h
@@ -83,6 +83,7 @@ struct mem_info {
#define MACH_SUN3X 11
#define MACH_M54XX 12
#define MACH_M5441X 13
+#define MACH_VIRT 14
All of the above are already in use in qemu, so I have to accept them ;-)
(Do I see a missed opportunity for adding DT support?...)
--- a/arch/m68k/kernel/head.S
+++ b/arch/m68k/kernel/head.S
@@ -3186,6 +3203,13 @@ func_start serial_putc,%d0/%d1/%a0/%a1
3:
#endif
+#ifdef CONFIG_VIRT
+ is_not_virt(L(serial_putc_done))
Please jump to a new label before the #endif, to make it easier to add
new platforms.
+
+ movel L(virt_gf_tty_base),%a1
+ moveb %d0,%a1@(GF_PUT_CHAR)
+#endif
+
L(serial_putc_done):
func_return serial_putc
--- a/arch/m68k/mm/kmap.c
+++ b/arch/m68k/mm/kmap.c
@@ -293,18 +299,20 @@ EXPORT_SYMBOL(__ioremap);
*/
void iounmap(void __iomem *addr)
{
-#ifdef CONFIG_AMIGA
- if ((!MACH_IS_AMIGA) ||
- (((unsigned long)addr < 0x40000000) ||
- ((unsigned long)addr > 0x60000000)))
- free_io_area((__force void *)addr);
+#if defined(CONFIG_AMIGA) || defined(CONFIG_VIRT)
Please split in two separate #ifdefs,...
+ if (MACH_IS_AMIGA &&
+ ((unsigned long)addr >= 0x40000000) &&
+ ((unsigned long)addr < 0x60000000))
+ return;
+ if (MACH_IS_VIRT && (unsigned long)addr >= 0xff000000)
+ return;
#else
... drop the #else, ...
#ifdef CONFIG_COLDFIRE
if (cf_internalio(addr))
return;
#endif
- free_io_area((__force void *)addr);
#endif
... and drop this #endif
+ free_io_area((__force void *)addr);
}
EXPORT_SYMBOL(iounmap);
--- /dev/null
+++ b/arch/m68k/virt/config.c
@@ -0,0 +1,118 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/serial_core.h>
+
+#include <asm/bootinfo.h>
+#include <asm/bootinfo-virt.h>
+#include <asm/byteorder.h>
+
Please drop this blank line.
+#include <asm/machdep.h>
+#include <asm/virt.h>
+static void virt_get_model(char *str)
+{
+ /* str is 80 characters long */
+ sprintf(str, "QEMU Virtual M68K Machine (%d.%d.%d)",
Please use %u for unsigned numbers.
+ (u8)(virt_bi_data.qemu_version >> 24),
+ (u8)(virt_bi_data.qemu_version >> 16),
+ (u8)(virt_bi_data.qemu_version >> 8));
+}
+
+extern void show_registers(struct pt_regs *);
This is unused.
+void __init config_virt(void)
+{
+ char earlycon[24];
+
+ if (!MACH_IS_VIRT)
+ pr_err("ERROR: no Virtual M68k Machine, but %s called!!\n",
+ __func__);
This cannot happen, so please drop.
diff --git a/arch/m68k/virt/ints.c b/arch/m68k/virt/ints.c
new file mode 100644
index 000000000000..aa94cb3b6d96
--- /dev/null
+++ b/arch/m68k/virt/ints.c
@@ -0,0 +1,110 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/types.h>
+#include <linux/kernel.h>
+#include <linux/sched.h>
+#include <linux/sched/debug.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/delay.h>
+
+#include <asm/virt.h>
+#include <asm/irq.h>
+#include <asm/hwtest.h>
+#include <asm/irq_regs.h>
Please sort includes.
+static void virt_irq_enable(struct irq_data *data)
+{
+ GF_PIC(data->irq).enable = 1 << GF_IRQ(data->irq);
+}
+
+static void virt_irq_disable(struct irq_data *data)
+{
+ GF_PIC(data->irq).disable = 1 << GF_IRQ(data->irq);
+}
+
+static unsigned int virt_irq_startup(struct irq_data *data)
+{
+ GF_PIC(data->irq).enable = 1 << GF_IRQ(data->irq);
Just call virt_irq_enable()?
+ return 0;
+}
+
+static void virt_irq_shutdown(struct irq_data *data)
+{
+ GF_PIC(data->irq).disable = 1 << GF_IRQ(data->irq);
+}
This is identical to virt_irq_disable(), so you can just use the latter below.
+
+static volatile int in_nmi;
This is used inside virt_nmi_handler() only, so please move it there.
+
+irqreturn_t virt_nmi_handler(int irq, void *dev_id)
+{
+ if (in_nmi)
+ return IRQ_HANDLED;
+ in_nmi = 1;
+
+ pr_info("Non-Maskable Interrupt\n");
pr_warn()? Or another pr_*()?
+ show_registers(get_irq_regs());
+
+ in_nmi = 0;
+ return IRQ_HANDLED;
+}
+
+static struct irq_chip virt_irq_chip = {
+ .name = "virt",
+ .irq_enable = virt_irq_enable,
+ .irq_disable = virt_irq_disable,
+ .irq_startup = virt_irq_startup,
+ .irq_shutdown = virt_irq_shutdown,
... = virt_irq_disable,
+};
+
+static void goldfish_pic_irq(struct irq_desc *desc)
+{
+ u32 irq_pending, irq_bit;
+ int irq_num;
+
+ irq_pending = gf_pic[desc->irq_data.irq - 1].irq_pending;
+ irq_num = IRQ_USER + (desc->irq_data.irq - 1) * 32;
+ irq_bit = 1;
+
+ do {
+ if (irq_pending & irq_bit) {
+ generic_handle_irq(irq_num);
+ irq_pending &= ~irq_bit;
+ }
+ ++irq_num;
+ irq_bit <<= 1;
+ } while (irq_pending);
This can be simplified by shifting irq_pending instead of irq_bit:
do {
if (irq_pending & 1)
generic_handle_irq(irq_num);
++irq_num;
irq_pending >>= 1;
} while (irq_pending);
Unfortunately m68k doesn't have a single-instruction __ffs().
+}
+
+void __init virt_init_IRQ(void)
+{
+ int i;
+
+ m68k_setup_irq_controller(&virt_irq_chip, handle_simple_irq, IRQ_USER,
+ NUM_VIRT_SOURCES - IRQ_USER);
+
+ for (i = 0; i < 6; i++) {
6 = NUM_VIRT_SOURCES / 32?
If yes, what about the last 8 irqs?
+ irq_set_chained_handler(virt_bi_data.pic.irq + i,
+ goldfish_pic_irq);
+ }
--- /dev/null
+++ b/arch/m68k/virt/platform.c
@@ -0,0 +1,80 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/platform_device.h>
+#include <linux/interrupt.h>
+#include <asm/virt.h>
+#include <asm/irq.h>
+
+static struct platform_device virt_m68k_goldfish_tty = {
+ .name = "goldfish_tty",
+ .id = PLATFORM_DEVID_NONE,
+ .num_resources = 2,
+ .resource = (struct resource [2]) { },
+};
+static struct platform_device virt_m68k_goldfish_rtc = {
+ .name = "goldfish_rtc",
+ .id = PLATFORM_DEVID_NONE,
+ .num_resources = 2,
+ .resource = (struct resource [2]) { },
+};
+
+#define VIRTIO_BUS_NB 128
+static struct platform_device virt_m68k_virtio_mmio_device[VIRTIO_BUS_NB];
+static struct resource virt_m68k_virtio_mmio_resources[VIRTIO_BUS_NB][2];
This consumes more than 40 KiB, even when unused.
While this doesn't matter much for a virtual machine with 3.2 GiB of
RAM, it does matter for running a multi-platform kernel on a real
machine. Hence please allocate dynamically.
+
+static int __init virt_platform_init(void)
+{
+ int err;
+ int i;
+ extern unsigned long min_low_pfn;
Please use reverse Christmas tree declaration order.
+
+ if (!MACH_IS_VIRT)
+ return -ENODEV;
+
+ min_low_pfn = 0;
Why is this needed?
+
+ virt_m68k_goldfish_tty.resource[0].flags = IORESOURCE_MEM;
+ virt_m68k_goldfish_tty.resource[0].start = virt_bi_data.tty.mmio;
+ virt_m68k_goldfish_tty.resource[0].end = virt_bi_data.tty.mmio;
+ virt_m68k_goldfish_tty.resource[1].flags = IORESOURCE_IRQ;
+ virt_m68k_goldfish_tty.resource[1].start = virt_bi_data.tty.irq;
+ virt_m68k_goldfish_tty.resource[1].end = virt_bi_data.tty.irq;
+
+ err = platform_device_register(&virt_m68k_goldfish_tty);
You could probably save a little bit of memory by calling
platform_device_register_simple() instead.
+ if (err)
+ return err;
+
+ virt_m68k_goldfish_rtc.resource[0].flags = IORESOURCE_MEM;
+ virt_m68k_goldfish_rtc.resource[0].start = virt_bi_data.rtc.mmio + 0x1000;
+ virt_m68k_goldfish_rtc.resource[0].end = virt_bi_data.rtc.mmio + 0x1fff;
+ virt_m68k_goldfish_rtc.resource[1].flags = IORESOURCE_IRQ;
+ virt_m68k_goldfish_rtc.resource[1].start = virt_bi_data.rtc.irq + 1;
+ virt_m68k_goldfish_rtc.resource[1].end = virt_bi_data.rtc.irq + 1;
+ err = platform_device_register(&virt_m68k_goldfish_rtc);
Likewise.
--- /dev/null
+++ b/arch/m68k/virt/timer.c
@@ -0,0 +1,91 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/interrupt.h>
+#include <linux/ioport.h>
+#include <linux/clocksource.h>
+#include <asm/virt.h>
+
+struct goldfish_timer {
+ u32 time_low;
+ u32 time_high;
+ u32 alarm_low;
+ u32 alarm_high;
+ u32 irq_enabled;
+ u32 clear_alarm;
+ u32 alarm_status;
+ u32 clear_interrupt;
+};
+
+#define gf_timer ((volatile struct goldfish_timer *)virt_bi_data.rtc.mmio)
+
+static u64 goldfish_timer_read(struct clocksource *cs)
+{
+ u64 ticks;
+
+ ticks = gf_timer->time_low;
+ ticks += (u64)gf_timer->time_high << 32;
Can time_low wrap in between the two reads?
+
+ return ticks;
+}
+
+static struct clocksource goldfish_timer = {
+ .name = "goldfish_timer",
+ .rating = 400,
+ .read = goldfish_timer_read,
+ .mask = CLOCKSOURCE_MASK(64),
+ .flags = 0,
+ .max_idle_ns = LONG_MAX,
+};
+
+static irqreturn_t golfish_timer_handler(int irq, void *dev_id)
+{
+ unsigned long flags;
+ u64 now;
+
+ local_irq_save(flags);
Do we need this in an interrupt handler?
+ gf_timer->clear_interrupt = 1;
+
+ now = gf_timer->time_low;
+ now += (u64)gf_timer->time_high << 32;
now = goldfish_timer_read();
+
+ legacy_timer_tick(1);
+
+ now += NSEC_PER_SEC / HZ;
+ gf_timer->alarm_high = now >> 32;
upper_32_bits(now)
+ gf_timer->alarm_low = (u32)now;
lower_32_bits(now)
+ local_irq_restore(flags);
+
+ return IRQ_HANDLED;
+}
+
+void __init virt_sched_init(void)
+{
+ u64 now;
+ static struct resource sched_res;
Please use reverse Christmas tree declaration order.
+
+ sched_res.name = "goldfish_timer";
+ sched_res.start = virt_bi_data.rtc.mmio;
+ sched_res.end = virt_bi_data.rtc.mmio + 0xfff;
+
+ if (request_resource(&iomem_resource, &sched_res)) {
+ pr_err("Cannot allocate goldfish-timer resource\n");
+ return;
+ }
+
+ if (request_irq(virt_bi_data.rtc.irq, golfish_timer_handler, IRQF_TIMER,
+ "timer", NULL)) {
+ pr_err("Couldn't register timer interrupt\n");
+ return;
+ }
+
+ now = gf_timer->time_low;
+ now += (u64)gf_timer->time_high << 32;
now = goldfish_timer_read();
+ now += NSEC_PER_SEC / HZ;
+
+ gf_timer->clear_interrupt = 1;
+ gf_timer->alarm_high = now >> 32;
upper_32_bits(now)
+ gf_timer->alarm_low = (u32)now;
lower_32_bits(now)
+ gf_timer->irq_enabled = 1;
+
+ clocksource_register_hz(&goldfish_timer, NSEC_PER_SEC);
+}
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
