Re: [PATCH qemu] x86: don't let decompressed kernel image clobber setup_data
On 12/31/22 20:55, Mika Penttilä wrote: If decompression does clobber the data, then we *also* need to figure out why that is. There are basically three possibilities: 1. If physical KASLR is NOT used: a. The boot loader doesn't honor the kernel safe area properly; b. Somewhere in the process a bug in the calculation of the kernel safe area has crept in. 2. If physical KASLR IS used: The decompressor doesn't correctly keep track of nor relocate all the keep-out zones before picking a target address. Seems setup_data is not included in those mem_avoid regions. [facepalm] One is a bootloader bug, two is a kernel bugs. My guess is (2) is the culprit, but (1b) should be checked, too. Correction: two are kernel bugs, i.e. (1b) and (2) are both kernel bugs. -hpa
Re: [PATCH qemu] x86: don't let decompressed kernel image clobber setup_data
On 12/31/22 10:22, Jason A. Donenfeld wrote: On Sat, Dec 31, 2022 at 03:24:32PM +0100, Borislav Petkov wrote: On Sat, Dec 31, 2022 at 02:51:28PM +0100, Jason A. Donenfeld wrote: That failure is unrelated to the ident mapping issue Peter and I discussed. The original failure is described in the commit message: decompression clobbers the data, so sd->next points to garbage. Right So with that understanding confirmed, I'm confused at your surprise that hpa's unrelated fix to the different issue didn't fix this issue. If decompression does clobber the data, then we *also* need to figure out why that is. There are basically three possibilities: 1. If physical KASLR is NOT used: a. The boot loader doesn't honor the kernel safe area properly; b. Somewhere in the process a bug in the calculation of the kernel safe area has crept in. 2. If physical KASLR IS used: The decompressor doesn't correctly keep track of nor relocate all the keep-out zones before picking a target address. One is a bootloader bug, two is a kernel bug. My guess is (2) is the culprit, but (1b) should be checked, too. -hpa
Re: [PATCH qemu] x86: don't let decompressed kernel image clobber setup_data
On 12/31/22 19:21, H. Peter Anvin wrote: Alternatively, setup_data could be relocated, the boot param protocol could be bumped, and then QEMU could conditionalized it's use of setup_data based on that protocol version. That'd work, but seems a bit more involved. I think this is at least worth considering because the kernel overwriting setup_data after having been told where that setup_data is located is not really nice. Well not explicitly at least - it gets the pointer to the first element and something needs to traverse that list to know which addresses are live. But still, that info is there so perhaps we need to take setup_data into consideration too before decompressing... It would probably be a good idea to add a "maximum physical address for initrd/setup_data/cmdline" field to struct kernel_info, though. It appears right now that those fields are being identity-mapped in the decompressor, and that means that if 48-bit addressing is used, physical memory may extend past the addressable range. -hpa
Re: [PATCH qemu] x86: don't let decompressed kernel image clobber setup_data
On 12/31/22 11:00, Borislav Petkov wrote: On Sat, Dec 31, 2022 at 07:22:47PM +0100, Jason A. Donenfeld wrote: So with that understanding confirmed, I'm confused at your surprise that hpa's unrelated fix to the different issue didn't fix this issue. No surprise there - I used a qemu variant without your patch to prevent the setup_data clobbering so hpa's fix can't help there. But since the kernel doesn't do this now, and the 62MiB bug also seems to apply to existing kernels, for the purposes of QEMU for now, I think the v3 patch is probably best, since it'll handle existing kernels. Right, we can't fix all those guests which are out there. Alternatively, setup_data could be relocated, the boot param protocol could be bumped, and then QEMU could conditionalized it's use of setup_data based on that protocol version. That'd work, but seems a bit more involved. I think this is at least worth considering because the kernel overwriting setup_data after having been told where that setup_data is located is not really nice. Well not explicitly at least - it gets the pointer to the first element and something needs to traverse that list to know which addresses are live. But still, that info is there so perhaps we need to take setup_data into consideration too before decompressing... As far as the decompression itself goes, it should only a problem if we are using physical KASLR since otherwise the kernel has a guaranteed safe zone already allocated by the boot loader. However, if physical KASLR is in use, then the decompressor needs to know everything there is to know about the memory map. However, there also seems to be some kind of interaction with AMD SEV-SNP. The bug appears to have been introduced by: b57feed2cc2622ae14b2fa62f19e973e5e0a60cf x86/compressed/64: Add identity mappings for setup_data entries https://lore.kernel.org/r/tycpr01mb694815cd815e98945f63c99183...@tycpr01mb6948.jpnprd01.prod.outlook.com ... which was included in version 5.19, so it is relatively recent. For a small amount of setup_data, the solution of just putting it next to the command line makes a lot of sense, and should be safe indefinitely. -hpa
RE: [PATCH v2 02/11] hw/watchdog/wdt_aspeed: Extend MMIO range to cover more registers
> When booting the Zephyr demo in [1] we get: > > aspeed.io: unimplemented device write (size 4, offset 0x185128, value > 0x030f1ff1) <-- > aspeed.io: unimplemented device write (size 4, offset 0x18512c, value > 0x03f1) > > This corresponds to this Zephyr code [2]: > > static int aspeed_wdt_init(const struct device *dev) > { > const struct aspeed_wdt_config *config = dev->config; > struct aspeed_wdt_data *const data = dev->data; > uint32_t reg_val; > > /* disable WDT by default */ > reg_val = sys_read32(config->ctrl_base + WDT_CTRL_REG); > reg_val &= ~WDT_CTRL_ENABLE; > sys_write32(reg_val, config->ctrl_base + WDT_CTRL_REG); > > sys_write32(data->rst_mask1, > config->ctrl_base + WDT_SW_RESET_MASK1_REG); <-- > sys_write32(data->rst_mask2, > config->ctrl_base + WDT_SW_RESET_MASK2_REG); > > return 0; > } > > The register definitions are [3]: > > #define WDT_RELOAD_VAL_REG 0x0004 > #define WDT_RESTART_REG 0x0008 > #define WDT_CTRL_REG0x000C > #define WDT_TIMEOUT_STATUS_REG 0x0010 > #define WDT_TIMEOUT_STATUS_CLR_REG 0x0014 > #define WDT_RESET_MASK1_REG 0x001C > #define WDT_RESET_MASK2_REG 0x0020 > #define WDT_SW_RESET_MASK1_REG 0x0028 <-- > #define WDT_SW_RESET_MASK2_REG 0x002C > #define WDT_SW_RESET_CTRL_REG 0x0024 > > Currently QEMU only cover a MMIO region of size 0x20: > > #define ASPEED_WDT_REGS_MAX(0x20 / 4) > > Change to map the whole 'iosize' which might be bigger, covering the other The root cause is that ASPEED_WDT_REGS_MAX is too small, right? Probably the Qemu is emulating an old version of the hardware. Given the meaning of ASPEED_WDT_REGS_MAX, it should be larger than iosize, not? Probably ASPEED_WDT_REGS_MAX should be per device type (aspeed_2400/2500), while iosize is for all devices, and its initial value comes from the per device type REGS_MAX. > registers. The MemoryRegionOps read/write handlers will report the accesses > as out-of-bounds guest-errors, but the next commit will report them as > unimplemented. > > [1] https://github.com/AspeedTech-BMC/zephyr/releases/tag/v00.01.07 > [2] https://github.com/AspeedTech-BMC/zephyr/commit/2e99f10ac27b > [3] https://github.com/AspeedTech- > BMC/zephyr/blob/v00.01.08/drivers/watchdog/wdt_aspeed.c#L31 > > Reviewed-by: Peter Delevoryas > Signed-off-by: Philippe Mathieu-Daudé > --- > hw/watchdog/wdt_aspeed.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/hw/watchdog/wdt_aspeed.c b/hw/watchdog/wdt_aspeed.c index > 958725a1b5..eefca31ae4 100644 > --- a/hw/watchdog/wdt_aspeed.c > +++ b/hw/watchdog/wdt_aspeed.c > @@ -260,6 +260,7 @@ static void aspeed_wdt_realize(DeviceState *dev, > Error **errp) { > SysBusDevice *sbd = SYS_BUS_DEVICE(dev); > AspeedWDTState *s = ASPEED_WDT(dev); > +AspeedWDTClass *awc = ASPEED_WDT_GET_CLASS(dev); > > assert(s->scu); > > @@ -271,7 +272,7 @@ static void aspeed_wdt_realize(DeviceState *dev, > Error **errp) > s->pclk_freq = PCLK_HZ; > > memory_region_init_io(&s->iomem, OBJECT(s), &aspeed_wdt_ops, s, > - TYPE_ASPEED_WDT, ASPEED_WDT_REGS_MAX * 4); > + TYPE_ASPEED_WDT, awc->iosize); > sysbus_init_mmio(sbd, &s->iomem); > } > > -- > 2.38.1 >
RE: [PATCH v2 01/11] hw/watchdog/wdt_aspeed: Rename MMIO region size as 'iosize'
> > Avoid confusing two different things: > - the WDT I/O region size ('iosize') > - at which offset the SoC map the WDT ('offset') While it is often the same, > we > can map smaller region sizes at larger offsets. > > Here we are interested in the I/O region size, so rename as 'iosize'. > > Reviewed-by: Peter Delevoryas > Signed-off-by: Philippe Mathieu-Daudé > --- > hw/arm/aspeed_ast10x0.c | 2 +- > hw/arm/aspeed_ast2600.c | 2 +- > hw/arm/aspeed_soc.c | 2 +- > hw/watchdog/wdt_aspeed.c | 8 > include/hw/watchdog/wdt_aspeed.h | 2 +- > 5 files changed, 8 insertions(+), 8 deletions(-) > > diff --git a/hw/arm/aspeed_ast10x0.c b/hw/arm/aspeed_ast10x0.c index > 4d0b9b115f..122b3fd3f3 100644 > --- a/hw/arm/aspeed_ast10x0.c > +++ b/hw/arm/aspeed_ast10x0.c > @@ -325,7 +325,7 @@ static void aspeed_soc_ast1030_realize(DeviceState > *dev_soc, Error **errp) > return; > } > aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->wdt[i]), 0, > -sc->memmap[ASPEED_DEV_WDT] + i * awc->offset); > +sc->memmap[ASPEED_DEV_WDT] + i * awc->iosize); How does the specification say here (I didn't find it)? I read this is for a case where multiple WDTs are implemented. And offset means the distance io registers of WDT[1] are located from WDT[0]. Or does the spec explicitly says the io registers of WDT[1] is located right after WDT[0] without any gaps in this case, iosize is better)? > } > > /* GPIO */ > diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c index > cd75465c2b..a79e05ddbd 100644 > --- a/hw/arm/aspeed_ast2600.c > +++ b/hw/arm/aspeed_ast2600.c > @@ -472,7 +472,7 @@ static void aspeed_soc_ast2600_realize(DeviceState > *dev, Error **errp) > return; > } > aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->wdt[i]), 0, > -sc->memmap[ASPEED_DEV_WDT] + i * awc->offset); > +sc->memmap[ASPEED_DEV_WDT] + i * awc->iosize); > } > > /* RAM */ > diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c index > b05b9dd416..2c0924d311 100644 > --- a/hw/arm/aspeed_soc.c > +++ b/hw/arm/aspeed_soc.c > @@ -393,7 +393,7 @@ static void aspeed_soc_realize(DeviceState *dev, > Error **errp) > return; > } > aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->wdt[i]), 0, > -sc->memmap[ASPEED_DEV_WDT] + i * awc->offset); > +sc->memmap[ASPEED_DEV_WDT] + i * awc->iosize); > } > > /* RAM */ > diff --git a/hw/watchdog/wdt_aspeed.c b/hw/watchdog/wdt_aspeed.c index > d753693a2e..958725a1b5 100644 > --- a/hw/watchdog/wdt_aspeed.c > +++ b/hw/watchdog/wdt_aspeed.c > @@ -309,7 +309,7 @@ static void aspeed_2400_wdt_class_init(ObjectClass > *klass, void *data) > AspeedWDTClass *awc = ASPEED_WDT_CLASS(klass); > > dc->desc = "ASPEED 2400 Watchdog Controller"; > -awc->offset = 0x20; > +awc->iosize = 0x20; > awc->ext_pulse_width_mask = 0xff; > awc->reset_ctrl_reg = SCU_RESET_CONTROL1; > awc->wdt_reload = aspeed_wdt_reload; @@ -346,7 +346,7 @@ static void > aspeed_2500_wdt_class_init(ObjectClass *klass, void *data) > AspeedWDTClass *awc = ASPEED_WDT_CLASS(klass); > > dc->desc = "ASPEED 2500 Watchdog Controller"; > -awc->offset = 0x20; > +awc->iosize = 0x20; > awc->ext_pulse_width_mask = 0xf; > awc->reset_ctrl_reg = SCU_RESET_CONTROL1; > awc->reset_pulse = aspeed_2500_wdt_reset_pulse; @@ -369,7 +369,7 > @@ static void aspeed_2600_wdt_class_init(ObjectClass *klass, void *data) > AspeedWDTClass *awc = ASPEED_WDT_CLASS(klass); > > dc->desc = "ASPEED 2600 Watchdog Controller"; > -awc->offset = 0x40; > +awc->iosize = 0x40; > awc->ext_pulse_width_mask = 0xf; /* TODO */ > awc->reset_ctrl_reg = AST2600_SCU_RESET_CONTROL1; > awc->reset_pulse = aspeed_2500_wdt_reset_pulse; @@ -392,7 +392,7 > @@ static void aspeed_1030_wdt_class_init(ObjectClass *klass, void *data) > AspeedWDTClass *awc = ASPEED_WDT_CLASS(klass); > > dc->desc = "ASPEED 1030 Watchdog Controller"; > -awc->offset = 0x80; > +awc->iosize = 0x80; > awc->ext_pulse_width_mask = 0xf; /* TODO */ > awc->reset_ctrl_reg = AST2600_SCU_RESET_CONTROL1; > awc->reset_pulse = aspeed_2500_wdt_reset_pulse; diff --git > a/include/hw/watchdog/wdt_aspeed.h > b/include/hw/watchdog/wdt_aspeed.h > index dfa5dfa424..db91ee6b51 100644 > --- a/include/hw/watchdog/wdt_aspeed.h > +++ b/include/hw/watchdog/wdt_aspeed.h > @@ -40,7 +40,7 @@ struct AspeedWDTState { struct AspeedWDTClass { > SysBusDeviceClass parent_class; > > -uint32_t offset; > +uint32_t iosize; > uint32_t ext_pulse_width_mask; > uint32_t reset_ctrl_reg; > void (*reset_pulse)(AspeedWDTState *s, uint32_t property); > -- > 2.38.1 >
Re: Question about TLBs
Nada Lachtar writes: > [[S/MIME Signed Part:Undecided]] > Hello, > > Does Qemu maintain two TLB for the x86_64 system (i.e iTLB and dTLB)? > If yes, can you please point me to how to access the dTLB and what > data structure maintains this information! Qemu's internal softmmu TLB is unified and holds information for code loads, memory accesses and re-directions for io access. See CPUTLBEntry and CPUTLBEntryFull. They do not directly map onto any target specific structures (although targets can use TARGET_PAGE_ENTRY_EXTRA to store extra information in the table). These entries are filled by target specific code via cpu->cc->tcg_ops->tlb_fill() which in the x86 case takes you to x86_cpu_tlb_fill(). This is the code responsible to walking the target page tables and filling in the details of the mappings. > > I would appreciate any help, > > > Thanks, > > > > > [[End of S/MIME Signed Part]] -- Alex Bennée Virtualisation Tech Lead @ Linaro
Re: [PATCH qemu] x86: don't let decompressed kernel image clobber setup_data
On Sat, Dec 31, 2022 at 07:22:47PM +0100, Jason A. Donenfeld wrote: > So with that understanding confirmed, I'm confused at your surprise that > hpa's unrelated fix to the different issue didn't fix this issue. No surprise there - I used a qemu variant without your patch to prevent the setup_data clobbering so hpa's fix can't help there. > But since the kernel doesn't do this now, and the 62MiB bug also seems > to apply to existing kernels, for the purposes of QEMU for now, I think > the v3 patch is probably best, since it'll handle existing kernels. Right, we can't fix all those guests which are out there. > Alternatively, setup_data could be relocated, the boot param protocol > could be bumped, and then QEMU could conditionalized it's use of > setup_data based on that protocol version. That'd work, but seems a bit > more involved. I think this is at least worth considering because the kernel overwriting setup_data after having been told where that setup_data is located is not really nice. Well not explicitly at least - it gets the pointer to the first element and something needs to traverse that list to know which addresses are live. But still, that info is there so perhaps we need to take setup_data into consideration too before decompressing... -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH qemu] x86: don't let decompressed kernel image clobber setup_data
On Sat, Dec 31, 2022 at 03:24:32PM +0100, Borislav Petkov wrote: > On Sat, Dec 31, 2022 at 02:51:28PM +0100, Jason A. Donenfeld wrote: > > That failure is unrelated to the ident mapping issue Peter and > > I discussed. The original failure is described in the commit message: > > decompression clobbers the data, so sd->next points to garbage. > > Right So with that understanding confirmed, I'm confused at your surprise that hpa's unrelated fix to the different issue didn't fix this issue. > and the fact that the kernel overwrites it still feels kinda wrong: the > kernel knows where setup_data is - the address is in the setup header so > *actually*, it should take care of not to clobber it. Yea, technically the bootloader could relocate all the setup_data links by copying them and updating ->next. This wouldn't be so hard to do. (Special care would have to be taken, though, to zero out SETUP_RNG_SEED, though, for forward secrecy and such.) But since the kernel doesn't do this now, and the 62MiB bug also seems to apply to existing kernels, for the purposes of QEMU for now, I think the v3 patch is probably best, since it'll handle existing kernels. Alternatively, setup_data could be relocated, the boot param protocol could be bumped, and then QEMU could conditionalized it's use of setup_data based on that protocol version. That'd work, but seems a bit more involved. So maybe for now, v3 works? Hopefully that looks like a correct approach to hpa, anyhow: https://lore.kernel.org/lkml/20221230220725.618763-1-ja...@zx2c4.com/ I think it should fit with what he described would work. Jason
Re: [PATCH qemu] x86: don't let decompressed kernel image clobber setup_data
On Sat, Dec 31, 2022 at 02:51:28PM +0100, Jason A. Donenfeld wrote: > That failure is unrelated to the ident mapping issue Peter and > I discussed. The original failure is described in the commit message: > decompression clobbers the data, so sd->next points to garbage. Right, and the fact that the kernel overwrites it still feels kinda wrong: the kernel knows where setup_data is - the address is in the setup header so *actually*, it should take care of not to clobber it. But hpa would correct me if I'm missing an aspect about whose responsibility it is to do what here... Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PULL 17/21] target/hexagon: prepare input for the idef-parser
On Thu, 29 Dec 2022 11:41:22 +0100 Thomas Huth wrote: > I'm now seeing changes to scripts/meson-buildoptions.sh after > rebuilding QEMU ... looks like you likely forgot to update that file > with the automatic update after changing meson_options.txt ? Ah, there seems to be another couple of unrelated missing updates to scripts/meson-buildoptions.sh. `Makefile` seems to rely on modification dates of files in the source tree, but git does not track those. git clone ... mkdir build ./configure ... make update-buildoptions # Does nothing touch ../meson_options.txt make update-buildoptions # Actually makes changes Also, AFAIU the `update-buildoptions` target is not available in ninja. However maybe this is not a bug since the doc says to use make. Anyway, patch coming soon! -- Alessandro Di Federico rev.ng Labs
Re: [PATCH 1/1] target/hexagon: work around unused variable in yyparser
On Sat, 31 Dec 2022 17:19:35 +0800 Zongyuan Li wrote: > Variable 'yynerrs' is recognized as unused variable in clang15, This issue is already handled by another similar patch (target/hexagon: suppress unused variable warning) that will soon end up in a pull request. Thanks for looking into this though. -- Alessandro Di Federico rev.ng Labs
Re: [PATCH qemu] x86: don't let decompressed kernel image clobber setup_data
On Sat, Dec 31, 2022 at 02:48:12PM +0100, Borislav Petkov wrote: > On Sat, Dec 31, 2022 at 02:44:08PM +0100, Jason A. Donenfeld wrote: > > Are you using patch v1 minus the 62 MiB thing? > > No, I want to see the original failure - the one that prompted you to send > > https://lore.kernel.org/r/20221228143831.396245-1-ja...@zx2c4.com That failure is unrelated to the ident mapping issue Peter and I discussed. The original failure is described in the commit message: decompression clobbers the data, so sd->next points to garbage.
Re: [PATCH qemu] x86: don't let decompressed kernel image clobber setup_data
On Sat, Dec 31, 2022 at 02:44:08PM +0100, Jason A. Donenfeld wrote: > Are you using patch v1 minus the 62 MiB thing? No, I want to see the original failure - the one that prompted you to send https://lore.kernel.org/r/20221228143831.396245-1-ja...@zx2c4.com -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v2 2/3] hw/mips/malta: Set PIIX4 IRQ routes in embedded bootloader
On 31/12/22 10:53, Bernhard Beschow wrote: Am 21. November 2022 15:34:05 UTC schrieb Bernhard Beschow : Am 27. Oktober 2022 20:47:19 UTC schrieb "Philippe Mathieu-Daudé" : Linux kernel expects the northbridge & southbridge chipsets configured by the BIOS firmware. We emulate that by writing a tiny bootloader code in write_bootloader(). Upon introduction in commit 5c2b87e34d ("PIIX4 support"), the PIIX4 configuration space included values specific to the Malta board. Set the Malta-specific IRQ routing values in the embedded bootloader, so the next commit can remove the Malta specific bits from the PIIX4 PCI-ISA bridge and make it generic (matching the real hardware). Signed-off-by: Philippe Mathieu-Daudé --- FIXME: Missing the nanoMIPS counter-part! Who will be taking care of this? I have absolutely no clue how the write_bootloader functions work, so I don't see how to fix it. Ping This comment has been taken care of: https://lore.kernel.org/qemu-devel/a3c3f639-dbb1-88a7-43fe-547a234c5...@linaro.org/ However while testing the MIPS pull request I prepared I found a bug in the GT64120 which I'm trying to fix since various days... Unfortunately your series depends on it, so this is a blocking issue. Sorry for this long delay... Phil.
Re: [PATCH qemu] x86: don't let decompressed kernel image clobber setup_data
On Sat, Dec 31, 2022 at 02:40:59PM +0100, Borislav Petkov wrote: > On Fri, Dec 30, 2022 at 05:06:55PM -0800, H. Peter Anvin wrote: > > This needs to be something like: > > > > kernel_add_identity_map(sd_addr, sd_addr + sizeof(*sd)); > > kernel_add_identity_map(sd_addr + sizeof(*sd), > > sd_addr + sizeof(*sd) + sd->len); > > It still #PFs with that: > > (gdb) bt > #0 0x84738576 in native_halt () at > ./arch/x86/include/asm/irqflags.h:57 > #1 halt () at ./arch/x86/include/asm/irqflags.h:98 > #2 early_fixup_exception (regs=regs@entry=0x84007dc8, > trapnr=trapnr@entry=14) at arch/x86/mm/extable.c:340 > #3 0x846ff465 in do_early_exception (regs=0x84007dc8, > trapnr=14) at arch/x86/kernel/head64.c:424 > #4 0x846ff14f in early_idt_handler_common () at > arch/x86/kernel/head_64.S:483 > #5 0xc149f9894908788d in ?? () > #6 0xff2003fc in ?? () > #7 0x0010 in fixed_percpu_data () > #8 0xdc00 in ?? () > #9 0x84007ea8 in init_thread_union () > #10 0xff20088d in ?? () > #11 0x in ?? () > > /me goes to dig more. Are you using patch v1 minus the 62 MiB thing? If you haven't applied patch v1 and then removed the 62 MiB limitation in it, then you've misunderstood the conversation again. Please see my reproduction steps to Peter: https://lore.kernel.org/lkml/y68k4mpuz6edq...@zx2c4.com/ Jason
Re: [PATCH qemu] x86: don't let decompressed kernel image clobber setup_data
On Sat, Dec 31, 2022 at 02:35:45PM +0100, Borislav Petkov wrote: > On Sat, Dec 31, 2022 at 01:54:50PM +0100, Jason A. Donenfeld wrote: > > Nothing special... `-kernel bzImage` should be enough to do it. Eric > > reported it, and then I was able to repro trivially. Sure you got the > > right version? > > Yeah, qemu executables confusion here - had wrongly something older of the > version 7.1... > > Now made sure I'm actually booting with the latest qemu: > > QEMU emulator version 7.2.50 (v7.2.0-333-g222059a0fccf) > > With that the kernel with your config hangs early during boot and the stack > trace is below. > > Seeing how it says trapnr 14, then that looks like something you are seeing. > > But lemme poke at it more. Yes. The cause is what I've described in the commit message. There are two proposed fixes, the v1, which has the 62 MiB limitation due to a kernel bug, and the v3, which seems to work fine, and is simpler, and I suspect that's the one QEMU upstream should take. https://lore.kernel.org/lkml/20221230220725.618763-1-ja...@zx2c4.com/
Re: [PATCH qemu] x86: don't let decompressed kernel image clobber setup_data
On Fri, Dec 30, 2022 at 05:06:55PM -0800, H. Peter Anvin wrote: > This needs to be something like: > > kernel_add_identity_map(sd_addr, sd_addr + sizeof(*sd)); > kernel_add_identity_map(sd_addr + sizeof(*sd), > sd_addr + sizeof(*sd) + sd->len); It still #PFs with that: (gdb) bt #0 0x84738576 in native_halt () at ./arch/x86/include/asm/irqflags.h:57 #1 halt () at ./arch/x86/include/asm/irqflags.h:98 #2 early_fixup_exception (regs=regs@entry=0x84007dc8, trapnr=trapnr@entry=14) at arch/x86/mm/extable.c:340 #3 0x846ff465 in do_early_exception (regs=0x84007dc8, trapnr=14) at arch/x86/kernel/head64.c:424 #4 0x846ff14f in early_idt_handler_common () at arch/x86/kernel/head_64.S:483 #5 0xc149f9894908788d in ?? () #6 0xff2003fc in ?? () #7 0x0010 in fixed_percpu_data () #8 0xdc00 in ?? () #9 0x84007ea8 in init_thread_union () #10 0xff20088d in ?? () #11 0x in ?? () /me goes to dig more. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH qemu] x86: don't let decompressed kernel image clobber setup_data
On Sat, Dec 31, 2022 at 01:54:50PM +0100, Jason A. Donenfeld wrote: > Nothing special... `-kernel bzImage` should be enough to do it. Eric > reported it, and then I was able to repro trivially. Sure you got the > right version? Yeah, qemu executables confusion here - had wrongly something older of the version 7.1... Now made sure I'm actually booting with the latest qemu: QEMU emulator version 7.2.50 (v7.2.0-333-g222059a0fccf) With that the kernel with your config hangs early during boot and the stack trace is below. Seeing how it says trapnr 14, then that looks like something you are seeing. But lemme poke at it more. #0 0x84738576 in native_halt () at ./arch/x86/include/asm/irqflags.h:57 #1 halt () at ./arch/x86/include/asm/irqflags.h:98 #2 early_fixup_exception (regs=regs@entry=0x84007dc8, trapnr=trapnr@entry=14) at arch/x86/mm/extable.c:340 #3 0x846ff465 in do_early_exception (regs=0x84007dc8, trapnr=14) at arch/x86/kernel/head64.c:424 #4 0x846ff14f in early_idt_handler_common () at arch/x86/kernel/head_64.S:483 #5 0x2404c7410cd0 in ?? () #6 0xff20073c in ?? () #7 0x0010 in fixed_percpu_data () #8 0xdc00 in ?? () #9 0x84007ea8 in init_thread_union () #10 0xff200cd0 in ?? () #11 0x in ?? () -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH qemu] x86: don't let decompressed kernel image clobber setup_data
On Fri, Dec 30, 2022 at 05:06:55PM -0800, H. Peter Anvin wrote: > > > On 12/30/22 14:10, Jason A. Donenfeld wrote: > > On Fri, Dec 30, 2022 at 01:58:39PM -0800, H. Peter Anvin wrote: > >> See the other thread fork. They have identified the problem already. > > > > Not sure I follow. Is there another thread where somebody worked out why > > this 62meg limit was happening? > > > > Note that I sent v2/v3, to fix the original problem in a different way, > > and if that looks good to the QEMU maintainers, then we can all be happy > > with that. But I *haven't* addressed and still don't fully understand > > why the 62meg limit applied to my v1 in the way it does. Did you find a > > bug there to fix? If so, please do CC me. > > > > Yes, you yourself posted the problem: > > > Then build qemu. Run it with `-kernel bzImage`, based on the kernel > > built with the .config I attached. > > > > You'll see that the CPU triple faults when hitting this line: > > > > sd = (struct setup_data *)boot_params->hdr.setup_data; > > while (sd) { > > unsigned long sd_addr = (unsigned long)sd; > > > > kernel_add_identity_map(sd_addr, sd_addr + sizeof(*sd) + > > sd->len); < > > sd = (struct setup_data *)sd->next; > > } > > > > , because it dereferences *sd. This does not happen if the decompressed > > size of the kernel is < 62 megs. > > > > So that's the "big and pretty serious" bug that might be worthy of > > investigation. > > This needs to be something like: > > kernel_add_identity_map(sd_addr, sd_addr + sizeof(*sd)); > kernel_add_identity_map(sd_addr + sizeof(*sd), > sd_addr + sizeof(*sd) + sd->len); > Oh, right, duh. Thanks for spelling it out. Jason
Re: [PATCH qemu] x86: don't let decompressed kernel image clobber setup_data
On Sat, Dec 31, 2022 at 10:48:16AM +0100, Borislav Petkov wrote: > On Fri, Dec 30, 2022 at 04:59:30PM +0100, Jason A. Donenfeld wrote: > > I'll attach a .config file. Apply the patch at the top of this thread to > > qemu, > > Hmm, so the patch at the top of this thread is fixing the clobbering of > setup_data. > > But I tried latest qemu with your .config and it boots ok here. So how do I > repro the original issue you're trying to fix? Nothing special... `-kernel bzImage` should be enough to do it. Eric reported it, and then I was able to repro trivially. Sure you got the right version? Jason
Re: qemu-system-i386: Could not install MSR_CORE_THREAD_COUNT handler: Success
Alexander, On Sat, Dec 31, 2022 at 12:34:45PM +0100, Alexander Graf wrote: > On 31.12.22 11:17, Vitaly Chikunov wrote: > > On Sat, Dec 31, 2022 at 10:28:21AM +0100, Alexander Graf wrote: > > > On 30.12.22 19:16, Vitaly Chikunov wrote: > > > > On Fri, Dec 30, 2022 at 06:44:14PM +0100, Alexander Graf wrote: > > > > > This is a kvm kernel bug and should be fixed with the latest stable > > > > > releases. Which kernel version are you running? > > > > This is on latest v6.0 stable - 6.0.15. > > > > > > > > Maybe there could be workaround for such situations? (Or maybe it's > > > > possible to make this error non-fatal?) We use qemu+kvm for testing and > > > > now we cannot test on x86. > > > I'm confused what's going wrong for you. I tried to reproduce the issue > > > locally, but am unable to: > > > > > > $ uname -a > > > Linux server 6.0.15-default #1 SMP PREEMPT_DYNAMIC Sat Dec 31 07:52:52 CET > > > 2022 x86_64 x86_64 x86_64 GNU/Linux > > > $ linux32 chroot . > > > $ uname -a > > > Linux server 6.0.15-default #1 SMP PREEMPT_DYNAMIC Sat Dec 31 07:52:52 CET > > > 2022 i686 GNU/Linux > > > $ cd qemu > > > $ file ./build/qemu-system-i386 > > > ./build/qemu-system-i386: ELF 32-bit LSB shared object, Intel 80386, > > > version > > > 1 (SYSV), dynamically linked, interpreter /lib/ld-linux.so.2, for > > > GNU/Linux > > > 3.2.0, BuildID[sha1]=f75e20572be5c604c121de4497397665c168aa4c, with > > > debug_info, not stripped > > > $ ./build/qemu-system-i386 --version > > > QEMU emulator version 7.2.0 (v7.2.0-dirty) > > > Copyright (c) 2003-2022 Fabrice Bellard and the QEMU Project developers > > > $ ./build/qemu-system-i386 -nographic -enable-kvm > > > SeaBIOS (version rel-1.16.1-0-g3208b098f51a-prebuilt.qemu.org) > > > [...] > > > > > > > > > Can you please double check whether your host kernel version is 6.0.15? > > > Please paste the output of "uname -a". > > Excuse me, I'm incorrectly reported kernel version I tried to boot instead > > of host one. Host kernels are quite old, 5.15.59 and even 5.17.15 -- > > where failure is occurring. > > > > I just tested on 5.15.85 and there is no failure. > > > Awesome, great to hear :). That means everything works as expected at least. > > > >builder@i586:/.in$ uname -a > >Linux localhost.localdomain 5.15.85-std-def-alt1 #1 SMP Wed Dec 21 > > 21:14:40 UTC 2022 i686 GNU/Linux > >builder@i586:/.in$ qemu-system-i386 -nographic -enable-kvm > >SeaBIOS (version 1.16.1-alt1) > > > > Perhaps, one of solutions it to reboot our build fleet to newer kernels. > > [This maybe hard, though, since special builder node image should be > > created and reboot shall be coordinated through all systems, in compare, > > updating QEMU would be easier since chroot is created on every build]. > > > I understand that it may be slightly painful to update your build fleet, but > given this is a genuine kernel bug that has a fix available upstream and it > only happens on niche corner cases (i386 QEMU on x86-64 Linux kernels with > the bug) that I doubt anyone will use in production, I'd prefer we keep the > QEMU logic as is :). > > In the meanwhile, while you're patching the build fleet, you can apply the > patch below as part of your build process to ensure you don't fail due to > the kernel bug. Just make sure to remove it again as soon as you're done > with the fleet update :). Thanks for the suggestions. Best wishes for the New Year! Vitaly, > > diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c > index a213209379..b9396bc7a6 100644 > --- a/target/i386/kvm/kvm.c > +++ b/target/i386/kvm/kvm.c > @@ -2632,7 +2632,11 @@ int kvm_arch_init(MachineState *ms, KVMState *s) > return ret; > } > } > +#ifdef __x86_64__ > if (kvm_vm_check_extension(s, KVM_CAP_X86_USER_SPACE_MSR)) { > +#else > + if (0) { > +#endif > bool r; > > ret = kvm_vm_enable_cap(s, KVM_CAP_X86_USER_SPACE_MSR, 0, > > Alex >
Re: qemu-system-i386: Could not install MSR_CORE_THREAD_COUNT handler: Success
Hi Vitaly, On 31.12.22 11:17, Vitaly Chikunov wrote: Alexander, On Sat, Dec 31, 2022 at 10:28:21AM +0100, Alexander Graf wrote: On 30.12.22 19:16, Vitaly Chikunov wrote: On Fri, Dec 30, 2022 at 06:44:14PM +0100, Alexander Graf wrote: This is a kvm kernel bug and should be fixed with the latest stable releases. Which kernel version are you running? This is on latest v6.0 stable - 6.0.15. Maybe there could be workaround for such situations? (Or maybe it's possible to make this error non-fatal?) We use qemu+kvm for testing and now we cannot test on x86. I'm confused what's going wrong for you. I tried to reproduce the issue locally, but am unable to: $ uname -a Linux server 6.0.15-default #1 SMP PREEMPT_DYNAMIC Sat Dec 31 07:52:52 CET 2022 x86_64 x86_64 x86_64 GNU/Linux $ linux32 chroot . $ uname -a Linux server 6.0.15-default #1 SMP PREEMPT_DYNAMIC Sat Dec 31 07:52:52 CET 2022 i686 GNU/Linux $ cd qemu $ file ./build/qemu-system-i386 ./build/qemu-system-i386: ELF 32-bit LSB shared object, Intel 80386, version 1 (SYSV), dynamically linked, interpreter /lib/ld-linux.so.2, for GNU/Linux 3.2.0, BuildID[sha1]=f75e20572be5c604c121de4497397665c168aa4c, with debug_info, not stripped $ ./build/qemu-system-i386 --version QEMU emulator version 7.2.0 (v7.2.0-dirty) Copyright (c) 2003-2022 Fabrice Bellard and the QEMU Project developers $ ./build/qemu-system-i386 -nographic -enable-kvm SeaBIOS (version rel-1.16.1-0-g3208b098f51a-prebuilt.qemu.org) [...] Can you please double check whether your host kernel version is 6.0.15? Please paste the output of "uname -a". Excuse me, I'm incorrectly reported kernel version I tried to boot instead of host one. Host kernels are quite old, 5.15.59 and even 5.17.15 -- where failure is occurring. I just tested on 5.15.85 and there is no failure. Awesome, great to hear :). That means everything works as expected at least. builder@i586:/.in$ uname -a Linux localhost.localdomain 5.15.85-std-def-alt1 #1 SMP Wed Dec 21 21:14:40 UTC 2022 i686 GNU/Linux builder@i586:/.in$ qemu-system-i386 -nographic -enable-kvm SeaBIOS (version 1.16.1-alt1) Perhaps, one of solutions it to reboot our build fleet to newer kernels. [This maybe hard, though, since special builder node image should be created and reboot shall be coordinated through all systems, in compare, updating QEMU would be easier since chroot is created on every build]. I understand that it may be slightly painful to update your build fleet, but given this is a genuine kernel bug that has a fix available upstream and it only happens on niche corner cases (i386 QEMU on x86-64 Linux kernels with the bug) that I doubt anyone will use in production, I'd prefer we keep the QEMU logic as is :). In the meanwhile, while you're patching the build fleet, you can apply the patch below as part of your build process to ensure you don't fail due to the kernel bug. Just make sure to remove it again as soon as you're done with the fleet update :). diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c index a213209379..b9396bc7a6 100644 --- a/target/i386/kvm/kvm.c +++ b/target/i386/kvm/kvm.c @@ -2632,7 +2632,11 @@ int kvm_arch_init(MachineState *ms, KVMState *s) return ret; } } +#ifdef __x86_64__ if (kvm_vm_check_extension(s, KVM_CAP_X86_USER_SPACE_MSR)) { +#else + if (0) { +#endif bool r; ret = kvm_vm_enable_cap(s, KVM_CAP_X86_USER_SPACE_MSR, 0, Alex
Re: qemu-system-i386: Could not install MSR_CORE_THREAD_COUNT handler: Success
Alexander, On Sat, Dec 31, 2022 at 10:28:21AM +0100, Alexander Graf wrote: > On 30.12.22 19:16, Vitaly Chikunov wrote: > > On Fri, Dec 30, 2022 at 06:44:14PM +0100, Alexander Graf wrote: > > > > > > This is a kvm kernel bug and should be fixed with the latest stable > > > releases. Which kernel version are you running? > > This is on latest v6.0 stable - 6.0.15. > > > > Maybe there could be workaround for such situations? (Or maybe it's > > possible to make this error non-fatal?) We use qemu+kvm for testing and > > now we cannot test on x86. > > I'm confused what's going wrong for you. I tried to reproduce the issue > locally, but am unable to: > > $ uname -a > Linux server 6.0.15-default #1 SMP PREEMPT_DYNAMIC Sat Dec 31 07:52:52 CET > 2022 x86_64 x86_64 x86_64 GNU/Linux > $ linux32 chroot . > $ uname -a > Linux server 6.0.15-default #1 SMP PREEMPT_DYNAMIC Sat Dec 31 07:52:52 CET > 2022 i686 GNU/Linux > $ cd qemu > $ file ./build/qemu-system-i386 > ./build/qemu-system-i386: ELF 32-bit LSB shared object, Intel 80386, version > 1 (SYSV), dynamically linked, interpreter /lib/ld-linux.so.2, for GNU/Linux > 3.2.0, BuildID[sha1]=f75e20572be5c604c121de4497397665c168aa4c, with > debug_info, not stripped > $ ./build/qemu-system-i386 --version > QEMU emulator version 7.2.0 (v7.2.0-dirty) > Copyright (c) 2003-2022 Fabrice Bellard and the QEMU Project developers > $ ./build/qemu-system-i386 -nographic -enable-kvm > SeaBIOS (version rel-1.16.1-0-g3208b098f51a-prebuilt.qemu.org) > [...] > > > Can you please double check whether your host kernel version is 6.0.15? > Please paste the output of "uname -a". Excuse me, I'm incorrectly reported kernel version I tried to boot instead of host one. Host kernels are quite old, 5.15.59 and even 5.17.15 -- where failure is occurring. I just tested on 5.15.85 and there is no failure. builder@i586:/.in$ uname -a Linux localhost.localdomain 5.15.85-std-def-alt1 #1 SMP Wed Dec 21 21:14:40 UTC 2022 i686 GNU/Linux builder@i586:/.in$ qemu-system-i386 -nographic -enable-kvm SeaBIOS (version 1.16.1-alt1) Perhaps, one of solutions it to reboot our build fleet to newer kernels. [This maybe hard, though, since special builder node image should be created and reboot shall be coordinated through all systems, in compare, updating QEMU would be easier since chroot is created on every build]. Thanks for your helpful replies! Thanks, > > > Alex
Re: [PATCH v2 2/3] hw/mips/malta: Set PIIX4 IRQ routes in embedded bootloader
Am 21. November 2022 15:34:05 UTC schrieb Bernhard Beschow : > > >Am 27. Oktober 2022 20:47:19 UTC schrieb "Philippe Mathieu-Daudé" >: >>Linux kernel expects the northbridge & southbridge chipsets >>configured by the BIOS firmware. We emulate that by writing >>a tiny bootloader code in write_bootloader(). >> >>Upon introduction in commit 5c2b87e34d ("PIIX4 support"), >>the PIIX4 configuration space included values specific to >>the Malta board. >> >>Set the Malta-specific IRQ routing values in the embedded >>bootloader, so the next commit can remove the Malta specific >>bits from the PIIX4 PCI-ISA bridge and make it generic >>(matching the real hardware). >> >>Signed-off-by: Philippe Mathieu-Daudé >>--- >>FIXME: Missing the nanoMIPS counter-part! > >Who will be taking care of this? I have absolutely no clue how the >write_bootloader functions work, so I don't see how to fix it. Ping >Couldn't we just do it like in pegasos2_init() where the registers are >initialized by QEMU directly if there is no bootloader binary configured? I >could do that. > >Best regards, >Bernhard >>--- >> hw/mips/malta.c | 19 +++ >> 1 file changed, 19 insertions(+) >> >>diff --git a/hw/mips/malta.c b/hw/mips/malta.c >>index df0f448b67..4403028778 100644 >>--- a/hw/mips/malta.c >>+++ b/hw/mips/malta.c >>@@ -804,6 +804,8 @@ static void write_bootloader_nanomips(uint8_t *base, >>uint64_t run_addr, >> stw_p(p++, 0x8422); stw_p(p++, 0x9088); >> /* sw t0, 0x88(t1) */ >> >>+/* TODO set PIIX IRQC[A:D] routing values! */ >>+ >> stw_p(p++, 0xe320 | NM_HI1(kernel_entry)); >> >> stw_p(p++, NM_HI2(kernel_entry)); >>@@ -841,6 +843,9 @@ static void write_bootloader_nanomips(uint8_t *base, >>uint64_t run_addr, >> static void write_bootloader(uint8_t *base, uint64_t run_addr, >> uint64_t kernel_entry) >> { >>+const char pci_pins_cfg[PCI_NUM_PINS] = { >>+10, 10, 11, 11 /* PIIX IRQRC[A:D] */ >>+}; >> uint32_t *p; >> >> /* Small bootloader */ >>@@ -915,6 +920,20 @@ static void write_bootloader(uint8_t *base, uint64_t >>run_addr, >> >> #undef cpu_to_gt32 >> >>+/* >>+ * The PIIX ISA bridge is on PCI bus 0 dev 10 func 0. >>+ * Load the PIIX IRQC[A:D] routing config address, then >>+ * write routing configuration to the config data register. >>+ */ >>+bl_gen_write_u32(&p, /* GT_PCI0_CFGADDR */ >>+ cpu_mips_phys_to_kseg1(NULL, 0x1be0 + 0xcf8), >>+ tswap32((1 << 31) /* ConfigEn */ >>+ | PCI_BUILD_BDF(0, PIIX4_PCI_DEVFN) << 8 >>+ | PIIX_PIRQCA)); >>+bl_gen_write_u32(&p, /* GT_PCI0_CFGDATA */ >>+ cpu_mips_phys_to_kseg1(NULL, 0x1be0 + 0xcfc), >>+ tswap32(ldl_be_p(pci_pins_cfg))); >>+ >> bl_gen_jump_kernel(&p, >>true, ENVP_VADDR - 64, >>/*
Re: [PATCH qemu] x86: don't let decompressed kernel image clobber setup_data
On Fri, Dec 30, 2022 at 04:59:30PM +0100, Jason A. Donenfeld wrote: > I'll attach a .config file. Apply the patch at the top of this thread to > qemu, Hmm, so the patch at the top of this thread is fixing the clobbering of setup_data. But I tried latest qemu with your .config and it boots ok here. So how do I repro the original issue you're trying to fix? Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
Question about TLBs
Hello, Does Qemu maintain two TLB for the x86_64 system (i.e iTLB and dTLB)? If yes, can you please point me to how to access the dTLB and what data structure maintains this information! I would appreciate any help, Thanks, smime.p7s Description: S/MIME cryptographic signature
Re: qemu-system-i386: Could not install MSR_CORE_THREAD_COUNT handler: Success
Hi Vitaly, On 30.12.22 19:16, Vitaly Chikunov wrote: Alexander, On Fri, Dec 30, 2022 at 06:44:14PM +0100, Alexander Graf wrote: Hi Vitaly, This is a kvm kernel bug and should be fixed with the latest stable releases. Which kernel version are you running? This is on latest v6.0 stable - 6.0.15. Maybe there could be workaround for such situations? (Or maybe it's possible to make this error non-fatal?) We use qemu+kvm for testing and now we cannot test on x86. I'm confused what's going wrong for you. I tried to reproduce the issue locally, but am unable to: $ uname -a Linux server 6.0.15-default #1 SMP PREEMPT_DYNAMIC Sat Dec 31 07:52:52 CET 2022 x86_64 x86_64 x86_64 GNU/Linux $ linux32 chroot . $ uname -a Linux server 6.0.15-default #1 SMP PREEMPT_DYNAMIC Sat Dec 31 07:52:52 CET 2022 i686 GNU/Linux $ cd qemu $ file ./build/qemu-system-i386 ./build/qemu-system-i386: ELF 32-bit LSB shared object, Intel 80386, version 1 (SYSV), dynamically linked, interpreter /lib/ld-linux.so.2, for GNU/Linux 3.2.0, BuildID[sha1]=f75e20572be5c604c121de4497397665c168aa4c, with debug_info, not stripped $ ./build/qemu-system-i386 --version QEMU emulator version 7.2.0 (v7.2.0-dirty) Copyright (c) 2003-2022 Fabrice Bellard and the QEMU Project developers $ ./build/qemu-system-i386 -nographic -enable-kvm SeaBIOS (version rel-1.16.1-0-g3208b098f51a-prebuilt.qemu.org) [...] Can you please double check whether your host kernel version is 6.0.15? Please paste the output of "uname -a". Alex
[PATCH 1/1] target/hexagon: work around unused variable in yyparser
Variable 'yynerrs' is recognized as unused variable in clang15, which is auto-generated by bison in parser file, as long as user code doesn't access it in '.y'. This is already fixed in bison 8.2. But for user who use latest clang, a simple harmless code piece would fix this building error. FYI: bison patch link https://mail.gnu.org/archive/html/bison-patches/2022-08/msg6.html Signed-off-by: Zongyuan Li --- target/hexagon/idef-parser/idef-parser.y | 3 +++ 1 file changed, 3 insertions(+) diff --git a/target/hexagon/idef-parser/idef-parser.y b/target/hexagon/idef-parser/idef-parser.y index 8be44a0ad1..07aa105aa2 100644 --- a/target/hexagon/idef-parser/idef-parser.y +++ b/target/hexagon/idef-parser/idef-parser.y @@ -99,6 +99,9 @@ /* Input file containing the description of each hexagon instruction */ input : instructions { + if (yynerrs != 0) { + YYABORT; + } YYACCEPT; } ; -- 2.38.1