[PATCH v2] MAINTAINERS: Update my email address
From: Bin Meng The old Wind River email address (bin.m...@windriver.com) is no longer available due to an internal infrastructure change within the company. While a new email address (bin.meng...@windriver.com) has been assigned to me, I am unable to find a way to send this patch directly from the new address. Presumably, the basic authentication with client submission (SMTP AUTH) [1] has been disabled by the company's IT. Switch to use my personal email address instead. Signed-off-by: Bin Meng Signed-off-by: Bin Meng [1] https://learn.microsoft.com/en-us/exchange/mail-flow-best-practices/how-to-set-up-a-multifunction-device-or-application-to-send-email-using-microsoft-365-or-office-365 --- Changes in v2: - Provide more background info for the email address change MAINTAINERS | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index 2f08cc528e..d3282ee29f 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -319,7 +319,7 @@ F: tests/tcg/ppc*/* RISC-V TCG CPUs M: Palmer Dabbelt M: Alistair Francis -M: Bin Meng +M: Bin Meng R: Weiwei Li R: Daniel Henrique Barboza R: Liu Zhiwei @@ -1602,7 +1602,7 @@ F: include/hw/riscv/opentitan.h F: include/hw/*/ibex_*.h Microchip PolarFire SoC Icicle Kit -M: Bin Meng +M: Bin Meng L: qemu-ri...@nongnu.org S: Supported F: docs/system/riscv/microchip-icicle-kit.rst @@ -1629,7 +1629,7 @@ F: include/hw/char/shakti_uart.h SiFive Machines M: Alistair Francis -M: Bin Meng +M: Bin Meng M: Palmer Dabbelt L: qemu-ri...@nongnu.org S: Supported @@ -2125,7 +2125,7 @@ F: hw/ssi/xilinx_* SD (Secure Card) M: Philippe Mathieu-Daudé -M: Bin Meng +M: Bin Meng L: qemu-bl...@nongnu.org S: Odd Fixes F: include/hw/sd/sd* -- 2.34.1
[PATCH] MAINTAINERS: Update my email address
The Wind River email address might change in the future. Use my personal email address instead. Signed-off-by: Bin Meng --- MAINTAINERS | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index f1f6922025..50729a0985 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -332,7 +332,7 @@ F: tests/tcg/ppc*/* RISC-V TCG CPUs M: Palmer Dabbelt M: Alistair Francis -M: Bin Meng +M: Bin Meng R: Weiwei Li R: Daniel Henrique Barboza R: Liu Zhiwei @@ -1614,7 +1614,7 @@ F: include/hw/riscv/opentitan.h F: include/hw/*/ibex_*.h Microchip PolarFire SoC Icicle Kit -M: Bin Meng +M: Bin Meng L: qemu-ri...@nongnu.org S: Supported F: docs/system/riscv/microchip-icicle-kit.rst @@ -1641,7 +1641,7 @@ F: include/hw/char/shakti_uart.h SiFive Machines M: Alistair Francis -M: Bin Meng +M: Bin Meng M: Palmer Dabbelt L: qemu-ri...@nongnu.org S: Supported @@ -2137,7 +2137,7 @@ F: hw/ssi/xilinx_* SD (Secure Card) M: Philippe Mathieu-Daudé -M: Bin Meng +M: Bin Meng L: qemu-bl...@nongnu.org S: Odd Fixes F: include/hw/sd/sd* -- 2.34.1
Re: [PATCH 3/7] scripts/nsis.py: Automatically package required DLLs of QEMU executables
On Mon, Feb 26, 2024 at 1:37 AM Stefan Weil wrote:
>
> Am 10.09.22 um 02:37 schrieb Bin Meng:
> > On Sat, Sep 10, 2022 at 12:49 AM Mark Cave-Ayland
> > wrote:
> >>
> >> On 08/09/2022 14:28, Bin Meng wrote:
> >>
> >>> From: Bin Meng
> >>>
> >>> At present packaging the required DLLs of QEMU executables is a
> >>> manual process, and error prone.
> >>>
> >>> Actually build/config-host.mak contains a GLIB_BINDIR variable
> >>> which is the directory where glib and other DLLs reside. This
> >>> works for both Windows native build and cross-build on Linux.
> >>> We can use it as the search directory for DLLs and automate
> >>> the whole DLL packaging process.
> >>>
> >>> Signed-off-by: Bin Meng
> >>> ---
> >>>
> >>>meson.build | 1 +
> >>>scripts/nsis.py | 46 ++
> >>>2 files changed, 43 insertions(+), 4 deletions(-)
> >>>
> [...]>>> diff --git a/scripts/nsis.py b/scripts/nsis.py
> >>> index baa6ef9594..03ed7608a2 100644
> >>> --- a/scripts/nsis.py
> >>> +++ b/scripts/nsis.py
> >>> @@ -18,12 +18,36 @@ def signcode(path):
> >>>return
> >>>subprocess.run([cmd, path])
> >>>
> >>> +def find_deps(exe_or_dll, search_path, analyzed_deps):
> >>> +deps = [exe_or_dll]
> >>> +output = subprocess.check_output(["objdump", "-p", exe_or_dll],
> >>> text=True)
>
> This fails on non x86 hosts where objdump does not know how to handle a
> Windows x86_64 exe file.
Does this command work in the MSYS2 environment on Windows Arm?
>
> >>> +output = output.split("\n")
> >>> +for line in output:
> >>> +if not line.startswith("\tDLL Name: "):
> >>> +continue
> >>> +
> >>> +dep = line.split("DLL Name: ")[1].strip()
> >>> +if dep in analyzed_deps:
> >>> +continue
> >>> +
> >>> +dll = os.path.join(search_path, dep)
> >>> +if not os.path.exists(dll):
> >>> +# assume it's a Windows provided dll, skip it
> >>> +continue
> >>> +
> >>> +analyzed_deps.add(dep)
> >>> +# locate the dll dependencies recursively
> >>> +rdeps = find_deps(dll, search_path, analyzed_deps)
> >>> +deps.extend(rdeps)
> >>> +
> >>> +return deps
> [...]
> >>
> >> FWIW I wrote a similar script a while back to help package a custom
> >> Windows build for
> >> a client, however I used ldd instead of objdump since it provided the full
> >> paths for
> >> DLLs installed in the msys2/mingw-w64 environment via pacman which were
> >> outside the
> >> QEMU build tree.
> >>
> >
> > Yep, ldd also works, but only on Windows native build. objdump can
> > work on both Windows native and Linux cross builds.
>
>
> objdump fails on Linux cross builds on any non x86 host, because objdump
> typically only supports the native host architecture.
>
> Therefore I get an error on an ARM64 host (podman on Mac M1):
>
> objdump: /tmp/tmpvae5u0qm/qemu/qemu-system-aarch64.exe: file
> format not recognized
>
> I could use x86_64-w64-mingw32-objdump to fix the cross builds, but then
> native builds might fail because they don't have x86_64-w64-mingw32-objdump.
>
> Is there a simple way how we can get the information here whether
> objdump requires a cross build prefix?
For QEMU Windows, I believe the only supported architecture is x86_64,
correct? Do we want to support (cross) building QEMU for Windows Arm?
Based on your observation, objdump on Linux cross builds on any x86
host works, but not on non x86 host.
Maybe it's a hint to ask for binutils guys to include the PE format
support for objdump Arm by default.
Regards,
Bin
[PATCH] util/cutil: Allow relocatable install with prefix /
When configuring QEMU with --prefix=/, the generated QEMU executables can't be relocated to other directories. Add an additional test logic in starts_with_prefix() to handle this. Signed-off-by: Bin Meng --- util/cutils.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/util/cutils.c b/util/cutils.c index 42364039a5..676bd757ba 100644 --- a/util/cutils.c +++ b/util/cutils.c @@ -1021,7 +1021,8 @@ static inline bool starts_with_prefix(const char *dir) #pragma GCC diagnostic ignored "-Warray-bounds=" #endif return !memcmp(dir, CONFIG_PREFIX, prefix_len) && -(!dir[prefix_len] || G_IS_DIR_SEPARATOR(dir[prefix_len])); +(!dir[prefix_len] || G_IS_DIR_SEPARATOR(dir[prefix_len]) || + !strcmp(CONFIG_PREFIX, "/")); #pragma GCC diagnostic pop } -- 2.34.1
[PATCH] hw/elf_ops: Ignore loadable segments with zero size
Some ELF files really do have segments of zero size, e.g.:
Program Headers:
Type Offset VirtAddr PhysAddr
FileSizMemSiz Flags Align
RISCV_ATTRIBUT 0x25b8 0x 0x
0x003e 0x R 0x1
LOAD 0x1000 0x8020 0x8020
0x01d1 0x01d1 R E0x1000
LOAD 0x11d1 0x802001d1 0x802001d1
0x0e37 0x0e37 RW 0x1000
LOAD 0x0120 0x 0x
0x 0x 0x1000
The current logic does not check for this condition, resulting in
the incorrect assignment of 'lowaddr' as zero.
There is already a piece of codes inside the segment traversal loop
that checks for zero-sized loadable segments for not creating empty
ROM blobs. Let's move this check to the beginning of the loop to
cover both scenarios.
Signed-off-by: Bin Meng
---
include/hw/elf_ops.h | 75 +++-
1 file changed, 39 insertions(+), 36 deletions(-)
diff --git a/include/hw/elf_ops.h b/include/hw/elf_ops.h
index 0a5c258fe6..f014399b50 100644
--- a/include/hw/elf_ops.h
+++ b/include/hw/elf_ops.h
@@ -427,6 +427,16 @@ static ssize_t glue(load_elf, SZ)(const char *name, int fd,
file_size = ph->p_filesz; /* Size of the allocated data */
data_offset = ph->p_offset; /* Offset where the data is located */
+/*
+ * Some ELF files really do have segments of zero size;
+ * just ignore them rather than trying to set the wrong addr,
+ * or create empty ROM blobs, because the zero-length blob can
+ * falsely trigger the overlapping-ROM-blobs check.
+ */
+if (mem_size == 0) {
+continue;
+}
+
if (file_size > 0) {
if (g_mapped_file_get_length(mapped_file) <
file_size + data_offset) {
@@ -530,45 +540,38 @@ static ssize_t glue(load_elf, SZ)(const char *name, int
fd,
*pentry = ehdr.e_entry - ph->p_vaddr + ph->p_paddr;
}
-/* Some ELF files really do have segments of zero size;
- * just ignore them rather than trying to create empty
- * ROM blobs, because the zero-length blob can falsely
- * trigger the overlapping-ROM-blobs check.
- */
-if (mem_size != 0) {
-if (load_rom) {
-g_autofree char *label =
-g_strdup_printf("%s ELF program header segment %d",
-name, i);
-
-/*
- * rom_add_elf_program() takes its own reference to
- * 'mapped_file'.
- */
-rom_add_elf_program(label, mapped_file, data, file_size,
-mem_size, addr, as);
-} else {
-MemTxResult res;
-
-res = address_space_write(as ? as : &address_space_memory,
- addr, MEMTXATTRS_UNSPECIFIED,
- data, file_size);
+if (load_rom) {
+g_autofree char *label =
+g_strdup_printf("%s ELF program header segment %d",
+name, i);
+
+/*
+ * rom_add_elf_program() takes its own reference to
+ * 'mapped_file'.
+ */
+rom_add_elf_program(label, mapped_file, data, file_size,
+mem_size, addr, as);
+} else {
+MemTxResult res;
+
+res = address_space_write(as ? as : &address_space_memory,
+ addr, MEMTXATTRS_UNSPECIFIED,
+ data, file_size);
+if (res != MEMTX_OK) {
+goto fail;
+}
+/*
+ * We need to zero'ify the space that is not copied
+ * from file
+ */
+if (file_size < mem_size) {
+res = address_space_set(as ? as : &address_space_memory,
+addr + file_size, 0,
+mem_size - file_size,
+MEMTXATTRS_UNSPECIFIED);
if (res != MEMTX_OK) {
goto fail;
}
-
Re: [PATCH 3/3] tests/acpi: Update virt/SSDT.memhp
On Mon, Jan 15, 2024 at 7:40 PM Alex Bennée wrote:
>
> Bin Meng writes:
>
> > The Arm dtb changes caused an address change:
> >
> > DefinitionBlock ("", "SSDT", 1, "BOCHS ", "NVDIMM", 0x0001)
> > {
> > [ ... ]
> > -Name (MEMA, 0x43C8)
> > +Name (MEMA, 0x43D8)
> > }
>
> I'm confused by why this changes. Isn't this declaring the size of a
> NVDIMM region of the memory map? Why does a DTB change affect an ACPI
> based boot?
>
I have no idea too. I suspect that's because the AllocateAlignedPages
call to allocate a 1 MiB aligned address in the BiosTableTest.c is
affected by the shrinked DTB now.
+ Laszlo who might know the root cause.
Regards,
Bin
[PATCH 1/3] hw/arm: Refactor struct arm_boot_info::get_dtb()
At present we expect struct arm_boot_info::get_dtb() to return the
device tree pointer as well as the device tree size. However, this
is not necessary as we can get the device tree size via the device
tree header directly. Change get_dtb() signature to drop the *size
argument, and get the size by ourselves.
Signed-off-by: Bin Meng
---
include/hw/arm/boot.h | 8
hw/arm/boot.c | 3 ++-
hw/arm/sbsa-ref.c | 3 +--
hw/arm/virt.c | 4 +---
hw/arm/xlnx-versal-virt.c | 4 +---
5 files changed, 9 insertions(+), 13 deletions(-)
diff --git a/include/hw/arm/boot.h b/include/hw/arm/boot.h
index 80c492d742..37fd1b520e 100644
--- a/include/hw/arm/boot.h
+++ b/include/hw/arm/boot.h
@@ -82,11 +82,11 @@ struct arm_boot_info {
const struct arm_boot_info *info);
/* if a board is able to create a dtb without a dtb file then it
* sets get_dtb. This will only be used if no dtb file is provided
- * by the user. On success, sets *size to the length of the created
- * dtb, and returns a pointer to it. (The caller must free this memory
- * with g_free() when it has finished with it.) On failure, returns NULL.
+ * by the user. On success, returns a pointer to it. (The caller must
+ * free this memory with g_free() when it has finished with it.)
+ * On failure, returns NULL.
*/
-void *(*get_dtb)(const struct arm_boot_info *info, int *size);
+void *(*get_dtb)(const struct arm_boot_info *info);
/* if a board needs to be able to modify a device tree provided by
* the user it should implement this hook.
*/
diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 84ea6a807a..ff1173299f 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -538,11 +538,12 @@ int arm_load_dtb(hwaddr addr, const struct arm_boot_info
*binfo,
}
g_free(filename);
} else {
-fdt = binfo->get_dtb(binfo, &size);
+fdt = binfo->get_dtb(binfo);
if (!fdt) {
fprintf(stderr, "Board was unable to create a dtb blob\n");
goto fail;
}
+size = fdt_totalsize(fdt);
}
if (addr_limit > addr && size > (addr_limit - addr)) {
diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
index 477dca0637..c5023871a7 100644
--- a/hw/arm/sbsa-ref.c
+++ b/hw/arm/sbsa-ref.c
@@ -681,12 +681,11 @@ static void create_pcie(SBSAMachineState *sms)
create_smmu(sms, pci->bus);
}
-static void *sbsa_ref_dtb(const struct arm_boot_info *binfo, int *fdt_size)
+static void *sbsa_ref_dtb(const struct arm_boot_info *binfo)
{
const SBSAMachineState *board = container_of(binfo, SBSAMachineState,
bootinfo);
-*fdt_size = board->fdt_size;
return board->fdt;
}
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 2793121cb4..1996fffa99 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1577,14 +1577,12 @@ static void create_secure_ram(VirtMachineState *vms,
g_free(nodename);
}
-static void *machvirt_dtb(const struct arm_boot_info *binfo, int *fdt_size)
+static void *machvirt_dtb(const struct arm_boot_info *binfo)
{
const VirtMachineState *board = container_of(binfo, VirtMachineState,
bootinfo);
MachineState *ms = MACHINE(board);
-
-*fdt_size = board->fdt_size;
return ms->fdt;
}
diff --git a/hw/arm/xlnx-versal-virt.c b/hw/arm/xlnx-versal-virt.c
index 537118224f..1e043c813e 100644
--- a/hw/arm/xlnx-versal-virt.c
+++ b/hw/arm/xlnx-versal-virt.c
@@ -551,12 +551,10 @@ static void versal_virt_modify_dtb(const struct
arm_boot_info *binfo,
fdt_add_memory_nodes(s, fdt, binfo->ram_size);
}
-static void *versal_virt_get_dtb(const struct arm_boot_info *binfo,
- int *fdt_size)
+static void *versal_virt_get_dtb(const struct arm_boot_info *binfo)
{
const VersalVirt *board = container_of(binfo, VersalVirt, binfo);
-*fdt_size = board->fdt_size;
return board->fdt;
}
--
2.34.1
[PATCH 3/3] tests/acpi: Update virt/SSDT.memhp
The Arm dtb changes caused an address change:
DefinitionBlock ("", "SSDT", 1, "BOCHS ", "NVDIMM", 0x0001)
{
[ ... ]
-Name (MEMA, 0x43C8)
+Name (MEMA, 0x43D8)
}
Signed-off-by: Bin Meng
---
tests/data/acpi/virt/SSDT.memhp | Bin 1817 -> 1817 bytes
1 file changed, 0 insertions(+), 0 deletions(-)
diff --git a/tests/data/acpi/virt/SSDT.memhp b/tests/data/acpi/virt/SSDT.memhp
index
fb3dcde5a10936667ad75a759b8bd444a7b19fc2..4d3ef733276bf5992da5b0bb967f6d60e243417d
100644
GIT binary patch
delta 22
dcmbQqH
[PATCH 0/3] hw/arm: Pack the QEMU generated device tree
By default, QEMU generates a 1 MiB sized device tree. This appears to be unnecessary, as the actual size is much smaller than what the DTB header claims. Let's pack it to save some room. Bin Meng (3): hw/arm: Refactor struct arm_boot_info::get_dtb() hw/arm: Pack the QEMU generated device tree tests/acpi: Update virt/SSDT.memhp include/hw/arm/boot.h | 8 hw/arm/boot.c | 14 +- hw/arm/sbsa-ref.c | 3 +-- hw/arm/virt.c | 4 +--- hw/arm/xlnx-versal-virt.c | 4 +--- tests/data/acpi/virt/SSDT.memhp | Bin 1817 -> 1817 bytes 6 files changed, 20 insertions(+), 13 deletions(-) -- 2.34.1
[PATCH 2/3] hw/arm: Pack the QEMU generated device tree
By default QEMU generates a 1 MiB sized device tree. Let's pack it
to save some room.
Signed-off-by: Bin Meng
---
hw/arm/boot.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index ff1173299f..511ec10ed0 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -662,6 +662,17 @@ int arm_load_dtb(hwaddr addr, const struct arm_boot_info
*binfo,
binfo->modify_dtb(binfo, fdt);
}
+/*
+ * By default QEMU generates a 1 MiB sized device tree.
+ * Let's pack it to save some room.
+ */
+if (binfo->get_dtb) {
+rc = fdt_pack(fdt);
+/* Should only fail if we've built a corrupted tree */
+g_assert(rc == 0);
+size = fdt_totalsize(fdt);
+}
+
qemu_fdt_dumpdtb(fdt, size);
/* Put the DTB into the memory map as a ROM image: this will ensure
--
2.34.1
Re: [PATCH] net/vmnet: Pad short Ethernet frames
On Sun, Jan 7, 2024 at 7:19 AM William Hooper wrote: > > At least on macOS 12.7.2, vmnet doesn't pad Ethernet frames, such as the > host's ARP replies, to the minimum size (60 bytes before the frame check > sequence) defined in IEEE Std 802.3-2022, so guests' Ethernet device > drivers may drop them with "frame too short" errors. > > This patch calls eth_pad_short_frame() to add padding, as in net/tap.c > and net/slirp.c. > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2058 > Signed-off-by: William Hooper > --- > net/vmnet-common.m | 20 +--- > 1 file changed, 17 insertions(+), 3 deletions(-) > Reviewed-by: Bin Meng
[PATCH] docs/system/riscv: sifive_u: Update S-mode U-Boot image build instructions
Currently, the documentation outlines the process for building the S-mode U-Boot image using `make menuconfig` and manual actions within the menuconfig UI. However, this approach is fragile due to Kconfig options potentially changing across different releases. For example, CONFIG_OF_PRIOR_STAGE has been replaced by CONFIG_BOARD since v2022.01 release, and CONFIG_TEXT_BASE has been moved to the 'General setup' menu from the 'Boot options' menu in v2024.01 release. This update aims to make the S-mode U-Boot image build instructions future-proof. It leverages the 'config' script provided in the U-Boot source tree to edit the .config file, followed by a `make olddefconfig`. Validated with U-Boot v2024.01 release. Signed-off-by: Bin Meng --- docs/system/riscv/sifive_u.rst | 33 - 1 file changed, 12 insertions(+), 21 deletions(-) diff --git a/docs/system/riscv/sifive_u.rst b/docs/system/riscv/sifive_u.rst index 7b166567f9..8f55ae8e31 100644 --- a/docs/system/riscv/sifive_u.rst +++ b/docs/system/riscv/sifive_u.rst @@ -210,7 +210,7 @@ command line options with ``qemu-system-riscv32``. Running U-Boot -- -U-Boot mainline v2021.07 release is tested at the time of writing. To build a +U-Boot mainline v2024.01 release is tested at the time of writing. To build a U-Boot mainline bootloader that can be booted by the ``sifive_u`` machine, use the sifive_unleashed_defconfig with similar commands as described above for Linux: @@ -325,15 +325,10 @@ configuration of U-Boot: $ export CROSS_COMPILE=riscv64-linux- $ make sifive_unleashed_defconfig - $ make menuconfig - -then manually select the following configuration: - - * Device Tree Control ---> Provider of DTB for DT Control ---> Prior Stage bootloader DTB - -and unselect the following configuration: - - * Library routines ---> Allow access to binman information in the device tree + $ ./scripts/config --enable OF_BOARD + $ ./scripts/config --disable BINMAN_FDT + $ ./scripts/config --disable SPL + $ make olddefconfig This changes U-Boot to use the QEMU generated device tree blob, and bypass running the U-Boot SPL stage. @@ -352,17 +347,13 @@ It's possible to create a 32-bit U-Boot S-mode image as well. $ export CROSS_COMPILE=riscv64-linux- $ make sifive_unleashed_defconfig - $ make menuconfig - -then manually update the following configuration in U-Boot: - - * Device Tree Control ---> Provider of DTB for DT Control ---> Prior Stage bootloader DTB - * RISC-V architecture ---> Base ISA ---> RV32I - * Boot options ---> Boot images ---> Text Base ---> 0x8040 - -and unselect the following configuration: - - * Library routines ---> Allow access to binman information in the device tree + $ ./scripts/config --disable ARCH_RV64I + $ ./scripts/config --enable ARCH_RV32I + $ ./scripts/config --set-val TEXT_BASE 0x8040 + $ ./scripts/config --enable OF_BOARD + $ ./scripts/config --disable BINMAN_FDT + $ ./scripts/config --disable SPL + $ make olddefconfig Use the same command line options to boot the 32-bit U-Boot S-mode image: -- 2.34.1
Re: [PATCH 1/1] Leaving Migration
On Wed, Jan 3, 2024 at 4:20 AM Juan Quintela wrote: > > I am leaving Red Hat, and as part of that I am leaving Migration > maintenarship. maintainership? > > You are left in good hands with Peter and Fabiano. > > Thanks for all the fish. Best wishes! > > Signed-off-by: Juan Quintela > --- > MAINTAINERS | 3 --- > .mailmap| 1 + > 2 files changed, 1 insertion(+), 3 deletions(-) > Reviewed-by: Bin Meng
[PATCH] roms/opensbi: Upgrade from v1.3.1 to v1.4
latform: update platform_requirements.md 3632f2b lib: sbi: Add support for mconfigptr ec0559e lib: sbi_misaligned_ldst: Fix handling of C.SWSP and C.SDSP cbdd869 include: sbi: Change spec version to 2.0 5d0ed1b lib: sbi: simplify sanitize_domain() c1a6987 platform: generic: thead: move to thead c9xx header to vendor specific postion 8e941e7 platform: generic: thead: separate implement of T-HEAD c9xx pmu 492d9b1 platform: generic: thead: separate implement of T-HEAD c9xx errata 3e21b96 platform: generic: thead: initialize PMU by default in thead generic platform a140a4e lib: sbi: Correctly limit flushes to a single ASID/VMID 88ae718 platform: generic: thead: improve tlb flush errata 52fd64b platform: Uses hart count as the default size of tlb info 07f2ccd lib: utils/serial: Optimize semihosting_putc implementation fccdf41 firmware: fw_base.S: Fix boot hart status synchronization d1e0f7f utils/reset: Remove fdt_reset_thead 896d2c9 lib: utils/timer: Allow ACLINT MTIMER driver to setup quirks accafb1 lib: utils/timer: mtimer: add separate T-Head C9xx CLINT mtimer compatible 98bc25f lib: utils/ipi: mswi: add separate T-Head C9xx CLINT mswi compatible 5b2f55d lib: sbi: separate the swap operation of domain region 3b03cdd lib: sbi: Add regions merging when sanitizing domain region 2bfdb9e platform: generic: Add Sophgo sg2042 platform support 280f7ae include: sbi: macros for mseccfg.sseed and .useed efcac33 lib: sbi: Add Zkr in hart extensions 6e5b0cf lib: sbi: enable seed access in S-mode 6602e11 lib: sbi: change sbi_hart_features.extensions as an array 3aaed4f lib: sbi: Make console_puts/console_putc interchangeable dc0bb19 lib: utils/serial: remove semihosting_putc 16bb930 lib: sbi: Fix PMP granularity handling in sbi_hart_map_saddr() 574b9c8 lib: sbi_pmu: avoid buffer overflow 791704c lib: utils/irqchip: Avoid redundant writes to APLIC CLRIE register f520256 lib: sbi: Allow relaxed MMIO writes in device ipi_send() callback b70d628 lib: sbi: Allow relaxed MMIO writes in device ipi_clear() callback bd74931 lib: ipi: Adjust Andes PLICSW to single-bit-per-hart scheme 291403f sbi: sbi_pmu: Improve sbi_pmu_init() error handling 090fa99 lib: sbi: Add XAndesPMU in hart extensions a48f2cf sbi: sbi_pmu: Add hw_counter_filter_mode() to pmu device 51ec60c platform: include: andes45: Add PMU related CSR defines effd89a platform: generic: Introduce pmu_init() platform override 1b9e743 platform: andes: Add Andes custom PMU support 2e50c24 platform: andes: Enable Andes PMU for AE350 535c661 platform: rzfive: Enable Andes PMU for RZ/Five 0b3262e lib: utils: fdt_fixup: Allow preserving PMU properties 009ae4e platform: andes: Factor out is_andes() helper 0308f93 lib: utils: fdt_pmu: Make the fdt_pmu_evt_select table global variable e19d419 lib: utils: fdt_pmu: Do not iterate over the fdt_pmu_evt_select table d162009 docs: pmu: Add Andes PMU node example 6b9a849 lib: sbi: Remove xchg/cmpxchg implemented via lr/sc 11bf49b lib: sbi: Fix __atomic_op_bit_ord and comments 8839869 lib: sbi: Replace __atomic_op_bit_ord with __atomic intrinsics 07419ec lib: sbi: Prevent redundant sbi_ipi_process 93da66b lib: sbi_hart: Store PMP granularity as log base 2 ee72517 lib: sbi_pmu: Add PMU snapshot definitions 11a0ba5 lib: sbi_pmu: Fix the counter info function 0696810 firmware: fix section types a25fc74 lib: sbi_hsm: Put the resume_pending hart in the interruptible hart mask 87aa306 platform: recalculate heap size to support new tlb entry number a2e254e lib: sbi: skip wait_for_coldboot when coolboot done 6112d58 lib: utils/fdt: Allow to use reg-names when parsing ACLINT 35cba92 lib: sbi_tlb: Check tlb_range_flush_limit only once per request a894187 lib: sbi_ipi: Do not ignore errors from sbi_ipi_send() 446fa65 lib: sbi_ipi: Process self-IPIs in sbi_ipi_send() 2707250 lib: sbi_ipi: Drop unnecessary ipi_process check 925ce14 lib: sbi: Simplify the initialization of root_hmask in sbi_domain_init 2c8be56 lib: sbi: Improve the code of privilege mode and extensions detection 056fe6f lib: sbi: Refactor the code for enable extensions in menvfg CSR 776770d lib: sbi: Using one array to define the name of extensions 3daac8f lib: sbi: Detect extensions from the ISA string in DT 416ceb3 lib: sbi_tlb: Reduce size of struct sbi_tlb_info 80169b2 platform: generic: Fine tune fw_platform_calculate_heap_size() cdebae2 lib: utils/irqchip: Add shared MMIO region for PLIC in root domain 3284bea lib: sbi: Allow ecall handlers to directly update register state 5a57e8c lib: sbi: Remove the SBI_ETRAP error code 2b80b92 lib: sbi: Do not enter OpenSBI with mseccfg.MML == 1 63e09ad lib: sbi: Fix shift bug in sbi_system_reset ba29293 lib: utils/timer: mtimer: only use regname for aclint bbd065d lib: sbi: Detect Zicntr extension only based on traps a2b255b include: Bump-up version to 1.4 Signed-off-by: Bin Meng --- Please pull the complete patch from https://github.com/lbmeng/qemu opensbi branch. .../opensbi-riscv32-generic-fw_dynamic.bin| Bin 135376 ->
[PATCH for 8.2.1] hw/net: cadence_gem: Fix MDIO_OP_xxx values
Testing upstream U-Boot with 'sifive_u' machine we see:
=> dhcp
ethernet@1009: PHY present at 0
Could not get PHY for ethernet@1009: addr 0
phy_connect failed
This has been working till QEMU 8.1 but broken since QEMU 8.2.
Fixes: 1b09eeb122aa ("hw/net/cadence_gem: use FIELD to describe PHYMNTNC
register fields")
Reported-by: Heinrich Schuchardt
Signed-off-by: Bin Meng
---
hw/net/cadence_gem.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index 296bba238e..472ce9c8cf 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -199,8 +199,8 @@ REG32(PHYMNTNC, 0x34) /* Phy Maintenance reg */
FIELD(PHYMNTNC, PHY_ADDR, 23, 5)
FIELD(PHYMNTNC, OP, 28, 2)
FIELD(PHYMNTNC, ST, 30, 2)
-#define MDIO_OP_READ0x3
-#define MDIO_OP_WRITE 0x2
+#define MDIO_OP_READ0x2
+#define MDIO_OP_WRITE 0x1
REG32(RXPAUSE, 0x38) /* RX Pause Time reg */
REG32(TXPAUSE, 0x3c) /* TX Pause Time reg */
--
2.34.1
Re: [PATCH 1/1] MAINTAINERS: update mail address for Weiwei Li
On Mon, Oct 30, 2023 at 4:17 PM Weiwei Li wrote: > > My Iscas mail account will be disabled soon, change to my personal > gmail account. > > Signed-off-by: Weiwei Li > --- > MAINTAINERS | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > Wish you best of luck on your new journey! Reviewed-by: Bin Meng
Re: [PATCH] hw/ufs: Fix incorrect register fields
On Tue, Oct 10, 2023 at 1:11 PM Jeuk Kim wrote:
>
> From: Jeuk Kim
>
> This patch fixes invalid ufs register fields.
> This fixes an issue reported by Bin Meng that
> caused ufs to fail over riscv.
>
Fixes: bc4e68d362ec ("hw/ufs: Initial commit for emulated
Universal-Flash-Storage")
Reported-by: Bin Meng
> Signed-off-by: Jeuk Kim
> ---
> include/block/ufs.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
Reviewed-by: Bin Meng
Tested-by: Bin Meng
Re: [PATCH 2/2] hw/char/riscv_htif: Fix the console syscall on big endian hosts
On Fri, Jul 21, 2023 at 5:48 PM Thomas Huth wrote:
>
> Values that have been read via cpu_physical_memory_read() from the
> guest's memory have to be swapped in case the host endianess differs
typo: endianness
> from the guest.
>
> Fixes: a6e13e31d5 ("riscv_htif: Support console output via proxy syscall")
> Signed-off-by: Thomas Huth
> ---
> hw/char/riscv_htif.c | 9 +
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
Reviewed-by: Bin Meng
Re: [PATCH 1/2] hw/char/riscv_htif: Fix printing of console characters on big endian hosts
On Fri, Jul 21, 2023 at 5:48 PM Thomas Huth wrote:
>
> The character that should be printed is stored in the 64 bit "payload"
> variable. The code currently tries to print it by taking the address
> of the variable and passing this pointer to qemu_chr_fe_write(). However,
> this only works on little endian hosts where the least significant bits
> are stored on the lowest address. To do this in a portable way, we have
> to store the value in an uint8_t variable instead.
>
> Fixes: 5033606780 ("RISC-V HTIF Console")
> Signed-off-by: Thomas Huth
> ---
> hw/char/riscv_htif.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
Reviewed-by: Bin Meng
[PATCH] roms/opensbi: Upgrade from v1.3 to v1.3.1
Upgrade OpenSBI from v1.3 to v1.3.1 and the pre-built bios images which fixes the boot failure seen when using QEMU to do a direct kernel boot with Microchip Icicle Kit board machine. The v1.3.1 release includes the following commits: 0907de3 lib: sbi: fix comment indent eb736a5 lib: sbi_pmu: Avoid out of bounds access 7828eeb gpio/desginware: add Synopsys DesignWare APB GPIO support c6a3573 lib: utils: Fix sbi_hartid_to_scratch() usage in ACLINT drivers 057eb10 lib: utils/gpio: Fix RV32 compile error for designware GPIO driver Signed-off-by: Bin Meng --- Please pull the complete patch from https://github.com/lbmeng/qemu opensbi branch. .../opensbi-riscv32-generic-fw_dynamic.bin| Bin 135344 -> 135376 bytes .../opensbi-riscv64-generic-fw_dynamic.bin| Bin 138304 -> 138368 bytes roms/opensbi | 2 +- 3 files changed, 1 insertion(+), 1 deletion(-) diff --git a/pc-bios/opensbi-riscv32-generic-fw_dynamic.bin b/pc-bios/opensbi-riscv32-generic-fw_dynamic.bin index 7b6c67e0ae..9a2ba3f2a4 100644 Binary files a/pc-bios/opensbi-riscv32-generic-fw_dynamic.bin and b/pc-bios/opensbi-riscv32-generic-fw_dynamic.bin differ diff --git a/pc-bios/opensbi-riscv64-generic-fw_dynamic.bin b/pc-bios/opensbi-riscv64-generic-fw_dynamic.bin index 1b831b412c..5d4e812819 100644 Binary files a/pc-bios/opensbi-riscv64-generic-fw_dynamic.bin and b/pc-bios/opensbi-riscv64-generic-fw_dynamic.bin differ diff --git a/roms/opensbi b/roms/opensbi index 2552799a1d..057eb10b6d 16 --- a/roms/opensbi +++ b/roms/opensbi @@ -1 +1 @@ -Subproject commit 2552799a1df30a3dcd2321a8b75d61d06f5fb9fc +Subproject commit 057eb10b6d523540012e6947d5c9f63e95244e94 -- 2.34.1
Re: Boot failure after QEMU's upgrade to OpenSBI v1.3 (was Re: [PATCH for-8.2 6/7] target/riscv: add 'max' CPU type)
Hi Anup,
On Thu, Jul 20, 2023 at 12:10 AM Anup Patel wrote:
>
> Hi Bin,
>
> On Wed, Jul 19, 2023 at 9:15 PM Bin Meng wrote:
> >
> > On Wed, Jul 19, 2023 at 11:22 PM Anup Patel wrote:
> > >
> > > On Wed, Jul 19, 2023 at 3:23 PM Alistair Francis
> > > wrote:
> > > >
> > > > On Wed, Jul 19, 2023 at 3:39 PM Anup Patel wrote:
> > > > >
> > > > > On Wed, Jul 19, 2023 at 7:03 AM Alistair Francis
> > > > > wrote:
> > > > > >
> > > > > > On Sat, Jul 15, 2023 at 7:14 PM Atish Patra
> > > > > > wrote:
> > > > > > >
> > > > > > > On Fri, Jul 14, 2023 at 5:29 AM Conor Dooley
> > > > > > > wrote:
> > > > > > > >
> > > > > > > > On Fri, Jul 14, 2023 at 11:19:34AM +0100, Conor Dooley wrote:
> > > > > > > > > On Fri, Jul 14, 2023 at 10:00:19AM +0530, Anup Patel wrote:
> > > > > > > > >
> > > > > > > > > > > > OpenSBI v1.3
> > > > > > > > > > > >_ _
> > > > > > > > > > > > / __ \ / | _ \_ _|
> > > > > > > > > > > > | | | |_ __ ___ _ __ | (___ | |_) || |
> > > > > > > > > > > > | | | | '_ \ / _ \ '_ \ \___ \| _ < | |
> > > > > > > > > > > > | |__| | |_) | __/ | | |) | |_) || |_
> > > > > > > > > > > > \/| .__/ \___|_| |_|_/|___/_|
> > > > > > > > > > > > | |
> > > > > > > > > > > > |_|
> > > > > > > > > > > >
> > > > > > > > > > > > init_coldboot: ipi init failed (error -1009)
> > > > > > > > > > > >
> > > > > > > > > > > > Just to note, because we use our own firmware that
> > > > > > > > > > > > vendors in OpenSBI
> > > > > > > > > > > > and compiles only a significantly cut down number of
> > > > > > > > > > > > files from it, we
> > > > > > > > > > > > do not use the fw_dynamic etc flow on our hardware. As
> > > > > > > > > > > > a result, we have
> > > > > > > > > > > > not tested v1.3, nor do we have any immediate plans to
> > > > > > > > > > > > change our
> > > > > > > > > > > > platform firmware to vendor v1.3 either.
> > > > > > > > > > > >
> > > > > > > > > > > > I unless there's something obvious to you, it sounds
> > > > > > > > > > > > like I will need to
> > > > > > > > > > > > go and bisect OpenSBI. That's a job for another day
> > > > > > > > > > > > though, given the
> > > > > > > > > > > > time.
> > > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > The real issue is some CPU/HART DT nodes marked as disabled
> > > > > > > > > > in the
> > > > > > > > > > DT passed to OpenSBI 1.3.
> > > > > > > > > >
> > > > > > > > > > This issue does not exist in any of the DTs generated by
> > > > > > > > > > QEMU but some
> > > > > > > > > > of the DTs in the kernel (such as microchip and SiFive
> > > > > > > > > > board DTs) have
> > > > > > > > > > the E-core disabled.
> > > > > > > > > >
> > > > > > > > > > I had discovered this issue in a totally different context
> > > > > > > > > > after the OpenSBI 1.3
> > > > > > > > > > release happened. This issue is already fixed in the latest
> > > > > > > > > > OpenSBI by the
> > > > > > > > > > following commit c6a35733b74aeff612398f274ed19a74f81d1f37
> > > > > > > > > > ("lib: util
Re: Boot failure after QEMU's upgrade to OpenSBI v1.3 (was Re: [PATCH for-8.2 6/7] target/riscv: add 'max' CPU type)
On Wed, Jul 19, 2023 at 11:22 PM Anup Patel wrote:
>
> On Wed, Jul 19, 2023 at 3:23 PM Alistair Francis wrote:
> >
> > On Wed, Jul 19, 2023 at 3:39 PM Anup Patel wrote:
> > >
> > > On Wed, Jul 19, 2023 at 7:03 AM Alistair Francis
> > > wrote:
> > > >
> > > > On Sat, Jul 15, 2023 at 7:14 PM Atish Patra
> > > > wrote:
> > > > >
> > > > > On Fri, Jul 14, 2023 at 5:29 AM Conor Dooley wrote:
> > > > > >
> > > > > > On Fri, Jul 14, 2023 at 11:19:34AM +0100, Conor Dooley wrote:
> > > > > > > On Fri, Jul 14, 2023 at 10:00:19AM +0530, Anup Patel wrote:
> > > > > > >
> > > > > > > > > > OpenSBI v1.3
> > > > > > > > > >_ _
> > > > > > > > > > / __ \ / | _ \_ _|
> > > > > > > > > > | | | |_ __ ___ _ __ | (___ | |_) || |
> > > > > > > > > > | | | | '_ \ / _ \ '_ \ \___ \| _ < | |
> > > > > > > > > > | |__| | |_) | __/ | | |) | |_) || |_
> > > > > > > > > > \/| .__/ \___|_| |_|_/|___/_|
> > > > > > > > > > | |
> > > > > > > > > > |_|
> > > > > > > > > >
> > > > > > > > > > init_coldboot: ipi init failed (error -1009)
> > > > > > > > > >
> > > > > > > > > > Just to note, because we use our own firmware that vendors
> > > > > > > > > > in OpenSBI
> > > > > > > > > > and compiles only a significantly cut down number of files
> > > > > > > > > > from it, we
> > > > > > > > > > do not use the fw_dynamic etc flow on our hardware. As a
> > > > > > > > > > result, we have
> > > > > > > > > > not tested v1.3, nor do we have any immediate plans to
> > > > > > > > > > change our
> > > > > > > > > > platform firmware to vendor v1.3 either.
> > > > > > > > > >
> > > > > > > > > > I unless there's something obvious to you, it sounds like I
> > > > > > > > > > will need to
> > > > > > > > > > go and bisect OpenSBI. That's a job for another day though,
> > > > > > > > > > given the
> > > > > > > > > > time.
> > > > > > > > > >
> > > > > > > >
> > > > > > > > The real issue is some CPU/HART DT nodes marked as disabled in
> > > > > > > > the
> > > > > > > > DT passed to OpenSBI 1.3.
> > > > > > > >
> > > > > > > > This issue does not exist in any of the DTs generated by QEMU
> > > > > > > > but some
> > > > > > > > of the DTs in the kernel (such as microchip and SiFive board
> > > > > > > > DTs) have
> > > > > > > > the E-core disabled.
> > > > > > > >
> > > > > > > > I had discovered this issue in a totally different context
> > > > > > > > after the OpenSBI 1.3
> > > > > > > > release happened. This issue is already fixed in the latest
> > > > > > > > OpenSBI by the
> > > > > > > > following commit c6a35733b74aeff612398f274ed19a74f81d1f37
> > > > > > > > ("lib: utils:
> > > > > > > > Fix sbi_hartid_to_scratch() usage in ACLINT drivers").
> > > > > > >
> > > > > > > Great, thanks Anup! I thought I had tested tip-of-tree too, but
> > > > > > > obviously not.
> > > > > > >
> > > > > > > > I always assumed that Microchip hss.bin is the preferred BIOS
> > > > > > > > for the
> > > > > > > > QEMU microchip-icicle-kit machine but I guess that's not true.
> > > > > > >
> > > > > > > Unfortunately the HSS has not worked in QEMU for a long time, and
> > > > > > > while
> > > > > > > I would love to fix it, but am pretty stretched for spare time to
> > > > > > > begin
> > > > > > > with.
> > > > > > > I usually just do direct kernel boots, which use the OpenSBI that
> > > > > > > comes
> > > > > > > with QEMU, as I am sure you already know :)
> > > > > > >
> > > > > > > > At this point, you can either:
> > > > > > > > 1) Use latest OpenSBI on QEMU microchip-icicle-kit machine
> > > > > >
> > > > > > I forgot to reply to this point, wondering what should be done with
> > > > > > QEMU. Bumping to v1.3 in QEMU introduces a regression here,
> > > > > > regardless
> > > > > > of whether I can go and build a fixed version of OpenSBI.
> > > > > >
> > > > > FYI: The no-map fix went in OpenSBI v1.3. Without the upgrade, any
> > > > > user using the latest kernel (> v6.4)
> > > > > may hit those random linear map related issues (in hibernation or EFI
> > > > > booting path).
> > > > >
> > > > > There are three possible scenarios:
> > > > >
> > > > > 1. Upgrade to OpenSBI v1.3: Any user of microchip-icicle-kit machine
> > > > > or sifive fu540 machine users
> > > > > may hit this issue if the device tree has the disabled hart (e core).
> > > > > 2. No upgrade to OpenSBI v1.2. Any user using hibernation or UEFI may
> > > > > have issues [1]
> > > > > 3. Include a non-release version OpenSBI in Qemu with the fix as an
> > > > > exception.
> > > > >
> > > > > #3 probably deviates from policy and sets a bad precedent. So I am not
> > > > > advocating for it though ;)
> > > > > For both #1 & #2, the solution would be to use the latest OpenSBI in
> > > > > -bios argument instead of the stock one.
> > > > > I could be wrong but my guess is the number of users facing #2 would
> > > > > be higher than #1.
> > > >
> > > > Thanks for
Re: [PATCH v4 0/6] net/tap: Fix QEMU frozen issue when the maximum number of file descriptors is very large
On Wed, Jun 28, 2023 at 11:29 PM Bin Meng wrote: > > > Current codes using a brute-force traversal of all file descriptors > do not scale on a system where the maximum number of file descriptors > is set to a very large value (e.g.: in a Docker container of Manjaro > distribution it is set to 1073741816). QEMU just looks frozen during > start-up. > > The close-on-exec flag (O_CLOEXEC) was introduced since Linux kernel > 2.6.23, FreeBSD 8.3, OpenBSD 5.0, Solaris 11. While it's true QEMU > doesn't need to manually close the fds for child process as the proper > O_CLOEXEC flag should have been set properly on files with its own > codes, QEMU uses a huge number of 3rd party libraries and we don't > trust them to reliably be using O_CLOEXEC on everything they open. > > Modern Linux and BSDs have the close_range() call we can use to do the > job, and on Linux we have one more way to walk through /proc/self/fd > to complete the task efficiently, which is what qemu_close_range() > does, a new API we add in util/osdep.c. > > V1 link: > https://lore.kernel.org/qemu-devel/20230406112041.798585-1-bm...@tinylab.org/ > > Changes in v4: > - add 'first > last' check logic > - reorder the ifdefs logic > - change i to unsigned int type > - use qemu_strtoi() instead of atoi() > - limit last upper value to sysconf(_SC_OPEN_MAX) - 1 > - call sysconf directly instead of using a variable > - put fd on its own line > > Changes in v3: > - fix win32 build failure > - limit the last_fd of qemu_close_range() to sysconf(_SC_OPEN_MAX) > > Changes in v2: > - new patch: "tests/tcg/cris: Fix the coding style" > - new patch: "tests/tcg/cris: Correct the off-by-one error" > - new patch: "util/async-teardown: Fall back to close fds one by one" > - new patch: "util/osdep: Introduce qemu_close_range()" > - new patch: "util/async-teardown: Use qemu_close_range() to close fds" > - Change to use qemu_close_range() to close fds for child process efficiently > - v1 link: > https://lore.kernel.org/qemu-devel/20230406112041.798585-1-bm...@tinylab.org/ > > Bin Meng (4): > tests/tcg/cris: Fix the coding style > tests/tcg/cris: Correct the off-by-one error > util/async-teardown: Fall back to close fds one by one > util/osdep: Introduce qemu_close_range() > > Zhangjin Wu (2): > util/async-teardown: Use qemu_close_range() to close fds > net: tap: Use qemu_close_range() to close fds > Ping for 8.1?
Re: [PATCH v7 0/9] net: Pad short frames for network backends
On Sun, Jun 25, 2023 at 9:59 AM Bin Meng wrote: > > The minimum Ethernet frame length is 60 bytes. For short frames with > smaller length like ARP packets (only 42 bytes), on a real world NIC > it can choose either padding its length to the minimum required 60 > bytes, or sending it out directly to the wire. Such behavior can be > hardcoded or controled by a register bit. Similarly on the receive > path, NICs can choose either dropping such short frames directly or > handing them over to software to handle. > > On the other hand, for the network backends like SLiRP/TAP, they > don't expose a way to control the short frame behavior. As of today > they just send/receive data from/to the other end connected to them, > which means any sized packet is acceptable. So they can send and > receive short frames without any problem. It is observed that ARP > packets sent from SLiRP/TAP are 42 bytes, and SLiRP/TAP just send > these ARP packets to the other end which might be a NIC model that > does not allow short frames to pass through. > > To provide better compatibility, for packets sent from QEMU network > backends like SLiRP/TAP, we change to pad short frames before sending > it out to the other end, if the other end does not forbid it via the > nc->do_not_pad flag. This ensures a backend as an Ethernet sender > does not violate the spec. But with this change, the behavior of > dropping short frames from SLiRP/TAP interfaces in the NIC model > cannot be emulated because it always receives a packet that is spec > complaint. The capability of sending short frames from NIC models is > still supported and short frames can still pass through SLiRP/TAP. > > This series should be able to fix the issue as reported with some > NIC models before, that ARP requests get dropped, preventing the > guest from becoming visible on the network. It was workarounded in > these NIC models on the receive path, that when a short frame is > received, it is padded up to 60 bytes. > > Only the first 4 patches of the v5 series [1] were applied in QEMU 6.0, > and the reset was said to be queued for 6.1 but for some reason they > never landed in QEMU mainline. > > Hopefully this series will make it for QEMU 8.1. > > [1] > https://lore.kernel.org/qemu-devel/859cd26a-feb2-ed62-98d5-764841a46...@redhat.com/ > > Changes in v7: > - new patch: "hw/net: ftgmac100: Drop the small packet check in the receive > path" > Ping?
[PATCH 1/2] roms/opensbi: Upgrade from v1.2 to v1.3
e: add gpio driver and support gpio reset 4b28afc make: Add a command line option for debugging OpenSBI e9d08bd lib: utils/i2c: Add minimal StarFive jh7110 I2C driver 568ea49 platform: starfive: add PMIC power ops in JH7110 visionfive2 board 506144f lib: serial: Cadence: Enable compatibility for cdns,uart-r1p8 1fe8dc9 lib: sbi_pmu: add callback for counter width 51951d9 lib: sbi_pmu: Implement sbi_pmu_counter_fw_read_hi 60c358e lib: sbi_pmu: Reserve space for implementation specific firmware events 548e4b4 lib: sbi_pmu: Rename fw_counter_value b51ddff lib: sbi_pmu: Update sbi_pmu dev ops 641d2e9 lib: sbi_pmu: Use dedicated event code for platform firmware events 57d3aa3 lib: sbi_pmu: Introduce fw_counter_write_value API c631a7d lib: sbi_pmu: Add hartid parameter PMU device ops d56049e lib: sbi: Refactor the calls to sbi_hart_switch_mode() e8e9ed3 lib: sbi: Set the state of a hart to START_PENDING after the hart is ready c6a092c lib: sbi: Clear IPIs before init_warm_startup in non-boot harts ed88a63 lib: sbi_scratch: Optimize the alignment code for alloc size 73ab11d lib: sbi: Fix how to check whether the domain contains fw_region f64dfcd lib: sbi: Introduce sbi_entry_count() function 30b9e7e lib: sbi_hsm: Fix sbi_hsm_hart_start() for platform with hart hotplug 8e90259 lib: sbi_hart: clear mip csr during hart init 45ba2b2 include: Add defines for SBI CPPC extension 33caae8 lib: sbi: Implement SBI CPPC extension 91767d0 lib: sbi: Print the CPPC device name edc9914 lib: sbi_pmu: Align the event type offset as per SBI specification ee016a7 docs: Correct FW_JUMP_FDT_ADDR calculation example 2868f26 lib: utils: fdt_fixup: avoid buffer overrun 66fa925 lib: sbi: Optimize sbi_tlb 24dde46 lib: sbi: Optimize sbi_ipi 80078ab sbi: tlb: Simplify to tlb_process_count/tlb_process function bf40e07 lib: sbi: Optimize sbi_tlb queue waiting eeab500 platform: generic: andes/renesas: Add SBI EXT to check for enabling IOCP errata f692289 firmware: Optimize loading relocation type e41dbb5 firmware: Change to use positive offset to access relocation entries bdb3c42 lib: sbi: Do not clear active_events for cycle/instret when stopping 674e019 lib: sbi: Fix counter index calculation for SBI_PMU_CFG_FLAG_SKIP_MATCH f5dfd99 lib: sbi: Don't check SBI error range for legacy console getchar 7919530 lib: sbi: Add debug print when sbi_pmu_init fails 4e33530 lib: sbi: Remove unnecessary semicolon 6bc02de lib: sbi: Simplify sbi_ipi_process remove goto dc1c7db lib: sbi: Simplify BITS_PER_LONG definition f58c140 lib: sbi: Introduce register_extensions extension callback e307ba7 lib: sbi: Narrow vendor extension range 042f0c3 lib: sbi: pmu: Remove unnecessary probe function 8b952d4 lib: sbi: Only register available extensions 767b5fc lib: sbi: Optimize probe of srst/susp c3e31cb lib: sbi: Remove 0/1 probe implementations 33f1722 lib: sbi: Document sbi_ecall_extension members d4c46e0 Makefile: Dereference symlinks on install 8b99a7f lib: sbi: Fix return of sbi_console_init 264d0be lib: utils: Improve fdt_serial_init 9a0bdd0 lib: utils: Improve fdt_ipi 122f226 lib: utils: Improve fdt_timer df75e09 lib: utils/ipi: buffer overrun aclint_mswi_cold_init bdde2ec lib: sbi: Align system suspend errors with spec aad7a37 include: sbi_scratch: Add helper macros to access data type 5cf9a54 platform: Allow platforms to specify heap size 40d36a6 lib: sbi: Introduce simple heap allocator 2a04f70 lib: sbi: Print scratch size and usage at boot time bbff53f lib: sbi_pmu: Use heap for per-HART PMU state ef4542d lib: sbi: Use heap for root domain creation 66daafe lib: sbi: Use scratch space to save per-HART domain pointer fa5ad2e lib: utils/gpio: Use heap in SiFive and StartFive GPIO drivers 903e88c lib: utils/i2c: Use heap in DesignWare and SiFive I2C drivers 5a8cfcd lib: utils/ipi: Use heap in ACLINT MSWI driver 3013716 lib: utils/irqchip: Use heap in PLIC, APLIC and IMSIC drivers 7e5636a lib: utils/timer: Use heap in ACLINT MTIMER driver 3c1c972 lib: utils/fdt: Use heap in FDT domain parsing acbd8fc lib: utils/ipi: Use scratch space to save per-HART MSWI pointer f0516be lib: utils/timer: Use scratch space to save per-HART MTIMER pointer b3594ac lib: utils/irqchip: Use scratch space to save per-HART PLIC pointer 1df52fa lib: utils/irqchip: Don't check hartid in imsic_update_hartid_table() 355796c lib: utils/irqchip: Use scratch space to save per-HART IMSIC pointer 524feec docs: Add OpenSBI logo and use it in the top-level README.md 932be2c README.md: Improve project copyright information 8153b26 platform/lib: Set no-map attribute on all PMP regions d64942f firmware: Fix find hart index 27c957a lib: reset: Move fdt_reset_init into generic_early_init 8bd666a lib: sbi: check A2 register in ecall_dbcn_handler. 2552799 include: Bump-up version to 1.3 Signed-off-by: Bin Meng --- Please pull the complete patch from https://github.com/lbmeng/qemu opensbi branch. .../opensbi-riscv32-generic-fw_dynamic.bin| Bin 123072 -> 135344 bytes .../opensbi-ris
[PATCH 2/2] tests/avocado: riscv: Enable 32-bit Spike OpenSBI boot testing
The 32-bit Spike boot issue has been fixed in the OpenSBI v1.3.
Let's enable the 32-bit Spike OpenSBI boot testing.
Signed-off-by: Bin Meng
---
tests/avocado/riscv_opensbi.py | 2 --
1 file changed, 2 deletions(-)
diff --git a/tests/avocado/riscv_opensbi.py b/tests/avocado/riscv_opensbi.py
index e02f0d404a..bfff9cc3c3 100644
--- a/tests/avocado/riscv_opensbi.py
+++ b/tests/avocado/riscv_opensbi.py
@@ -6,7 +6,6 @@
# later. See the COPYING file in the top-level directory.
from avocado_qemu import QemuSystemTest
-from avocado import skip
from avocado_qemu import wait_for_console_pattern
class RiscvOpenSBI(QemuSystemTest):
@@ -21,7 +20,6 @@ def boot_opensbi(self):
wait_for_console_pattern(self, 'Platform Name')
wait_for_console_pattern(self, 'Boot HART MEDELEG')
-@skip("requires OpenSBI fix to work")
def test_riscv32_spike(self):
"""
:avocado: tags=arch:riscv32
--
2.34.1
Re: [PATCH v3 4/6] util/osdep: Introduce qemu_close_range()
On 2023/6/19 17:22:53, "Markus Armbruster" wrote:
Bin Meng writes:
This introduces a new QEMU API qemu_close_range() that closes all
open file descriptors from first to last (included).
This API will try a more efficient call to close_range(), or walk
through of /proc/self/fd whenever these are possible, otherwise it
falls back to a plain close loop.
Co-developed-by: Zhangjin Wu
Signed-off-by: Bin Meng
---
Changes in v3:
- fix win32 build failure
Changes in v2:
- new patch: "util/osdep: Introduce qemu_close_range()"
include/qemu/osdep.h | 1 +
util/osdep.c | 48
2 files changed, 49 insertions(+)
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index cc61b00ba9..e22434ce10 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -560,6 +560,7 @@ int qemu_open_old(const char *name, int flags, ...);
int qemu_open(const char *name, int flags, Error **errp);
int qemu_create(const char *name, int flags, mode_t mode, Error **errp);
int qemu_close(int fd);
+int qemu_close_range(unsigned int first, unsigned int last);
int qemu_unlink(const char *name);
#ifndef _WIN32
int qemu_dup_flags(int fd, int flags);
diff --git a/util/osdep.c b/util/osdep.c
index e996c4744a..91275e70f8 100644
--- a/util/osdep.c
+++ b/util/osdep.c
@@ -30,6 +30,7 @@
#include "qemu/mprotect.h"
#include "qemu/hw-version.h"
#include "monitor/monitor.h"
+#include
static const char *hw_version = QEMU_HW_VERSION;
@@ -411,6 +412,53 @@ int qemu_close(int fd)
return close(fd);
}
+int qemu_close_range(unsigned int first, unsigned int last)
+{
+DIR *dir = NULL;
+
+#ifdef CONFIG_CLOSE_RANGE
+int r = close_range(first, last, 0);
close_range(2) explains flag
CLOSE_RANGE_UNSHARE
Unshare the specified file descriptors from any other processes
before closing them, avoiding races with other threads sharing
the file descriptor table.
Can anybody explain the races this avoids?
The kernel commit [1] which adds the close_range syscall says:
unshare(CLONE_FILES) should only be needed if the calling
task is multi-threaded and shares the file descriptor table with another
thread in which case two threads could race with one thread allocating
file
descriptors and the other one closing them via close_range().
+if (!r) {
+/* Success, no need to try other ways. */
+return 0;
+}
What are the failure modes of close_range() where the other ways are
worth trying?
Added first < last check in v4 so that the technically close_range()
should not fail.
+#endif
+
+#ifdef __linux__
+dir = opendir("/proc/self/fd");
+#endif
+if (!dir) {
+/*
+ * If /proc is not mounted or /proc/self/fd is not supported,
+ * try close() from first to last.
+ */
+for (int i = first; i <= last; i++) {
+close(i);
+}
+
+return 0;
+}
+
+#ifndef _WIN32
+/* Avoid closing the directory */
+int dfd = dirfd(dir);
This directory contains "." "..", "0", "1", ...
+
+for (struct dirent *de = readdir(dir); de; de = readdir(dir)) {
+int fd = atoi(de->d_name);
Maps "." and ".." to 0. Unclean.
Please use qemu_strtoi(de->d_name, NULL, 10, &fd), and skip entries
where it fails.
Fixed in v4.
+if (fd < first || fd > last) {
+/* Exclude the fds outside the target range */
+continue;
+}
+if (fd != dfd) {
+close(fd);
+}
+}
+closedir(dir);
+#endif /* _WIN32 */
+
+return 0;
+}
I'd prefer to order this from most to least preferred:
close_range()
iterate over /proc/self/fd
iterate from first to last
Unlike close_range(), qemu_close_range() returns 0 when last < first.
If we want to deviate from close_range(), we better document the
differences.
This is a generalized version of async-teardown.c's close_all_open_fd().
I'd mention this in the commit message. Suggestion, not demand.
[1]
https://github.com/torvalds/linux/commit/278a5fbaed89dacd04e9d052f4594ffd0e0585de
Regards,
Bin
[PATCH v4 5/6] util/async-teardown: Use qemu_close_range() to close fds
From: Zhangjin Wu
Based on the old close_all_open_fd() of util/async-teardown.c, a new
generic qemu_close_range() has been added in osdep.c.
Now, let's switch over to use the generic qemu_close_range().
Signed-off-by: Zhangjin Wu
Signed-off-by: Bin Meng
---
Changes in v4:
- call sysconf directly instead of using a variable
Changes in v3:
- limit the last_fd of qemu_close_range() to sysconf(_SC_OPEN_MAX)
Changes in v2:
- new patch: "util/async-teardown: Use qemu_close_range() to close fds"
util/async-teardown.c | 41 +
1 file changed, 1 insertion(+), 40 deletions(-)
diff --git a/util/async-teardown.c b/util/async-teardown.c
index 7e0177a8da..a038a255ff 100644
--- a/util/async-teardown.c
+++ b/util/async-teardown.c
@@ -29,44 +29,6 @@
static pid_t the_ppid;
-/*
- * Close all open file descriptors.
- */
-static void close_all_open_fd(void)
-{
-struct dirent *de;
-int fd, dfd;
-DIR *dir;
-
-#ifdef CONFIG_CLOSE_RANGE
-int r = close_range(0, ~0U, 0);
-if (!r) {
-/* Success, no need to try other ways. */
-return;
-}
-#endif
-
-dir = opendir("/proc/self/fd");
-if (!dir) {
-/* If /proc is not mounted, close fds one by one. */
-int open_max = sysconf(_SC_OPEN_MAX), i;
-for (i = 0; i < open_max; i++) {
-close(i);
-}
-return;
-}
-/* Avoid closing the directory. */
-dfd = dirfd(dir);
-
-for (de = readdir(dir); de; de = readdir(dir)) {
-fd = atoi(de->d_name);
-if (fd != dfd) {
-close(fd);
-}
-}
-closedir(dir);
-}
-
static void hup_handler(int signal)
{
/* Check every second if this process has been reparented. */
@@ -92,9 +54,8 @@ static int async_teardown_fn(void *arg)
/*
* Close all file descriptors that might have been inherited from the
* main qemu process when doing clone, needed to make libvirt happy.
- * Not using close_range for increased compatibility with older kernels.
*/
-close_all_open_fd();
+qemu_close_range(0, sysconf(_SC_OPEN_MAX) - 1);
/* Set up a handler for SIGHUP and unblock SIGHUP. */
sigaction(SIGHUP, &sa, NULL);
--
2.34.1
[PATCH v4 3/6] util/async-teardown: Fall back to close fds one by one
When opening /proc/self/fd fails, current codes just return directly,
but we can fall back to close fds one by one.
Signed-off-by: Bin Meng
---
- feel free to drop this patch if it does not make too much sense
(no changes since v2)
Changes in v2:
- new patch: "util/async-teardown: Fall back to close fds one by one"
util/async-teardown.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/util/async-teardown.c b/util/async-teardown.c
index 3ab19c8740..7e0177a8da 100644
--- a/util/async-teardown.c
+++ b/util/async-teardown.c
@@ -48,7 +48,11 @@ static void close_all_open_fd(void)
dir = opendir("/proc/self/fd");
if (!dir) {
-/* If /proc is not mounted, there is nothing that can be done. */
+/* If /proc is not mounted, close fds one by one. */
+int open_max = sysconf(_SC_OPEN_MAX), i;
+for (i = 0; i < open_max; i++) {
+close(i);
+}
return;
}
/* Avoid closing the directory. */
--
2.34.1
[PATCH v4 4/6] util/osdep: Introduce qemu_close_range()
This introduces a new QEMU API qemu_close_range() that closes all
open file descriptors from first to last (included).
This API will try a more efficient call to close_range(), or walk
through of /proc/self/fd whenever these are possible, otherwise it
falls back to a plain close loop.
Co-developed-by: Zhangjin Wu
Signed-off-by: Bin Meng
---
Changes in v4:
- add 'first > last' check logic
- reorder the ifdefs logic
- change i to unsigned int type
- use qemu_strtoi() instead of atoi()
- limit last upper value to sysconf(_SC_OPEN_MAX) - 1
Changes in v3:
- fix win32 build failure
Changes in v2:
- new patch: "util/osdep: Introduce qemu_close_range()"
include/qemu/osdep.h | 1 +
util/osdep.c | 60
2 files changed, 61 insertions(+)
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index cc61b00ba9..e22434ce10 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -560,6 +560,7 @@ int qemu_open_old(const char *name, int flags, ...);
int qemu_open(const char *name, int flags, Error **errp);
int qemu_create(const char *name, int flags, mode_t mode, Error **errp);
int qemu_close(int fd);
+int qemu_close_range(unsigned int first, unsigned int last);
int qemu_unlink(const char *name);
#ifndef _WIN32
int qemu_dup_flags(int fd, int flags);
diff --git a/util/osdep.c b/util/osdep.c
index e996c4744a..1d8c719b3f 100644
--- a/util/osdep.c
+++ b/util/osdep.c
@@ -411,6 +411,66 @@ int qemu_close(int fd)
return close(fd);
}
+int qemu_close_range(unsigned int first, unsigned int last)
+{
+if (first > last) {
+errno = EINVAL;
+return -1;
+}
+
+#ifndef _WIN32
+if (last >= sysconf(_SC_OPEN_MAX)) {
+last = sysconf(_SC_OPEN_MAX) - 1;
+}
+#endif
+
+#ifdef CONFIG_CLOSE_RANGE
+int r = close_range(first, last, 0);
+if (!r) {
+/* Success, no need to try other ways */
+return 0;
+}
+#endif
+
+#ifdef __linux__
+DIR *dir = opendir("/proc/self/fd");
+if (dir) {
+/* Avoid closing the directory */
+int dfd = dirfd(dir);
+
+for (struct dirent *de = readdir(dir); de; de = readdir(dir)) {
+int fd, ret;
+
+ret = qemu_strtoi(de->d_name, NULL, 10, &fd);
+if (ret) {
+/* skip "." and ".." */
+continue;
+}
+if (fd < first || fd > last) {
+/* Exclude the fds outside the target range */
+continue;
+}
+if (fd != dfd) {
+close(fd);
+}
+}
+closedir(dir);
+
+return 0;
+}
+#endif
+
+/*
+ * If /proc is not mounted or /proc/self/fd is not supported,
+ * try close() from first to last.
+ */
+for (unsigned int i = first; i <= last; i++) {
+close(i);
+}
+
+return 0;
+}
+
/*
* Delete a file from the filesystem, unless the filename is /dev/fdset/...
*
--
2.34.1
[PATCH v4 6/6] net: tap: Use qemu_close_range() to close fds
From: Zhangjin Wu
Current codes using a brute-force traversal of all file descriptors
do not scale on a system where the maximum number of file descriptors
is set to a very large value (e.g.: in a Docker container of Manjaro
distribution it is set to 1073741816). QEMU just looks frozen during
start-up.
The close-on-exec flag (O_CLOEXEC) was introduced since Linux kernel
2.6.23, FreeBSD 8.3, OpenBSD 5.0, Solaris 11. While it's true QEMU
doesn't need to manually close the fds for child process as the proper
O_CLOEXEC flag should have been set properly on files with its own
codes, QEMU uses a huge number of 3rd party libraries and we don't
trust them to reliably be using O_CLOEXEC on everything they open.
Modern Linux and BSDs have the close_range() call we can use to do the
job, and on Linux we have one more way to walk through /proc/self/fd
to complete the task efficiently, which is what qemu_close_range() does.
Reported-by: Zhangjin Wu
Co-developed-by: Bin Meng
Signed-off-by: Zhangjin Wu
Signed-off-by: Bin Meng
Reviewed-by: Richard Henderson
---
Changes in v4:
- put fd on its own line
Changes in v2:
- Change to use qemu_close_range() to close fds for child process efficiently
- v1 link:
https://lore.kernel.org/qemu-devel/20230406112041.798585-1-bm...@tinylab.org/
net/tap.c | 24
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/net/tap.c b/net/tap.c
index 1bf085d422..9f080215f0 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -446,13 +446,13 @@ static void launch_script(const char *setup_script, const
char *ifname,
return;
}
if (pid == 0) {
-int open_max = sysconf(_SC_OPEN_MAX), i;
+unsigned int last_fd = sysconf(_SC_OPEN_MAX) - 1;
+
+/* skip stdin, stdout and stderr */
+qemu_close_range(3, fd - 1);
+/* skip the currently used fd */
+qemu_close_range(fd + 1, last_fd);
-for (i = 3; i < open_max; i++) {
-if (i != fd) {
-close(i);
-}
-}
parg = args;
*parg++ = (char *)setup_script;
*parg++ = (char *)ifname;
@@ -536,16 +536,16 @@ static int net_bridge_run_helper(const char *helper,
const char *bridge,
return -1;
}
if (pid == 0) {
-int open_max = sysconf(_SC_OPEN_MAX), i;
+unsigned int last_fd = sysconf(_SC_OPEN_MAX) - 1;
+unsigned int fd = sv[1];
char *fd_buf = NULL;
char *br_buf = NULL;
char *helper_cmd = NULL;
-for (i = 3; i < open_max; i++) {
-if (i != sv[1]) {
-close(i);
-}
-}
+/* skip stdin, stdout and stderr */
+qemu_close_range(3, fd - 1);
+/* skip the currently used fd */
+qemu_close_range(fd + 1, last_fd);
fd_buf = g_strdup_printf("%s%d", "--fd=", sv[1]);
--
2.34.1
[PATCH v4 0/6] net/tap: Fix QEMU frozen issue when the maximum number of file descriptors is very large
Current codes using a brute-force traversal of all file descriptors do not scale on a system where the maximum number of file descriptors is set to a very large value (e.g.: in a Docker container of Manjaro distribution it is set to 1073741816). QEMU just looks frozen during start-up. The close-on-exec flag (O_CLOEXEC) was introduced since Linux kernel 2.6.23, FreeBSD 8.3, OpenBSD 5.0, Solaris 11. While it's true QEMU doesn't need to manually close the fds for child process as the proper O_CLOEXEC flag should have been set properly on files with its own codes, QEMU uses a huge number of 3rd party libraries and we don't trust them to reliably be using O_CLOEXEC on everything they open. Modern Linux and BSDs have the close_range() call we can use to do the job, and on Linux we have one more way to walk through /proc/self/fd to complete the task efficiently, which is what qemu_close_range() does, a new API we add in util/osdep.c. V1 link: https://lore.kernel.org/qemu-devel/20230406112041.798585-1-bm...@tinylab.org/ Changes in v4: - add 'first > last' check logic - reorder the ifdefs logic - change i to unsigned int type - use qemu_strtoi() instead of atoi() - limit last upper value to sysconf(_SC_OPEN_MAX) - 1 - call sysconf directly instead of using a variable - put fd on its own line Changes in v3: - fix win32 build failure - limit the last_fd of qemu_close_range() to sysconf(_SC_OPEN_MAX) Changes in v2: - new patch: "tests/tcg/cris: Fix the coding style" - new patch: "tests/tcg/cris: Correct the off-by-one error" - new patch: "util/async-teardown: Fall back to close fds one by one" - new patch: "util/osdep: Introduce qemu_close_range()" - new patch: "util/async-teardown: Use qemu_close_range() to close fds" - Change to use qemu_close_range() to close fds for child process efficiently - v1 link: https://lore.kernel.org/qemu-devel/20230406112041.798585-1-bm...@tinylab.org/ Bin Meng (4): tests/tcg/cris: Fix the coding style tests/tcg/cris: Correct the off-by-one error util/async-teardown: Fall back to close fds one by one util/osdep: Introduce qemu_close_range() Zhangjin Wu (2): util/async-teardown: Use qemu_close_range() to close fds net: tap: Use qemu_close_range() to close fds include/qemu/osdep.h| 1 + net/tap.c | 24 ++-- tests/tcg/cris/libc/check_openpf5.c | 57 +-- util/async-teardown.c | 37 +- util/osdep.c| 60 + 5 files changed, 101 insertions(+), 78 deletions(-) -- 2.34.1
[PATCH v4 1/6] tests/tcg/cris: Fix the coding style
The code style does not conform with QEMU's. Correct it so that the
upcoming commit does not trigger checkpatch warnings.
Signed-off-by: Bin Meng
Acked-by: Richard Henderson
Reviewed-by: Philippe Mathieu-Daudé
---
(no changes since v2)
Changes in v2:
- new patch: "tests/tcg/cris: Fix the coding style"
tests/tcg/cris/libc/check_openpf5.c | 57 ++---
1 file changed, 27 insertions(+), 30 deletions(-)
diff --git a/tests/tcg/cris/libc/check_openpf5.c
b/tests/tcg/cris/libc/check_openpf5.c
index 1f86ea283d..0037fbca4c 100644
--- a/tests/tcg/cris/libc/check_openpf5.c
+++ b/tests/tcg/cris/libc/check_openpf5.c
@@ -13,44 +13,41 @@
#include
#include
-int main (int argc, char *argv[])
+int main(int argc, char *argv[])
{
- int i;
- int filemax;
+int i;
+int filemax;
#ifdef OPEN_MAX
- filemax = OPEN_MAX;
+filemax = OPEN_MAX;
#else
- filemax = sysconf (_SC_OPEN_MAX);
+filemax = sysconf(_SC_OPEN_MAX);
#endif
- char *fn = malloc (strlen (argv[0]) + 2);
- if (fn == NULL)
-abort ();
- strcpy (fn, "/");
- strcat (fn, argv[0]);
+char *fn = malloc(strlen(argv[0]) + 2);
+if (fn == NULL) {
+abort();
+}
+strcpy(fn, "/");
+strcat(fn, argv[0]);
- for (i = 0; i < filemax + 1; i++)
-{
- if (open (fn, O_RDONLY) < 0)
- {
- /* Shouldn't happen too early. */
- if (i < filemax - 3 - 1)
- {
- fprintf (stderr, "i: %d\n", i);
- abort ();
- }
- if (errno != EMFILE)
- {
- perror ("open");
- abort ();
- }
- goto ok;
- }
+for (i = 0; i < filemax + 1; i++) {
+if (open(fn, O_RDONLY) < 0) {
+/* Shouldn't happen too early. */
+if (i < filemax - 3 - 1) {
+fprintf(stderr, "i: %d\n", i);
+abort();
+}
+if (errno != EMFILE) {
+perror("open");
+abort();
+}
+goto ok;
+}
}
- abort ();
+abort();
ok:
- printf ("pass\n");
- exit (0);
+printf("pass\n");
+exit(0);
}
--
2.34.1
[PATCH v4 2/6] tests/tcg/cris: Correct the off-by-one error
sysconf(_SC_OPEN_MAX) returns the maximum number of files that
a process can have open at any time, which means the fd should
not be larger than or equal to the return value.
Signed-off-by: Bin Meng
Acked-by: Richard Henderson
Reviewed-by: Philippe Mathieu-Daudé
---
(no changes since v2)
Changes in v2:
- new patch: "tests/tcg/cris: Correct the off-by-one error"
tests/tcg/cris/libc/check_openpf5.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/tcg/cris/libc/check_openpf5.c
b/tests/tcg/cris/libc/check_openpf5.c
index 0037fbca4c..7f585c6d37 100644
--- a/tests/tcg/cris/libc/check_openpf5.c
+++ b/tests/tcg/cris/libc/check_openpf5.c
@@ -31,10 +31,10 @@ int main(int argc, char *argv[])
strcpy(fn, "/");
strcat(fn, argv[0]);
-for (i = 0; i < filemax + 1; i++) {
+for (i = 0; i < filemax; i++) {
if (open(fn, O_RDONLY) < 0) {
/* Shouldn't happen too early. */
-if (i < filemax - 3 - 1) {
+if (i < filemax - 3) {
fprintf(stderr, "i: %d\n", i);
abort();
}
--
2.34.1
Re: [PATCH v3 4/6] util/osdep: Introduce qemu_close_range()
On 2023/6/19 17:18:53, "Claudio Imbrenda"
wrote:
On Sat, 17 Jun 2023 13:36:19 +0800
Bin Meng wrote:
This introduces a new QEMU API qemu_close_range() that closes all
open file descriptors from first to last (included).
This API will try a more efficient call to close_range(), or walk
through of /proc/self/fd whenever these are possible, otherwise it
falls back to a plain close loop.
Co-developed-by: Zhangjin Wu
Signed-off-by: Bin Meng
---
Changes in v3:
- fix win32 build failure
Changes in v2:
- new patch: "util/osdep: Introduce qemu_close_range()"
include/qemu/osdep.h | 1 +
util/osdep.c | 48
2 files changed, 49 insertions(+)
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index cc61b00ba9..e22434ce10 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -560,6 +560,7 @@ int qemu_open_old(const char *name, int flags, ...);
int qemu_open(const char *name, int flags, Error **errp);
int qemu_create(const char *name, int flags, mode_t mode, Error **errp);
int qemu_close(int fd);
+int qemu_close_range(unsigned int first, unsigned int last);
int qemu_unlink(const char *name);
#ifndef _WIN32
int qemu_dup_flags(int fd, int flags);
diff --git a/util/osdep.c b/util/osdep.c
index e996c4744a..91275e70f8 100644
--- a/util/osdep.c
+++ b/util/osdep.c
@@ -30,6 +30,7 @@
#include "qemu/mprotect.h"
#include "qemu/hw-version.h"
#include "monitor/monitor.h"
+#include
static const char *hw_version = QEMU_HW_VERSION;
@@ -411,6 +412,53 @@ int qemu_close(int fd)
return close(fd);
}
+int qemu_close_range(unsigned int first, unsigned int last)
+{
+DIR *dir = NULL;
+
+#ifdef CONFIG_CLOSE_RANGE
+int r = close_range(first, last, 0);
+if (!r) {
+/* Success, no need to try other ways. */
+return 0;
+}
+#endif
+
+#ifdef __linux__
+dir = opendir("/proc/self/fd");
+#endif
+if (!dir) {
+/*
+ * If /proc is not mounted or /proc/self/fd is not supported,
+ * try close() from first to last.
+ */
+for (int i = first; i <= last; i++) {
+close(i);
will this compile on windows?
Yes, it will.
Regards,
Bin
Re: [PATCH v3 3/6] util/async-teardown: Fall back to close fds one by one
On 2023/6/19 17:18:23, "Claudio Imbrenda" wrote:
On Sat, 17 Jun 2023 13:36:18 +0800
Bin Meng wrote:
When opening /proc/self/fd fails, current codes just return directly,
but we can fall back to close fds one by one.
Signed-off-by: Bin Meng
---
(no changes since v2)
Changes in v2:
- new patch: "util/async-teardown: Fall back to close fds one by one"
util/async-teardown.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/util/async-teardown.c b/util/async-teardown.c
index 3ab19c8740..7e0177a8da 100644
--- a/util/async-teardown.c
+++ b/util/async-teardown.c
@@ -48,7 +48,11 @@ static void close_all_open_fd(void)
dir = opendir("/proc/self/fd");
if (!dir) {
-/* If /proc is not mounted, there is nothing that can be done. */
+/* If /proc is not mounted, close fds one by one. */
+int open_max = sysconf(_SC_OPEN_MAX), i;
+for (i = 0; i < open_max; i++) {
+close(i);
+}
return;
}
/* Avoid closing the directory. */
a few patches later, you replace the whole close_all_open_fd() with a
generic version, I don't see a point in changing the code here.
I meant to do a 100% replacement, and this patch added the missing loop.
this patch is useless, just drop it
Regards,
Bin
[PATCH v7 1/9] hw/net: e1000: Remove the logic of padding short frames in the receive path
Now that we have implemented unified short frames padding in the
QEMU networking codes, remove the same logic in the NIC codes.
This actually reverts commit 78aeb23eded2d0b765bf9145c71f80025b568acd.
Signed-off-by: Bin Meng
---
(no changes since v1)
hw/net/e1000.c | 11 +--
1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index aae5f0bdc0..093c2d4531 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -888,7 +888,6 @@ e1000_receive_iov(NetClientState *nc, const struct iovec
*iov, int iovcnt)
uint16_t vlan_special = 0;
uint8_t vlan_status = 0;
uint8_t min_buf[ETH_ZLEN];
-struct iovec min_iov;
uint8_t *filter_buf = iov->iov_base;
size_t size = iov_size(iov, iovcnt);
size_t iov_ofs = 0;
@@ -905,15 +904,7 @@ e1000_receive_iov(NetClientState *nc, const struct iovec
*iov, int iovcnt)
return 0;
}
-/* Pad to minimum Ethernet frame length */
-if (size < sizeof(min_buf)) {
-iov_to_buf(iov, iovcnt, 0, min_buf, size);
-memset(&min_buf[size], 0, sizeof(min_buf) - size);
-min_iov.iov_base = filter_buf = min_buf;
-min_iov.iov_len = size = sizeof(min_buf);
-iovcnt = 1;
-iov = &min_iov;
-} else if (iov->iov_len < MAXIMUM_ETHERNET_HDR_LEN) {
+if (iov->iov_len < MAXIMUM_ETHERNET_HDR_LEN) {
/* This is very unlikely, but may happen. */
iov_to_buf(iov, iovcnt, 0, min_buf, MAXIMUM_ETHERNET_HDR_LEN);
filter_buf = min_buf;
--
2.34.1
[PATCH v7 2/9] hw/net: vmxnet3: Remove the logic of padding short frames in the receive path
Now that we have implemented unified short frames padding in the
QEMU networking codes, remove the same logic in the NIC codes.
This actually reverts commit 40a87c6c9b11ef9c14e0301f76abf0eb2582f08e.
Signed-off-by: Bin Meng
---
(no changes since v1)
hw/net/vmxnet3.c | 10 --
1 file changed, 10 deletions(-)
diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
index 18b9edfdb2..5dfacb1098 100644
--- a/hw/net/vmxnet3.c
+++ b/hw/net/vmxnet3.c
@@ -40,7 +40,6 @@
#define PCI_DEVICE_ID_VMWARE_VMXNET3_REVISION 0x1
#define VMXNET3_MSIX_BAR_SIZE 0x2000
-#define MIN_BUF_SIZE 60
/* Compatibility flags for migration */
#define VMXNET3_COMPAT_FLAG_OLD_MSI_OFFSETS_BIT 0
@@ -1977,7 +1976,6 @@ vmxnet3_receive(NetClientState *nc, const uint8_t *buf,
size_t size)
{
VMXNET3State *s = qemu_get_nic_opaque(nc);
size_t bytes_indicated;
-uint8_t min_buf[MIN_BUF_SIZE];
if (!vmxnet3_can_receive(nc)) {
VMW_PKPRN("Cannot receive now");
@@ -1990,14 +1988,6 @@ vmxnet3_receive(NetClientState *nc, const uint8_t *buf,
size_t size)
size -= sizeof(struct virtio_net_hdr);
}
-/* Pad to minimum Ethernet frame length */
-if (size < sizeof(min_buf)) {
-memcpy(min_buf, buf, size);
-memset(&min_buf[size], 0, sizeof(min_buf) - size);
-buf = min_buf;
-size = sizeof(min_buf);
-}
-
net_rx_pkt_set_packet_type(s->rx_pkt,
get_eth_packet_type(PKT_GET_ETH_HDR(buf)));
--
2.34.1
[PATCH v7 8/9] hw/net: sunhme: Remove the logic of padding short frames in the receive path
Now that we have implemented unified short frames padding in the
QEMU networking codes, remove the same logic in the NIC codes.
Signed-off-by: Bin Meng
---
(no changes since v1)
hw/net/sunhme.c | 11 ---
1 file changed, 11 deletions(-)
diff --git a/hw/net/sunhme.c b/hw/net/sunhme.c
index 1f3d8011ae..391d26fb82 100644
--- a/hw/net/sunhme.c
+++ b/hw/net/sunhme.c
@@ -714,8 +714,6 @@ static inline void sunhme_set_rx_ring_nr(SunHMEState *s,
int i)
s->erxregs[HME_ERXI_RING >> 2] = ring;
}
-#define MIN_BUF_SIZE 60
-
static ssize_t sunhme_receive(NetClientState *nc, const uint8_t *buf,
size_t size)
{
@@ -724,7 +722,6 @@ static ssize_t sunhme_receive(NetClientState *nc, const
uint8_t *buf,
dma_addr_t rb, addr;
uint32_t intstatus, status, buffer, buffersize, sum;
uint16_t csum;
-uint8_t buf1[60];
int nr, cr, len, rxoffset, csum_offset;
trace_sunhme_rx_incoming(size);
@@ -775,14 +772,6 @@ static ssize_t sunhme_receive(NetClientState *nc, const
uint8_t *buf,
trace_sunhme_rx_filter_accept();
-/* If too small buffer, then expand it */
-if (size < MIN_BUF_SIZE) {
-memcpy(buf1, buf, size);
-memset(buf1 + size, 0, MIN_BUF_SIZE - size);
-buf = buf1;
-size = MIN_BUF_SIZE;
-}
-
rb = s->erxregs[HME_ERXI_RING >> 2] & HME_ERXI_RING_ADDR;
nr = sunhme_get_rx_ring_count(s);
cr = sunhme_get_rx_ring_nr(s);
--
2.34.1
[PATCH v7 6/9] hw/net: rtl8139: Remove the logic of padding short frames in the receive path
Now that we have implemented unified short frames padding in the
QEMU networking codes, remove the same logic in the NIC codes.
Signed-off-by: Bin Meng
---
(no changes since v1)
hw/net/rtl8139.c | 12
1 file changed, 12 deletions(-)
diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c
index 5f1a4d359b..b4df75b2c9 100644
--- a/hw/net/rtl8139.c
+++ b/hw/net/rtl8139.c
@@ -826,7 +826,6 @@ static ssize_t rtl8139_do_receive(NetClientState *nc, const
uint8_t *buf, size_t
uint32_t packet_header = 0;
-uint8_t buf1[MIN_BUF_SIZE + VLAN_HLEN];
static const uint8_t broadcast_macaddr[6] =
{ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
@@ -938,17 +937,6 @@ static ssize_t rtl8139_do_receive(NetClientState *nc,
const uint8_t *buf, size_t
}
}
-/* if too small buffer, then expand it
- * Include some tailroom in case a vlan tag is later removed. */
-if (size < MIN_BUF_SIZE + VLAN_HLEN) {
-memcpy(buf1, buf, size);
-memset(buf1 + size, 0, MIN_BUF_SIZE + VLAN_HLEN - size);
-buf = buf1;
-if (size < MIN_BUF_SIZE) {
-size = MIN_BUF_SIZE;
-}
-}
-
if (rtl8139_cp_receiver_enabled(s))
{
if (!rtl8139_cp_rx_valid(s)) {
--
2.34.1
[PATCH v7 9/9] hw/net: ftgmac100: Drop the small packet check in the receive path
Now that we have implemented unified short frames padding in the
QEMU networking codes, the small packet check logic in the receive
path is no longer needed.
Suggested-by: Cédric Le Goater
Signed-off-by: Bin Meng
---
Changes in v7:
- new patch: "hw/net: ftgmac100: Drop the small packet check in the receive
path"
hw/net/ftgmac100.c | 8
1 file changed, 8 deletions(-)
diff --git a/hw/net/ftgmac100.c b/hw/net/ftgmac100.c
index d3bf14be53..702b001be2 100644
--- a/hw/net/ftgmac100.c
+++ b/hw/net/ftgmac100.c
@@ -968,14 +968,6 @@ static ssize_t ftgmac100_receive(NetClientState *nc, const
uint8_t *buf,
return -1;
}
-/* TODO : Pad to minimum Ethernet frame length */
-/* handle small packets. */
-if (size < 10) {
-qemu_log_mask(LOG_GUEST_ERROR, "%s: dropped frame of %zd bytes\n",
- __func__, size);
-return size;
-}
-
if (!ftgmac100_filter(s, buf, size)) {
return size;
}
--
2.34.1
[PATCH v7 4/9] hw/net: ne2000: Remove the logic of padding short frames in the receive path
Now that we have implemented unified short frames padding in the
QEMU networking codes, remove the same logic in the NIC codes.
Signed-off-by: Bin Meng
---
(no changes since v1)
hw/net/ne2000.c | 12
1 file changed, 12 deletions(-)
diff --git a/hw/net/ne2000.c b/hw/net/ne2000.c
index 3f31d04efb..d79c884d50 100644
--- a/hw/net/ne2000.c
+++ b/hw/net/ne2000.c
@@ -167,15 +167,12 @@ static int ne2000_buffer_full(NE2000State *s)
return 0;
}
-#define MIN_BUF_SIZE 60
-
ssize_t ne2000_receive(NetClientState *nc, const uint8_t *buf, size_t size_)
{
NE2000State *s = qemu_get_nic_opaque(nc);
size_t size = size_;
uint8_t *p;
unsigned int total_len, next, avail, len, index, mcast_idx;
-uint8_t buf1[60];
static const uint8_t broadcast_macaddr[6] =
{ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
@@ -213,15 +210,6 @@ ssize_t ne2000_receive(NetClientState *nc, const uint8_t
*buf, size_t size_)
}
}
-
-/* if too small buffer, then expand it */
-if (size < MIN_BUF_SIZE) {
-memcpy(buf1, buf, size);
-memset(buf1 + size, 0, MIN_BUF_SIZE - size);
-buf = buf1;
-size = MIN_BUF_SIZE;
-}
-
index = s->curpag << 8;
if (index >= NE2000_PMEM_END) {
index = s->start;
--
2.34.1
[PATCH v7 5/9] hw/net: pcnet: Remove the logic of padding short frames in the receive path
Now that we have implemented unified short frames padding in the
QEMU networking codes, remove the same logic in the NIC codes.
Signed-off-by: Bin Meng
---
(no changes since v1)
hw/net/pcnet.c | 9 -
1 file changed, 9 deletions(-)
diff --git a/hw/net/pcnet.c b/hw/net/pcnet.c
index d456094575..02828ae716 100644
--- a/hw/net/pcnet.c
+++ b/hw/net/pcnet.c
@@ -987,7 +987,6 @@ ssize_t pcnet_receive(NetClientState *nc, const uint8_t
*buf, size_t size_)
{
PCNetState *s = qemu_get_nic_opaque(nc);
int is_padr = 0, is_bcast = 0, is_ladr = 0;
-uint8_t buf1[60];
int remaining;
int crc_err = 0;
size_t size = size_;
@@ -1000,14 +999,6 @@ ssize_t pcnet_receive(NetClientState *nc, const uint8_t
*buf, size_t size_)
printf("pcnet_receive size=%zu\n", size);
#endif
-/* if too small buffer, then expand it */
-if (size < MIN_BUF_SIZE) {
-memcpy(buf1, buf, size);
-memset(buf1 + size, 0, MIN_BUF_SIZE - size);
-buf = buf1;
-size = MIN_BUF_SIZE;
-}
-
if (CSR_PROM(s)
|| (is_padr=padr_match(s, buf, size))
|| (is_bcast=padr_bcast(s, buf, size))
--
2.34.1
[PATCH v7 0/9] net: Pad short frames for network backends
The minimum Ethernet frame length is 60 bytes. For short frames with smaller length like ARP packets (only 42 bytes), on a real world NIC it can choose either padding its length to the minimum required 60 bytes, or sending it out directly to the wire. Such behavior can be hardcoded or controled by a register bit. Similarly on the receive path, NICs can choose either dropping such short frames directly or handing them over to software to handle. On the other hand, for the network backends like SLiRP/TAP, they don't expose a way to control the short frame behavior. As of today they just send/receive data from/to the other end connected to them, which means any sized packet is acceptable. So they can send and receive short frames without any problem. It is observed that ARP packets sent from SLiRP/TAP are 42 bytes, and SLiRP/TAP just send these ARP packets to the other end which might be a NIC model that does not allow short frames to pass through. To provide better compatibility, for packets sent from QEMU network backends like SLiRP/TAP, we change to pad short frames before sending it out to the other end, if the other end does not forbid it via the nc->do_not_pad flag. This ensures a backend as an Ethernet sender does not violate the spec. But with this change, the behavior of dropping short frames from SLiRP/TAP interfaces in the NIC model cannot be emulated because it always receives a packet that is spec complaint. The capability of sending short frames from NIC models is still supported and short frames can still pass through SLiRP/TAP. This series should be able to fix the issue as reported with some NIC models before, that ARP requests get dropped, preventing the guest from becoming visible on the network. It was workarounded in these NIC models on the receive path, that when a short frame is received, it is padded up to 60 bytes. Only the first 4 patches of the v5 series [1] were applied in QEMU 6.0, and the reset was said to be queued for 6.1 but for some reason they never landed in QEMU mainline. Hopefully this series will make it for QEMU 8.1. [1] https://lore.kernel.org/qemu-devel/859cd26a-feb2-ed62-98d5-764841a46...@redhat.com/ Changes in v7: - new patch: "hw/net: ftgmac100: Drop the small packet check in the receive path" Bin Meng (9): hw/net: e1000: Remove the logic of padding short frames in the receive path hw/net: vmxnet3: Remove the logic of padding short frames in the receive path hw/net: i82596: Remove the logic of padding short frames in the receive path hw/net: ne2000: Remove the logic of padding short frames in the receive path hw/net: pcnet: Remove the logic of padding short frames in the receive path hw/net: rtl8139: Remove the logic of padding short frames in the receive path hw/net: sungem: Remove the logic of padding short frames in the receive path hw/net: sunhme: Remove the logic of padding short frames in the receive path hw/net: ftgmac100: Drop the small packet check in the receive path hw/net/e1000.c | 11 +-- hw/net/ftgmac100.c | 8 hw/net/i82596.c| 18 -- hw/net/ne2000.c| 12 hw/net/pcnet.c | 9 - hw/net/rtl8139.c | 12 hw/net/sungem.c| 14 -- hw/net/sunhme.c| 11 --- hw/net/vmxnet3.c | 10 -- 9 files changed, 1 insertion(+), 104 deletions(-) -- 2.34.1
[PATCH v7 3/9] hw/net: i82596: Remove the logic of padding short frames in the receive path
Now that we have implemented unified short frames padding in the
QEMU networking codes, remove the same logic in the NIC codes.
Signed-off-by: Bin Meng
---
(no changes since v1)
hw/net/i82596.c | 18 --
1 file changed, 18 deletions(-)
diff --git a/hw/net/i82596.c b/hw/net/i82596.c
index ec21e2699a..ab26f8bea1 100644
--- a/hw/net/i82596.c
+++ b/hw/net/i82596.c
@@ -72,10 +72,6 @@ enum commands {
#define I596_EOF0x8000
#define SIZE_MASK 0x3fff
-#define ETHER_TYPE_LEN 2
-#define VLAN_TCI_LEN 2
-#define VLAN_HLEN (ETHER_TYPE_LEN + VLAN_TCI_LEN)
-
/* various flags in the chip config registers */
#define I596_PREFETCH (s->config[0] & 0x80)
#define I596_PROMISC(s->config[8] & 0x01)
@@ -488,8 +484,6 @@ bool i82596_can_receive(NetClientState *nc)
return true;
}
-#define MIN_BUF_SIZE 60
-
ssize_t i82596_receive(NetClientState *nc, const uint8_t *buf, size_t sz)
{
I82596State *s = qemu_get_nic_opaque(nc);
@@ -500,7 +494,6 @@ ssize_t i82596_receive(NetClientState *nc, const uint8_t
*buf, size_t sz)
size_t bufsz = sz; /* length of data in buf */
uint32_t crc;
uint8_t *crc_ptr;
-uint8_t buf1[MIN_BUF_SIZE + VLAN_HLEN];
static const uint8_t broadcast_macaddr[6] = {
0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
@@ -583,17 +576,6 @@ ssize_t i82596_receive(NetClientState *nc, const uint8_t
*buf, size_t sz)
}
}
-/* if too small buffer, then expand it */
-if (len < MIN_BUF_SIZE + VLAN_HLEN) {
-memcpy(buf1, buf, len);
-memset(buf1 + len, 0, MIN_BUF_SIZE + VLAN_HLEN - len);
-buf = buf1;
-if (len < MIN_BUF_SIZE) {
-len = MIN_BUF_SIZE;
-}
-bufsz = len;
-}
-
/* Calculate the ethernet checksum (4 bytes) */
len += 4;
crc = cpu_to_be32(crc32(~0, buf, sz));
--
2.34.1
[PATCH v7 7/9] hw/net: sungem: Remove the logic of padding short frames in the receive path
Now that we have implemented unified short frames padding in the
QEMU networking codes, remove the same logic in the NIC codes.
Signed-off-by: Bin Meng
---
(no changes since v1)
hw/net/sungem.c | 14 --
1 file changed, 14 deletions(-)
diff --git a/hw/net/sungem.c b/hw/net/sungem.c
index eb01520790..103376c133 100644
--- a/hw/net/sungem.c
+++ b/hw/net/sungem.c
@@ -550,7 +550,6 @@ static ssize_t sungem_receive(NetClientState *nc, const
uint8_t *buf,
PCIDevice *d = PCI_DEVICE(s);
uint32_t mac_crc, done, kick, max_fsize;
uint32_t fcs_size, ints, rxdma_cfg, rxmac_cfg, csum, coff;
-uint8_t smallbuf[60];
struct gem_rxd desc;
uint64_t dbase, baddr;
unsigned int rx_cond;
@@ -584,19 +583,6 @@ static ssize_t sungem_receive(NetClientState *nc, const
uint8_t *buf,
return size;
}
-/* We don't drop too small frames since we get them in qemu, we pad
- * them instead. We should probably use the min frame size register
- * but I don't want to use a variable size staging buffer and I
- * know both MacOS and Linux use the default 64 anyway. We use 60
- * here to account for the non-existent FCS.
- */
-if (size < 60) {
-memcpy(smallbuf, buf, size);
-memset(&smallbuf[size], 0, 60 - size);
-buf = smallbuf;
-size = 60;
-}
-
/* Get MAC crc */
mac_crc = net_crc32_le(buf, ETH_ALEN);
--
2.34.1
[PATCH v3 5/6] util/async-teardown: Use qemu_close_range() to close fds
From: Zhangjin Wu
Based on the old close_all_open_fd() of util/async-teardown.c, a new
generic qemu_close_range() has been added in osdep.c.
Now, let's switch over to use the generic qemu_close_range().
Signed-off-by: Zhangjin Wu
Signed-off-by: Bin Meng
---
Changes in v3:
- limit the last_fd of qemu_close_range() to sysconf(_SC_OPEN_MAX)
Changes in v2:
- new patch: "util/async-teardown: Use qemu_close_range() to close fds"
util/async-teardown.c | 42 ++
1 file changed, 2 insertions(+), 40 deletions(-)
diff --git a/util/async-teardown.c b/util/async-teardown.c
index 7e0177a8da..e102912f3f 100644
--- a/util/async-teardown.c
+++ b/util/async-teardown.c
@@ -29,44 +29,6 @@
static pid_t the_ppid;
-/*
- * Close all open file descriptors.
- */
-static void close_all_open_fd(void)
-{
-struct dirent *de;
-int fd, dfd;
-DIR *dir;
-
-#ifdef CONFIG_CLOSE_RANGE
-int r = close_range(0, ~0U, 0);
-if (!r) {
-/* Success, no need to try other ways. */
-return;
-}
-#endif
-
-dir = opendir("/proc/self/fd");
-if (!dir) {
-/* If /proc is not mounted, close fds one by one. */
-int open_max = sysconf(_SC_OPEN_MAX), i;
-for (i = 0; i < open_max; i++) {
-close(i);
-}
-return;
-}
-/* Avoid closing the directory. */
-dfd = dirfd(dir);
-
-for (de = readdir(dir); de; de = readdir(dir)) {
-fd = atoi(de->d_name);
-if (fd != dfd) {
-close(fd);
-}
-}
-closedir(dir);
-}
-
static void hup_handler(int signal)
{
/* Check every second if this process has been reparented. */
@@ -84,6 +46,7 @@ static int async_teardown_fn(void *arg)
struct sigaction sa = { .sa_handler = hup_handler };
sigset_t hup_signal;
char name[16];
+int open_max = sysconf(_SC_OPEN_MAX);
/* Set a meaningful name for this process. */
snprintf(name, 16, "cleanup/%d", the_ppid);
@@ -92,9 +55,8 @@ static int async_teardown_fn(void *arg)
/*
* Close all file descriptors that might have been inherited from the
* main qemu process when doing clone, needed to make libvirt happy.
- * Not using close_range for increased compatibility with older kernels.
*/
-close_all_open_fd();
+qemu_close_range(0, open_max - 1);
/* Set up a handler for SIGHUP and unblock SIGHUP. */
sigaction(SIGHUP, &sa, NULL);
--
2.34.1
[PATCH v3 0/6] net/tap: Fix QEMU frozen issue when the maximum number of file descriptors is very large
Current codes using a brute-force traversal of all file descriptors do not scale on a system where the maximum number of file descriptors is set to a very large value (e.g.: in a Docker container of Manjaro distribution it is set to 1073741816). QEMU just looks frozen during start-up. The close-on-exec flag (O_CLOEXEC) was introduced since Linux kernel 2.6.23, FreeBSD 8.3, OpenBSD 5.0, Solaris 11. While it's true QEMU doesn't need to manually close the fds for child process as the proper O_CLOEXEC flag should have been set properly on files with its own codes, QEMU uses a huge number of 3rd party libraries and we don't trust them to reliably be using O_CLOEXEC on everything they open. Modern Linux and BSDs have the close_range() call we can use to do the job, and on Linux we have one more way to walk through /proc/self/fd to complete the task efficiently, which is what qemu_close_range() does, a new API we add in util/osdep.c. V1 link: https://lore.kernel.org/qemu-devel/20230406112041.798585-1-bm...@tinylab.org/ Changes in v3: - fix win32 build failure - limit the last_fd of qemu_close_range() to sysconf(_SC_OPEN_MAX) Changes in v2: - new patch: "tests/tcg/cris: Fix the coding style" - new patch: "tests/tcg/cris: Correct the off-by-one error" - new patch: "util/async-teardown: Fall back to close fds one by one" - new patch: "util/osdep: Introduce qemu_close_range()" - new patch: "util/async-teardown: Use qemu_close_range() to close fds" - Change to use qemu_close_range() to close fds for child process efficiently - v1 link: https://lore.kernel.org/qemu-devel/20230406112041.798585-1-bm...@tinylab.org/ Bin Meng (4): tests/tcg/cris: Fix the coding style tests/tcg/cris: Correct the off-by-one error util/async-teardown: Fall back to close fds one by one util/osdep: Introduce qemu_close_range() Zhangjin Wu (2): util/async-teardown: Use qemu_close_range() to close fds net: tap: Use qemu_close_range() to close fds include/qemu/osdep.h| 1 + net/tap.c | 23 ++-- tests/tcg/cris/libc/check_openpf5.c | 57 ++--- util/async-teardown.c | 38 +-- util/osdep.c| 48 5 files changed, 89 insertions(+), 78 deletions(-) -- 2.34.1
[PATCH v3 4/6] util/osdep: Introduce qemu_close_range()
This introduces a new QEMU API qemu_close_range() that closes all
open file descriptors from first to last (included).
This API will try a more efficient call to close_range(), or walk
through of /proc/self/fd whenever these are possible, otherwise it
falls back to a plain close loop.
Co-developed-by: Zhangjin Wu
Signed-off-by: Bin Meng
---
Changes in v3:
- fix win32 build failure
Changes in v2:
- new patch: "util/osdep: Introduce qemu_close_range()"
include/qemu/osdep.h | 1 +
util/osdep.c | 48
2 files changed, 49 insertions(+)
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index cc61b00ba9..e22434ce10 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -560,6 +560,7 @@ int qemu_open_old(const char *name, int flags, ...);
int qemu_open(const char *name, int flags, Error **errp);
int qemu_create(const char *name, int flags, mode_t mode, Error **errp);
int qemu_close(int fd);
+int qemu_close_range(unsigned int first, unsigned int last);
int qemu_unlink(const char *name);
#ifndef _WIN32
int qemu_dup_flags(int fd, int flags);
diff --git a/util/osdep.c b/util/osdep.c
index e996c4744a..91275e70f8 100644
--- a/util/osdep.c
+++ b/util/osdep.c
@@ -30,6 +30,7 @@
#include "qemu/mprotect.h"
#include "qemu/hw-version.h"
#include "monitor/monitor.h"
+#include
static const char *hw_version = QEMU_HW_VERSION;
@@ -411,6 +412,53 @@ int qemu_close(int fd)
return close(fd);
}
+int qemu_close_range(unsigned int first, unsigned int last)
+{
+DIR *dir = NULL;
+
+#ifdef CONFIG_CLOSE_RANGE
+int r = close_range(first, last, 0);
+if (!r) {
+/* Success, no need to try other ways. */
+return 0;
+}
+#endif
+
+#ifdef __linux__
+dir = opendir("/proc/self/fd");
+#endif
+if (!dir) {
+/*
+ * If /proc is not mounted or /proc/self/fd is not supported,
+ * try close() from first to last.
+ */
+for (int i = first; i <= last; i++) {
+close(i);
+}
+
+return 0;
+}
+
+#ifndef _WIN32
+/* Avoid closing the directory */
+int dfd = dirfd(dir);
+
+for (struct dirent *de = readdir(dir); de; de = readdir(dir)) {
+int fd = atoi(de->d_name);
+if (fd < first || fd > last) {
+/* Exclude the fds outside the target range */
+continue;
+}
+if (fd != dfd) {
+close(fd);
+}
+}
+closedir(dir);
+#endif /* _WIN32 */
+
+return 0;
+}
+
/*
* Delete a file from the filesystem, unless the filename is /dev/fdset/...
*
--
2.34.1
[PATCH v3 6/6] net: tap: Use qemu_close_range() to close fds
From: Zhangjin Wu
Current codes using a brute-force traversal of all file descriptors
do not scale on a system where the maximum number of file descriptors
is set to a very large value (e.g.: in a Docker container of Manjaro
distribution it is set to 1073741816). QEMU just looks frozen during
start-up.
The close-on-exec flag (O_CLOEXEC) was introduced since Linux kernel
2.6.23, FreeBSD 8.3, OpenBSD 5.0, Solaris 11. While it's true QEMU
doesn't need to manually close the fds for child process as the proper
O_CLOEXEC flag should have been set properly on files with its own
codes, QEMU uses a huge number of 3rd party libraries and we don't
trust them to reliably be using O_CLOEXEC on everything they open.
Modern Linux and BSDs have the close_range() call we can use to do the
job, and on Linux we have one more way to walk through /proc/self/fd
to complete the task efficiently, which is what qemu_close_range() does.
Reported-by: Zhangjin Wu
Co-developed-by: Bin Meng
Signed-off-by: Zhangjin Wu
Signed-off-by: Bin Meng
---
(no changes since v2)
Changes in v2:
- Change to use qemu_close_range() to close fds for child process efficiently
- v1 link:
https://lore.kernel.org/qemu-devel/20230406112041.798585-1-bm...@tinylab.org/
net/tap.c | 23 +++
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/net/tap.c b/net/tap.c
index 1bf085d422..d482fabdff 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -446,13 +446,13 @@ static void launch_script(const char *setup_script, const
char *ifname,
return;
}
if (pid == 0) {
-int open_max = sysconf(_SC_OPEN_MAX), i;
+unsigned int last_fd = sysconf(_SC_OPEN_MAX) - 1;
+
+/* skip stdin, stdout and stderr */
+qemu_close_range(3, fd - 1);
+/* skip the currently used fd */
+qemu_close_range(fd + 1, last_fd);
-for (i = 3; i < open_max; i++) {
-if (i != fd) {
-close(i);
-}
-}
parg = args;
*parg++ = (char *)setup_script;
*parg++ = (char *)ifname;
@@ -536,16 +536,15 @@ static int net_bridge_run_helper(const char *helper,
const char *bridge,
return -1;
}
if (pid == 0) {
-int open_max = sysconf(_SC_OPEN_MAX), i;
+unsigned int last_fd = sysconf(_SC_OPEN_MAX) - 1, fd = sv[1];
char *fd_buf = NULL;
char *br_buf = NULL;
char *helper_cmd = NULL;
-for (i = 3; i < open_max; i++) {
-if (i != sv[1]) {
-close(i);
-}
-}
+/* skip stdin, stdout and stderr */
+qemu_close_range(3, fd - 1);
+/* skip the currently used fd */
+qemu_close_range(fd + 1, last_fd);
fd_buf = g_strdup_printf("%s%d", "--fd=", sv[1]);
--
2.34.1
[PATCH v3 3/6] util/async-teardown: Fall back to close fds one by one
When opening /proc/self/fd fails, current codes just return directly,
but we can fall back to close fds one by one.
Signed-off-by: Bin Meng
---
(no changes since v2)
Changes in v2:
- new patch: "util/async-teardown: Fall back to close fds one by one"
util/async-teardown.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/util/async-teardown.c b/util/async-teardown.c
index 3ab19c8740..7e0177a8da 100644
--- a/util/async-teardown.c
+++ b/util/async-teardown.c
@@ -48,7 +48,11 @@ static void close_all_open_fd(void)
dir = opendir("/proc/self/fd");
if (!dir) {
-/* If /proc is not mounted, there is nothing that can be done. */
+/* If /proc is not mounted, close fds one by one. */
+int open_max = sysconf(_SC_OPEN_MAX), i;
+for (i = 0; i < open_max; i++) {
+close(i);
+}
return;
}
/* Avoid closing the directory. */
--
2.34.1
[PATCH v3 2/6] tests/tcg/cris: Correct the off-by-one error
sysconf(_SC_OPEN_MAX) returns the maximum number of files that
a process can have open at any time, which means the fd should
not be larger than or equal to the return value.
Signed-off-by: Bin Meng
---
(no changes since v2)
Changes in v2:
- new patch: "tests/tcg/cris: Correct the off-by-one error"
tests/tcg/cris/libc/check_openpf5.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/tcg/cris/libc/check_openpf5.c
b/tests/tcg/cris/libc/check_openpf5.c
index 0037fbca4c..7f585c6d37 100644
--- a/tests/tcg/cris/libc/check_openpf5.c
+++ b/tests/tcg/cris/libc/check_openpf5.c
@@ -31,10 +31,10 @@ int main(int argc, char *argv[])
strcpy(fn, "/");
strcat(fn, argv[0]);
-for (i = 0; i < filemax + 1; i++) {
+for (i = 0; i < filemax; i++) {
if (open(fn, O_RDONLY) < 0) {
/* Shouldn't happen too early. */
-if (i < filemax - 3 - 1) {
+if (i < filemax - 3) {
fprintf(stderr, "i: %d\n", i);
abort();
}
--
2.34.1
[PATCH v3 1/6] tests/tcg/cris: Fix the coding style
The code style does not conform with QEMU's. Correct it so that the
upcoming commit does not trigger checkpatch warnings.
Signed-off-by: Bin Meng
---
(no changes since v2)
Changes in v2:
- new patch: "tests/tcg/cris: Fix the coding style"
tests/tcg/cris/libc/check_openpf5.c | 57 ++---
1 file changed, 27 insertions(+), 30 deletions(-)
diff --git a/tests/tcg/cris/libc/check_openpf5.c
b/tests/tcg/cris/libc/check_openpf5.c
index 1f86ea283d..0037fbca4c 100644
--- a/tests/tcg/cris/libc/check_openpf5.c
+++ b/tests/tcg/cris/libc/check_openpf5.c
@@ -13,44 +13,41 @@
#include
#include
-int main (int argc, char *argv[])
+int main(int argc, char *argv[])
{
- int i;
- int filemax;
+int i;
+int filemax;
#ifdef OPEN_MAX
- filemax = OPEN_MAX;
+filemax = OPEN_MAX;
#else
- filemax = sysconf (_SC_OPEN_MAX);
+filemax = sysconf(_SC_OPEN_MAX);
#endif
- char *fn = malloc (strlen (argv[0]) + 2);
- if (fn == NULL)
-abort ();
- strcpy (fn, "/");
- strcat (fn, argv[0]);
+char *fn = malloc(strlen(argv[0]) + 2);
+if (fn == NULL) {
+abort();
+}
+strcpy(fn, "/");
+strcat(fn, argv[0]);
- for (i = 0; i < filemax + 1; i++)
-{
- if (open (fn, O_RDONLY) < 0)
- {
- /* Shouldn't happen too early. */
- if (i < filemax - 3 - 1)
- {
- fprintf (stderr, "i: %d\n", i);
- abort ();
- }
- if (errno != EMFILE)
- {
- perror ("open");
- abort ();
- }
- goto ok;
- }
+for (i = 0; i < filemax + 1; i++) {
+if (open(fn, O_RDONLY) < 0) {
+/* Shouldn't happen too early. */
+if (i < filemax - 3 - 1) {
+fprintf(stderr, "i: %d\n", i);
+abort();
+}
+if (errno != EMFILE) {
+perror("open");
+abort();
+}
+goto ok;
+}
}
- abort ();
+abort();
ok:
- printf ("pass\n");
- exit (0);
+printf("pass\n");
+exit(0);
}
--
2.34.1
[PATCH v2 5/6] util/async-teardown: Use qemu_close_range() to close fds
From: Zhangjin Wu
Based on the old close_all_open_fd() of util/async-teardown.c, a new
generic qemu_close_range() has been added in osdep.c.
Now, let's switch over to use the generic qemu_close_range().
Signed-off-by: Zhangjin Wu
Signed-off-by: Bin Meng
---
(no changes since v1)
util/async-teardown.c | 41 +
1 file changed, 1 insertion(+), 40 deletions(-)
diff --git a/util/async-teardown.c b/util/async-teardown.c
index 7e0177a8da..200ec05101 100644
--- a/util/async-teardown.c
+++ b/util/async-teardown.c
@@ -29,44 +29,6 @@
static pid_t the_ppid;
-/*
- * Close all open file descriptors.
- */
-static void close_all_open_fd(void)
-{
-struct dirent *de;
-int fd, dfd;
-DIR *dir;
-
-#ifdef CONFIG_CLOSE_RANGE
-int r = close_range(0, ~0U, 0);
-if (!r) {
-/* Success, no need to try other ways. */
-return;
-}
-#endif
-
-dir = opendir("/proc/self/fd");
-if (!dir) {
-/* If /proc is not mounted, close fds one by one. */
-int open_max = sysconf(_SC_OPEN_MAX), i;
-for (i = 0; i < open_max; i++) {
-close(i);
-}
-return;
-}
-/* Avoid closing the directory. */
-dfd = dirfd(dir);
-
-for (de = readdir(dir); de; de = readdir(dir)) {
-fd = atoi(de->d_name);
-if (fd != dfd) {
-close(fd);
-}
-}
-closedir(dir);
-}
-
static void hup_handler(int signal)
{
/* Check every second if this process has been reparented. */
@@ -92,9 +54,8 @@ static int async_teardown_fn(void *arg)
/*
* Close all file descriptors that might have been inherited from the
* main qemu process when doing clone, needed to make libvirt happy.
- * Not using close_range for increased compatibility with older kernels.
*/
-close_all_open_fd();
+qemu_close_range(0, ~0U);
/* Set up a handler for SIGHUP and unblock SIGHUP. */
sigaction(SIGHUP, &sa, NULL);
--
2.34.1
[PATCH v2 6/6] net: tap: Use qemu_close_range() to close fds
From: Zhangjin Wu
Current codes using a brute-force traversal of all file descriptors
do not scale on a system where the maximum number of file descriptors
is set to a very large value (e.g.: in a Docker container of Manjaro
distribution it is set to 1073741816). QEMU just looks frozen during
start-up.
The close-on-exec flag (O_CLOEXEC) was introduced since Linux kernel
2.6.23, FreeBSD 8.3, OpenBSD 5.0, Solaris 11. While it's true QEMU
doesn't need to manually close the fds for child process as the proper
O_CLOEXEC flag should have been set properly on files with its own
codes, QEMU uses a huge number of 3rd party libraries and we don't
trust them to reliably be using O_CLOEXEC on everything they open.
Modern Linux and BSDs have the close_range() call we can use to do the
job, and on Linux we have one more way to walk through /proc/self/fd
to complete the task efficiently, which is what qemu_close_range() does.
Reported-by: Zhangjin Wu
Signed-off-by: Zhangjin Wu
Signed-off-by: Bin Meng
---
Changes in v2:
- Change to use qemu_close_range() to close fds for child process efficiently
- v1 link:
https://lore.kernel.org/qemu-devel/20230406112041.798585-1-bm...@tinylab.org/
net/tap.c | 23 +++
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/net/tap.c b/net/tap.c
index 1bf085d422..d482fabdff 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -446,13 +446,13 @@ static void launch_script(const char *setup_script, const
char *ifname,
return;
}
if (pid == 0) {
-int open_max = sysconf(_SC_OPEN_MAX), i;
+unsigned int last_fd = sysconf(_SC_OPEN_MAX) - 1;
+
+/* skip stdin, stdout and stderr */
+qemu_close_range(3, fd - 1);
+/* skip the currently used fd */
+qemu_close_range(fd + 1, last_fd);
-for (i = 3; i < open_max; i++) {
-if (i != fd) {
-close(i);
-}
-}
parg = args;
*parg++ = (char *)setup_script;
*parg++ = (char *)ifname;
@@ -536,16 +536,15 @@ static int net_bridge_run_helper(const char *helper,
const char *bridge,
return -1;
}
if (pid == 0) {
-int open_max = sysconf(_SC_OPEN_MAX), i;
+unsigned int last_fd = sysconf(_SC_OPEN_MAX) - 1, fd = sv[1];
char *fd_buf = NULL;
char *br_buf = NULL;
char *helper_cmd = NULL;
-for (i = 3; i < open_max; i++) {
-if (i != sv[1]) {
-close(i);
-}
-}
+/* skip stdin, stdout and stderr */
+qemu_close_range(3, fd - 1);
+/* skip the currently used fd */
+qemu_close_range(fd + 1, last_fd);
fd_buf = g_strdup_printf("%s%d", "--fd=", sv[1]);
--
2.34.1
[PATCH v2 0/6] net/tap: Fix QEMU frozen issue when the maximum number of file descriptors is very large
Current codes using a brute-force traversal of all file descriptors do not scale on a system where the maximum number of file descriptors is set to a very large value (e.g.: in a Docker container of Manjaro distribution it is set to 1073741816). QEMU just looks frozen during start-up. The close-on-exec flag (O_CLOEXEC) was introduced since Linux kernel 2.6.23, FreeBSD 8.3, OpenBSD 5.0, Solaris 11. While it's true QEMU doesn't need to manually close the fds for child process as the proper O_CLOEXEC flag should have been set properly on files with its own codes, QEMU uses a huge number of 3rd party libraries and we don't trust them to reliably be using O_CLOEXEC on everything they open. Modern Linux and BSDs have the close_range() call we can use to do the job, and on Linux we have one more way to walk through /proc/self/fd to complete the task efficiently, which is what qemu_close_range() does, a new API we add in utils/osdep.c. Changes in v2: - Change to use qemu_close_range() to close fds for child process efficiently - v1 link: https://lore.kernel.org/qemu-devel/20230406112041.798585-1-bm...@tinylab.org/ Bin Meng (4): tests/tcg/cris: Fix the coding style tests/tcg/cris: Correct the off-by-one error util/async-teardown: Fall back to close fds one by one utils/osdep: Introduce qemu_close_range() Zhangjin Wu (2): util/async-teardown: Use qemu_close_range() to close fds net: tap: Use qemu_close_range() to close fds include/qemu/osdep.h| 1 + net/tap.c | 23 ++-- tests/tcg/cris/libc/check_openpf5.c | 57 ++--- util/async-teardown.c | 37 +-- util/osdep.c| 47 5 files changed, 87 insertions(+), 78 deletions(-) -- 2.34.1
[PATCH v2 1/6] tests/tcg/cris: Fix the coding style
The code style does not conform with QEMU's. Correct it so that the
upcoming commit does not trigger checkpatch warnings.
Signed-off-by: Bin Meng
---
(no changes since v1)
tests/tcg/cris/libc/check_openpf5.c | 57 ++---
1 file changed, 27 insertions(+), 30 deletions(-)
diff --git a/tests/tcg/cris/libc/check_openpf5.c
b/tests/tcg/cris/libc/check_openpf5.c
index 1f86ea283d..0037fbca4c 100644
--- a/tests/tcg/cris/libc/check_openpf5.c
+++ b/tests/tcg/cris/libc/check_openpf5.c
@@ -13,44 +13,41 @@
#include
#include
-int main (int argc, char *argv[])
+int main(int argc, char *argv[])
{
- int i;
- int filemax;
+int i;
+int filemax;
#ifdef OPEN_MAX
- filemax = OPEN_MAX;
+filemax = OPEN_MAX;
#else
- filemax = sysconf (_SC_OPEN_MAX);
+filemax = sysconf(_SC_OPEN_MAX);
#endif
- char *fn = malloc (strlen (argv[0]) + 2);
- if (fn == NULL)
-abort ();
- strcpy (fn, "/");
- strcat (fn, argv[0]);
+char *fn = malloc(strlen(argv[0]) + 2);
+if (fn == NULL) {
+abort();
+}
+strcpy(fn, "/");
+strcat(fn, argv[0]);
- for (i = 0; i < filemax + 1; i++)
-{
- if (open (fn, O_RDONLY) < 0)
- {
- /* Shouldn't happen too early. */
- if (i < filemax - 3 - 1)
- {
- fprintf (stderr, "i: %d\n", i);
- abort ();
- }
- if (errno != EMFILE)
- {
- perror ("open");
- abort ();
- }
- goto ok;
- }
+for (i = 0; i < filemax + 1; i++) {
+if (open(fn, O_RDONLY) < 0) {
+/* Shouldn't happen too early. */
+if (i < filemax - 3 - 1) {
+fprintf(stderr, "i: %d\n", i);
+abort();
+}
+if (errno != EMFILE) {
+perror("open");
+abort();
+}
+goto ok;
+}
}
- abort ();
+abort();
ok:
- printf ("pass\n");
- exit (0);
+printf("pass\n");
+exit(0);
}
--
2.34.1
[PATCH v2 3/6] util/async-teardown: Fall back to close fds one by one
When opening /proc/self/fd fails, current codes just return directly,
but we can fall back to close fds one by one.
Signed-off-by: Bin Meng
---
(no changes since v1)
util/async-teardown.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/util/async-teardown.c b/util/async-teardown.c
index 3ab19c8740..7e0177a8da 100644
--- a/util/async-teardown.c
+++ b/util/async-teardown.c
@@ -48,7 +48,11 @@ static void close_all_open_fd(void)
dir = opendir("/proc/self/fd");
if (!dir) {
-/* If /proc is not mounted, there is nothing that can be done. */
+/* If /proc is not mounted, close fds one by one. */
+int open_max = sysconf(_SC_OPEN_MAX), i;
+for (i = 0; i < open_max; i++) {
+close(i);
+}
return;
}
/* Avoid closing the directory. */
--
2.34.1
[PATCH v2 2/6] tests/tcg/cris: Correct the off-by-one error
sysconf(_SC_OPEN_MAX) returns the maximum number of files that
a process can have open at any time, which means the fd should
not be larger than or equal to the return value.
Signed-off-by: Bin Meng
---
(no changes since v1)
tests/tcg/cris/libc/check_openpf5.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/tcg/cris/libc/check_openpf5.c
b/tests/tcg/cris/libc/check_openpf5.c
index 0037fbca4c..7f585c6d37 100644
--- a/tests/tcg/cris/libc/check_openpf5.c
+++ b/tests/tcg/cris/libc/check_openpf5.c
@@ -31,10 +31,10 @@ int main(int argc, char *argv[])
strcpy(fn, "/");
strcat(fn, argv[0]);
-for (i = 0; i < filemax + 1; i++) {
+for (i = 0; i < filemax; i++) {
if (open(fn, O_RDONLY) < 0) {
/* Shouldn't happen too early. */
-if (i < filemax - 3 - 1) {
+if (i < filemax - 3) {
fprintf(stderr, "i: %d\n", i);
abort();
}
--
2.34.1
[PATCH v2 4/6] utils/osdep: Introduce qemu_close_range()
This introduces a new QEMU API qemu_close_range() that closes all
open file descriptors from first to last (included).
This API will try a more efficient call to close_range(), or walk
through of /proc/self/fd whenever these are possible, otherwise it
falls back to a plain close loop.
Co-developed-by: Zhangjin Wu
Signed-off-by: Bin Meng
---
(no changes since v1)
include/qemu/osdep.h | 1 +
util/osdep.c | 47
2 files changed, 48 insertions(+)
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index cc61b00ba9..e22434ce10 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -560,6 +560,7 @@ int qemu_open_old(const char *name, int flags, ...);
int qemu_open(const char *name, int flags, Error **errp);
int qemu_create(const char *name, int flags, mode_t mode, Error **errp);
int qemu_close(int fd);
+int qemu_close_range(unsigned int first, unsigned int last);
int qemu_unlink(const char *name);
#ifndef _WIN32
int qemu_dup_flags(int fd, int flags);
diff --git a/util/osdep.c b/util/osdep.c
index e996c4744a..fd7dd2dbdf 100644
--- a/util/osdep.c
+++ b/util/osdep.c
@@ -411,6 +411,53 @@ int qemu_close(int fd)
return close(fd);
}
+int qemu_close_range(unsigned int first, unsigned int last)
+{
+DIR *dir = NULL;
+
+#ifdef CONFIG_CLOSE_RANGE
+int r = close_range(first, last, 0);
+if (!r) {
+/* Success, no need to try other ways. */
+return 0;
+}
+#endif
+
+#ifdef __linux__
+dir = opendir("/proc/self/fd");
+#endif
+if (!dir) {
+/*
+ * If /proc is not mounted or /proc/self/fd is not supported,
+ * try close() from first to last.
+ */
+for (int i = first; i <= last; i++) {
+close(i);
+}
+
+return 0;
+}
+
+#ifndef _WIN32
+/* Avoid closing the directory */
+int dfd = dirfd(dir);
+
+for (struct dirent *de = readdir(dir); de; de = readdir(dir)) {
+int fd = atoi(de->d_name);
+if (fd < first || fd > last) {
+/* Exclude the fds outside the target range */
+continue;
+}
+if (fd != dfd) {
+close(fd);
+}
+}
+closedir(dir);
+#endif /* _WIN32 */
+
+return 0;
+}
+
/*
* Delete a file from the filesystem, unless the filename is /dev/fdset/...
*
--
2.34.1
Re: [PATCH] ui/sdl2: fix surface_gl_update_texture: Assertion 'gls' failed
On Thu, May 11, 2023 at 3:43 PM wrote:
>
> From: Marc-André Lureau
>
> Before sdl2_gl_update() is called, sdl2_gl_switch() may decide to
> destroy the console window and its associated shaders.
>
> Fixes:
> https://gitlab.com/qemu-project/qemu/-/issues/1644
This should be:
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1644
>
> Fixes: commit c84ab0a5 ("ui/console: optionally update after gfx switch")
This should be:
Fixes: c84ab0a500a8 ("ui/console: optionally update after gfx switch")
>
> Signed-off-by: Marc-André Lureau
> ---
> ui/sdl2-gl.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/ui/sdl2-gl.c b/ui/sdl2-gl.c
> index b36ba6415a..cc83c98349 100644
> --- a/ui/sdl2-gl.c
> +++ b/ui/sdl2-gl.c
> @@ -67,6 +67,10 @@ void sdl2_gl_update(DisplayChangeListener *dcl,
>
> assert(scon->opengl);
>
> +if (!scon->real_window) {
> +return;
> +}
> +
> SDL_GL_MakeCurrent(scon->real_window, scon->winctx);
> surface_gl_update_texture(scon->gls, scon->surface, x, y, w, h);
> scon->updates++;
> --
Testing this patch with the "-device virtio-gpu-pci -display
sdl,gl=on" command line, the assertion mentioned in
https://gitlab.com/qemu-project/qemu/-/issues/1644 is fixed.
Tested-by: Bin Meng
Regards,
Bin
Re: [PATCH] hw/riscv: virt: Enable booting M-mode or S-mode FW from pflash0
On Fri, Apr 21, 2023 at 12:44 PM Sunil V L wrote: > > On Fri, Apr 21, 2023 at 12:39:46PM +0800, Bin Meng wrote: > > On Fri, Apr 21, 2023 at 12:34 PM Sunil V L wrote: > > > > > > Currently, virt machine supports two pflash instances each with > > > 32MB size. However, the first pflash is always assumed to > > > contain M-mode firmware and reset vector is set to this if > > > enabled. Hence, for S-mode payloads like EDK2, only one pflash > > > instance is available for use. This means both code and NV variables > > > of EDK2 will need to use the same pflash. > > > > > > The OS distros keep the EDK2 FW code as readonly. When non-volatile > > > variables also need to share the same pflash, it is not possible > > > to keep it as readonly since variables need write access. > > > > > > To resolve this issue, the code and NV variables need to be separated. > > > But in that case we need an extra flash. Hence, modify the convention > > > such that pflash0 will contain the M-mode FW only when "-bios none" > > > option is used. Otherwise, pflash0 will contain the S-mode payload FW. > > > This enables both pflash instances available for EDK2 use. > > > > > > Example usage: > > > 1) pflash0 containing M-mode FW > > > qemu-system-riscv64 -bios none -pflash -machine virt > > > or > > > qemu-system-riscv64 -bios none \ > > > -drive file=,if=pflash,format=raw,unit=0 -machine virt > > > > > > 2) pflash0 containing S-mode payload like EDK2 > > > qemu-system-riscv64 -pflash -pflash -machine > > > virt > > > or > > > qemu-system-riscv64 -bios \ > > > -pflash \ > > > -pflash \ > > > -machine virt > > > or > > > qemu-system-riscv64 -bios \ > > > -drive file=,if=pflash,format=raw,unit=0,readonly=on \ > > > -drive file=,if=pflash,format=raw,unit=1 \ > > > -machine virt > > > > Please update the docs in docs/system/riscv/virt.rst to include how to > > run EDK2 bios with these settings. > > > Thanks Bin. Shall I do it as a separate patch after this gets approved? > The reason is, I need to make changes in EDK2 to work with this. Once > EDK2 changes are also in place, will send a patch to update this > documentation. Does it make sense? > Yeah, in the doc we should provide the EDK2 URL and commit that was verified to work, so it makes sense we can add such info when EDK2 is available. Regards, Bin
Re: [PATCH] hw/riscv: virt: Enable booting M-mode or S-mode FW from pflash0
On Fri, Apr 21, 2023 at 12:34 PM Sunil V L wrote:
>
> Currently, virt machine supports two pflash instances each with
> 32MB size. However, the first pflash is always assumed to
> contain M-mode firmware and reset vector is set to this if
> enabled. Hence, for S-mode payloads like EDK2, only one pflash
> instance is available for use. This means both code and NV variables
> of EDK2 will need to use the same pflash.
>
> The OS distros keep the EDK2 FW code as readonly. When non-volatile
> variables also need to share the same pflash, it is not possible
> to keep it as readonly since variables need write access.
>
> To resolve this issue, the code and NV variables need to be separated.
> But in that case we need an extra flash. Hence, modify the convention
> such that pflash0 will contain the M-mode FW only when "-bios none"
> option is used. Otherwise, pflash0 will contain the S-mode payload FW.
> This enables both pflash instances available for EDK2 use.
>
> Example usage:
> 1) pflash0 containing M-mode FW
> qemu-system-riscv64 -bios none -pflash -machine virt
> or
> qemu-system-riscv64 -bios none \
> -drive file=,if=pflash,format=raw,unit=0 -machine virt
>
> 2) pflash0 containing S-mode payload like EDK2
> qemu-system-riscv64 -pflash -pflash -machine
> virt
> or
> qemu-system-riscv64 -bios \
> -pflash \
> -pflash \
> -machine virt
> or
> qemu-system-riscv64 -bios \
> -drive file=,if=pflash,format=raw,unit=0,readonly=on \
> -drive file=,if=pflash,format=raw,unit=1 \
> -machine virt
Please update the docs in docs/system/riscv/virt.rst to include how to
run EDK2 bios with these settings.
>
> Signed-off-by: Sunil V L
> Reported-by: Heinrich Schuchardt
> ---
> hw/riscv/virt.c | 51 ++---
> 1 file changed, 19 insertions(+), 32 deletions(-)
>
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index 4e3efbee16..1187a60d6e 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -1245,7 +1245,7 @@ static void virt_machine_done(Notifier *notifier, void
> *data)
> target_ulong firmware_end_addr, kernel_start_addr;
> const char *firmware_name = riscv_default_firmware_name(&s->soc[0]);
> uint32_t fdt_load_addr;
> -uint64_t kernel_entry;
> +uint64_t kernel_entry = 0;
>
> /*
> * Only direct boot kernel is currently supported for KVM VM,
> @@ -1266,42 +1266,29 @@ static void virt_machine_done(Notifier *notifier,
> void *data)
> firmware_end_addr = riscv_find_and_load_firmware(machine, firmware_name,
> start_addr, NULL);
>
> -if (drive_get(IF_PFLASH, 0, 1)) {
> -/*
> - * S-mode FW like EDK2 will be kept in second plash (unit 1).
> - * When both kernel, initrd and pflash options are provided in the
> - * command line, the kernel and initrd will be copied to the fw_cfg
> - * table and opensbi will jump to the flash address which is the
> - * entry point of S-mode FW. It is the job of the S-mode FW to load
> - * the kernel and initrd using fw_cfg table.
> - *
> - * If only pflash is given but not -kernel, then it is the job of
> - * of the S-mode firmware to locate and load the kernel.
> - * In either case, the next_addr for opensbi will be the flash
> address.
> - */
> -riscv_setup_firmware_boot(machine);
> -kernel_entry = virt_memmap[VIRT_FLASH].base +
> - virt_memmap[VIRT_FLASH].size / 2;
> -} else if (machine->kernel_filename) {
> +if (drive_get(IF_PFLASH, 0, 0)) {
> +if (machine->firmware && !strcmp(machine->firmware, "none")) {
> +/*
> + * Pflash was supplied but bios is none, let's overwrite the
> + * address we jump to after reset to the base of the flash.
> + */
> +start_addr = virt_memmap[VIRT_FLASH].base;
> +} else {
> +/*
> + * Pflash was supplied but bios is not none. In this case,
> + * base of the flash would contain S-mode payload.
> + */
> +riscv_setup_firmware_boot(machine);
> +kernel_entry = virt_memmap[VIRT_FLASH].base;
> +}
> +}
> +
> +if (machine->kernel_filename && !kernel_entry) {
> kernel_start_addr = riscv_calc_kernel_start_addr(&s->soc[0],
> firmware_end_addr);
>
> kernel_entry = riscv_load_kernel(machine, &s->soc[0],
> kernel_start_addr, true, NULL);
> -} else {
> - /*
> -* If dynamic firmware is used, it doesn't know where is the next mode
> -* if kernel argument is not set.
> -*/
> -kernel_entry = 0;
> -}
> -
> -if (drive_get(IF_PFLASH, 0, 0)) {
> -/*
> - * Pflash was supplied, let's overwrite the address we jump to after
> - * reset to the base o
[PATCH v2] target/riscv: Restore the predicate() NULL check behavior
When reading a non-existent CSR QEMU should raise illegal instruction
exception, but currently it just exits due to the g_assert() check.
This actually reverts commit 0ee342256af9205e7388efdf193a6d8f1ba1a617.
Some comments are also added to indicate that predicate() must be
provided for an implemented CSR.
Reported-by: Fei Wu
Signed-off-by: Bin Meng
Reviewed-by: Daniel Henrique Barboza
Reviewed-by: Weiwei Li
Reviewed-by: Alistair Francis
Reviewed-by: LIU Zhiwei
---
Changes in v2:
- rebase on top of Alistair's riscv-to-apply.next tree
target/riscv/csr.c | 11 +--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index f4d2dcfdc8..7000eb3350 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -3817,6 +3817,11 @@ static inline RISCVException
riscv_csrrw_check(CPURISCVState *env,
return RISCV_EXCP_ILLEGAL_INST;
}
+/* ensure CSR is implemented by checking predicate */
+if (!csr_ops[csrno].predicate) {
+return RISCV_EXCP_ILLEGAL_INST;
+}
+
/* privileged spec version check */
if (env->priv_ver < csr_min_priv) {
return RISCV_EXCP_ILLEGAL_INST;
@@ -3834,7 +3839,6 @@ static inline RISCVException
riscv_csrrw_check(CPURISCVState *env,
* illegal instruction exception should be triggered instead of virtual
* instruction exception. Hence this comes after the read / write check.
*/
-g_assert(csr_ops[csrno].predicate != NULL);
RISCVException ret = csr_ops[csrno].predicate(env, csrno);
if (ret != RISCV_EXCP_NONE) {
return ret;
@@ -4023,7 +4027,10 @@ static RISCVException write_jvt(CPURISCVState *env, int
csrno,
return RISCV_EXCP_NONE;
}
-/* Control and Status Register function table */
+/*
+ * Control and Status Register function table
+ * riscv_csr_operations::predicate() must be provided for an implemented CSR
+ */
riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
/* User Floating-Point CSRs */
[CSR_FFLAGS] = { "fflags", fs, read_fflags, write_fflags },
--
2.25.1
[PATCH] target/riscv: Restore the predicate() NULL check behavior
When reading a non-existent CSR QEMU should raise illegal instruction
exception, but currently it just exits due to the g_assert() check.
This actually reverts commit 0ee342256af9205e7388efdf193a6d8f1ba1a617,
Some comments are also added to indicate that predicate() must be
provided for an implemented CSR.
Reported-by: Fei Wu
Signed-off-by: Bin Meng
---
target/riscv/csr.c | 11 +--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index d522efc0b6..736ab64275 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -3797,6 +3797,11 @@ static inline RISCVException
riscv_csrrw_check(CPURISCVState *env,
return RISCV_EXCP_ILLEGAL_INST;
}
+/* ensure CSR is implemented by checking predicate */
+if (!csr_ops[csrno].predicate) {
+return RISCV_EXCP_ILLEGAL_INST;
+}
+
/* privileged spec version check */
if (env->priv_ver < csr_min_priv) {
return RISCV_EXCP_ILLEGAL_INST;
@@ -3814,7 +3819,6 @@ static inline RISCVException
riscv_csrrw_check(CPURISCVState *env,
* illegal instruction exception should be triggered instead of virtual
* instruction exception. Hence this comes after the read / write check.
*/
-g_assert(csr_ops[csrno].predicate != NULL);
RISCVException ret = csr_ops[csrno].predicate(env, csrno);
if (ret != RISCV_EXCP_NONE) {
return ret;
@@ -3991,7 +3995,10 @@ RISCVException riscv_csrrw_debug(CPURISCVState *env, int
csrno,
return ret;
}
-/* Control and Status Register function table */
+/*
+ * Control and Status Register function table
+ * riscv_csr_operations::predicate() must be provided for an implemented CSR
+ */
riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
/* User Floating-Point CSRs */
[CSR_FFLAGS] = { "fflags", fs, read_fflags, write_fflags },
--
2.25.1
Re: riscv: g_assert for NULL predicate?
On Wed, Apr 5, 2023 at 2:07 PM Alistair Francis wrote: > > On Mon, Apr 3, 2023 at 11:43 PM Wu, Fei wrote: > > > > Recent commit 0ee342256af92 switches to g_assert() for the predicate() > > NULL check from returning RISCV_EXCP_ILLEGAL_INST. Qemu doesn't have > > predicate() for un-allocated CSRs, then a buggy userspace application > > reads CSR such as 0x4 causes qemu to exit, I don't think it's expected. > > Hm That's not good. Userspace shouldn't be able to crash QEMU. I > think we want to revert that patch then. > > @Bin Meng any thoughts? > Agree, I will send a patch for this. Regards, Bin
Re: [PATCH] net: tap: Drop the close of fds for child process
On Thu, Apr 6, 2023 at 8:34 PM Philippe Mathieu-Daudé wrote:
>
> On 6/4/23 13:20, Bin Meng wrote:
> > Current codes using a brute-force traversal of all file descriptors
> > do not scale on a system where the maximum number of file descriptors
> > are set to a very large value (e.g.: in a Docker container of Manjaro
> > distribution it is set to 1073741816). QEMU just looks freezed during
> > start-up.
> >
> > The close-on-exec flag was introduced since a faily old Linux kernel
> > (2.6.23). With recent newer kernels that QEMU supports, we don't need
> > to manually close the fds for child process as the proper O_CLOEXEC
> > flag should have been set properly on files that we don't want child
> > process to see.
>
> But this file is common to all POSIX implementations, not only Linux.
Yes, this file is used for Linux, BSD and Solaris.
I checked that O_CLOEXEC is available on Linux (2.6.23), FreeBSD
(8.3), OpenBSD 5.0, Solaris 11. This flag is part of POSIX.1-2008.
Question is do we still need to support OSes that are older and do not
have this support?
>
> > Reported-by: Zhangjin Wu
> > Signed-off-by: Bin Meng
> > ---
> >
> > net/tap.c | 14 --
> > 1 file changed, 14 deletions(-)
> >
> > diff --git a/net/tap.c b/net/tap.c
> > index 1bf085d422..49e1915484 100644
> > --- a/net/tap.c
> > +++ b/net/tap.c
> > @@ -446,13 +446,6 @@ static void launch_script(const char *setup_script,
> > const char *ifname,
> > return;
> > }
> > if (pid == 0) {
>
> Maybe guard with #ifndef O_CLOEXEC
>
> > -int open_max = sysconf(_SC_OPEN_MAX), i;
> > -
> > -for (i = 3; i < open_max; i++) {
> > -if (i != fd) {
> > -close(i);
> > -}
> > -}
>
> or add qemu_close_cloexec() in util/osdep.c similar to qemu_open_cloexec()?
>
> > parg = args;
> > *parg++ = (char *)setup_script;
> > *parg++ = (char *)ifname;
> > @@ -536,17 +529,10 @@ static int net_bridge_run_helper(const char *helper,
> > const char *bridge,
> > return -1;
> > }
> > if (pid == 0) {
> > -int open_max = sysconf(_SC_OPEN_MAX), i;
> > char *fd_buf = NULL;
> > char *br_buf = NULL;
> > char *helper_cmd = NULL;
> >
> > -for (i = 3; i < open_max; i++) {
> > -if (i != sv[1]) {
> > -close(i);
> > -}
> > -}
> > -
> > fd_buf = g_strdup_printf("%s%d", "--fd=", sv[1]);
> >
> > if (strrchr(helper, ' ') || strrchr(helper, '\t')) {
Regards,
Bin
[PATCH] net: tap: Drop the close of fds for child process
Current codes using a brute-force traversal of all file descriptors
do not scale on a system where the maximum number of file descriptors
are set to a very large value (e.g.: in a Docker container of Manjaro
distribution it is set to 1073741816). QEMU just looks freezed during
start-up.
The close-on-exec flag was introduced since a faily old Linux kernel
(2.6.23). With recent newer kernels that QEMU supports, we don't need
to manually close the fds for child process as the proper O_CLOEXEC
flag should have been set properly on files that we don't want child
process to see.
Reported-by: Zhangjin Wu
Signed-off-by: Bin Meng
---
net/tap.c | 14 --
1 file changed, 14 deletions(-)
diff --git a/net/tap.c b/net/tap.c
index 1bf085d422..49e1915484 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -446,13 +446,6 @@ static void launch_script(const char *setup_script, const
char *ifname,
return;
}
if (pid == 0) {
-int open_max = sysconf(_SC_OPEN_MAX), i;
-
-for (i = 3; i < open_max; i++) {
-if (i != fd) {
-close(i);
-}
-}
parg = args;
*parg++ = (char *)setup_script;
*parg++ = (char *)ifname;
@@ -536,17 +529,10 @@ static int net_bridge_run_helper(const char *helper,
const char *bridge,
return -1;
}
if (pid == 0) {
-int open_max = sysconf(_SC_OPEN_MAX), i;
char *fd_buf = NULL;
char *br_buf = NULL;
char *helper_cmd = NULL;
-for (i = 3; i < open_max; i++) {
-if (i != sv[1]) {
-close(i);
-}
-}
-
fd_buf = g_strdup_printf("%s%d", "--fd=", sv[1]);
if (strrchr(helper, ' ') || strrchr(helper, '\t')) {
--
2.34.1
Re: [PATCH v2 09/10] contrib/gitdm: add more individual contributors
On 2023/3/11 2:03:31, "Alex Bennée" wrote: I'll only add names explicitly acked here. Let me know if you want contributions mapped to a company instead. Signed-off-by: Alex Bennée Cc: Bernhard Beschow Cc: Amarjargal Gundjalam Cc: Bin Meng Cc: Jason A. Donenfeld Cc: Strahinja Jankovic --- contrib/gitdm/group-map-individuals | 5 + 1 file changed, 5 insertions(+) diff --git a/contrib/gitdm/group-map-individuals b/contrib/gitdm/group-map-individuals index e2263a5ee3..0e4618f1ce 100644 --- a/contrib/gitdm/group-map-individuals +++ b/contrib/gitdm/group-map-individuals @@ -38,3 +38,8 @@ p...@nowt.org g...@xen0n.name si...@simonsafar.com research_tra...@irq.a4lg.com +shen...@gmail.com +bm...@tinylab.org +amarjarga...@gmail.com +strahinjapjanko...@gmail.com +ja...@zx2c4.com -- Acked-by: Bin Meng
Re: [PATCH qemu v2] linux-user: Emulate /proc/cpuinfo output for riscv
On Tue, Mar 14, 2023 at 4:29 AM ~abordado wrote:
>
> From: Afonso Bordado
>
> RISC-V does not expose all extensions via hwcaps, thus some userspace
> applications may want to query these via /proc/cpuinfo.
>
> Currently when querying this file the host's file is shown instead
> which is slightly confusing. Emulate a basic /proc/cpuinfo file
> with mmu info and an ISA string.
>
> Changes from V1:
The changelog should go below ---
>
> - Call `g_free` on ISA string.
> - Use `riscv_cpu_cfg` API.
> - Query `cpu_env->xl` to check for RV32.
>
> Signed-off-by: Afonso Bordado
> Reviewed-by: Palmer Dabbelt
> Acked-by: Palmer Dabbelt
> Reviewed-by: Laurent Vivier
> ---
> linux-user/syscall.c | 34 +--
> tests/tcg/riscv64/Makefile.target | 1 +
> tests/tcg/riscv64/cpuinfo.c | 30 +++
> 3 files changed, 63 insertions(+), 2 deletions(-)
> create mode 100644 tests/tcg/riscv64/cpuinfo.c
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 24cea6fb6a..0388f8b0b0 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -8230,7 +8230,8 @@ void target_exception_dump(CPUArchState *env, const
> char *fmt, int code)
> }
>
> #if HOST_BIG_ENDIAN != TARGET_BIG_ENDIAN || \
> -defined(TARGET_SPARC) || defined(TARGET_M68K) || defined(TARGET_HPPA)
> +defined(TARGET_SPARC) || defined(TARGET_M68K) || defined(TARGET_HPPA) ||
> \
> +defined(TARGET_RISCV)
> static int is_proc(const char *filename, const char *entry)
> {
> return strcmp(filename, entry) == 0;
> @@ -8308,6 +8309,35 @@ static int open_cpuinfo(CPUArchState *cpu_env, int fd)
> }
> #endif
>
> +#if defined(TARGET_RISCV)
> +static int open_cpuinfo(CPUArchState *cpu_env, int fd)
> +{
> +int i;
> +int num_cpus = sysconf(_SC_NPROCESSORS_ONLN);
> +RISCVCPU *cpu = env_archcpu(cpu_env);
> +const RISCVCPUConfig *cfg = riscv_cpu_cfg((CPURISCVState *) cpu_env);
> +char *isa_string = riscv_isa_string(cpu);
> +const char *mmu;
> +
> +if (cfg->mmu) {
> +mmu = (cpu_env->xl == MXL_RV32) ? "sv32" : "sv48";
> +} else {
> +mmu = "none";
> +}
> +
> +for (i = 0; i < num_cpus; i++) {
> +dprintf(fd, "processor\t: %d\n", i);
> +dprintf(fd, "hart\t\t: %d\n", i);
> +dprintf(fd, "isa\t\t: %s\n", isa_string);
> +dprintf(fd, "mmu\t\t: %s\n", mmu);
> +dprintf(fd, "uarch\t\t: qemu\n\n");
> +}
> +
> +g_free(isa_string);
> +return 0;
> +}
> +#endif
> +
> #if defined(TARGET_M68K)
> static int open_hardware(CPUArchState *cpu_env, int fd)
> {
> @@ -8332,7 +8362,7 @@ static int do_openat(CPUArchState *cpu_env, int dirfd,
> const char *pathname, int
> #if HOST_BIG_ENDIAN != TARGET_BIG_ENDIAN
> { "/proc/net/route", open_net_route, is_proc },
> #endif
> -#if defined(TARGET_SPARC) || defined(TARGET_HPPA)
> +#if defined(TARGET_SPARC) || defined(TARGET_HPPA) || defined(TARGET_RISCV)
> { "/proc/cpuinfo", open_cpuinfo, is_proc },
> #endif
> #if defined(TARGET_M68K)
> diff --git a/tests/tcg/riscv64/Makefile.target
> b/tests/tcg/riscv64/Makefile.target
> index cc3ed65ffd..df93a2ce1f 100644
> --- a/tests/tcg/riscv64/Makefile.target
> +++ b/tests/tcg/riscv64/Makefile.target
> @@ -4,6 +4,7 @@
> VPATH += $(SRC_PATH)/tests/tcg/riscv64
> TESTS += test-div
> TESTS += noexec
> +TESTS += cpuinfo
>
> # Disable compressed instructions for test-noc
> TESTS += test-noc
> diff --git a/tests/tcg/riscv64/cpuinfo.c b/tests/tcg/riscv64/cpuinfo.c
> new file mode 100644
> index 00..296abd0a8c
> --- /dev/null
> +++ b/tests/tcg/riscv64/cpuinfo.c
> @@ -0,0 +1,30 @@
> +#include
> +#include
> +#include
> +#include
> +
> +#define BUFFER_SIZE 1024
> +
> +int main(void)
> +{
> +char buffer[BUFFER_SIZE];
> +FILE *fp = fopen("/proc/cpuinfo", "r");
> +assert(fp != NULL);
> +
> +while (fgets(buffer, BUFFER_SIZE, fp) != NULL) {
> +if (strstr(buffer, "processor") != NULL) {
> +assert(strstr(buffer, "processor\t: ") == buffer);
> +} else if (strstr(buffer, "hart") != NULL) {
> +assert(strstr(buffer, "hart\t\t: ") == buffer);
> +} else if (strstr(buffer, "isa") != NULL) {
> +assert(strcmp(buffer, "isa\t\t: rv64imafdc_zicsr_zifencei\n") ==
> 0);
> +} else if (strstr(buffer, "mmu") != NULL) {
> +assert(strcmp(buffer, "mmu\t\t: sv48\n") == 0);
> +} else if (strstr(buffer, "uarch") != NULL) {
> +assert(strcmp(buffer, "uarch\t\t: qemu\n") == 0);
> +}
> +}
> +
> +fclose(fp);
> +return 0;
> +}
Regards,
Bin
Re: [PATCH-for-8.0] gitlab-ci: Remove job building EDK2 firmware binaries
On Mon, Mar 13, 2023 at 6:01 PM Philippe Mathieu-Daudé
wrote:
>
> On 13/3/23 10:35, Bin Meng wrote:
> > Hi Philippe,
> >
> > On Mon, Mar 13, 2023 at 4:51 PM Philippe Mathieu-Daudé
> > wrote:
> >>
> >> On 13/3/23 03:09, Bin Meng wrote:
> >>> On Fri, Mar 10, 2023 at 9:50 PM Philippe Mathieu-Daudé
> >>> wrote:
> >>>>
> >>>> On 10/3/23 14:38, Peter Maydell wrote:
> >>>>> On Fri, 10 Mar 2023 at 13:33, Philippe Mathieu-Daudé
> >>>>> wrote:
> >>>>>>
> >>>>>> When we introduced this Gitlab-CI job in commit 71920809ce
> >>>>>> ("gitlab-ci.yml: Add jobs to build EDK2 firmware binaries"),
> >>>>>> the naive plan was to have reproducible binaries by downloading
> >>>>>> what this job would build, testing it and eventually committing
> >>>>>> it. With retrospective, nothing happened 3 years later and this
> >>>>>> job is just bitrotting:
> >>>>>>
> >>>>>> Step 1/3 : FROM ubuntu:18.04
> >>>>>> 18.04: Pulling from library/ubuntu
> >>>>>> mediaType in manifest should be
> >>>>>> 'application/vnd.docker.distribution.manifest.v2+json' not
> >>>>>> 'application/vnd.oci.image.manifest.v1+json'
> >>>>>>
> >>>>>> Remove this job to avoid wasting maintenance and CI ressources.
> >>>>>
> >>>>> Does the same thing hold for the opensbi job ?
> >>>>
> >>>> Cc'ing Bin, I have no idea.
> >>>>
> >>>
> >>> The OpenSBI job now builds fine. I have no preference on keeping vs.
> >>> removing it.
> >>>
> >>> I remember our previous goal was to create CI jobs for every pc-bios
> >>> image but apparently that never happened.
> >>
> >> Yes, and I don't see interest in the community (neither worry that
> >> pc-bios/ images committed are built on each maintainer workstations).
> >>
> >> If it isn't consumed by QEMU, then better remove that job and save
> >> precious CI minutes. I presume OpenSBI itself is already tested
> >> by its mainstream project.
> >
> > Not sure what does "consumed" here mean?
> >
> > QEMU uses OpenSBI images on RISC-V machines by default.
>
> QEMU repository allows building QEMU system binaries which 'consume'
> the following (committed) files:
> - pc-bios/opensbi-riscv32-generic-fw_dynamic.bin
> - pc-bios/opensbi-riscv64-generic-fw_dynamic.bin
>
> We don't need to run the build-opensbi job to run QEMU: we use the
> prebuilt images.
Okay, thanks for the clarification. I believe that's the case for
every pc-bios image? If yes, we don't need to build these pc-bios
images in QEMU CI.
Regards,
Bin
Re: [PATCH-for-8.0] gitlab-ci: Remove job building EDK2 firmware binaries
Hi Philippe,
On Mon, Mar 13, 2023 at 4:51 PM Philippe Mathieu-Daudé
wrote:
>
> On 13/3/23 03:09, Bin Meng wrote:
> > On Fri, Mar 10, 2023 at 9:50 PM Philippe Mathieu-Daudé
> > wrote:
> >>
> >> On 10/3/23 14:38, Peter Maydell wrote:
> >>> On Fri, 10 Mar 2023 at 13:33, Philippe Mathieu-Daudé
> >>> wrote:
> >>>>
> >>>> When we introduced this Gitlab-CI job in commit 71920809ce
> >>>> ("gitlab-ci.yml: Add jobs to build EDK2 firmware binaries"),
> >>>> the naive plan was to have reproducible binaries by downloading
> >>>> what this job would build, testing it and eventually committing
> >>>> it. With retrospective, nothing happened 3 years later and this
> >>>> job is just bitrotting:
> >>>>
> >>>> Step 1/3 : FROM ubuntu:18.04
> >>>> 18.04: Pulling from library/ubuntu
> >>>> mediaType in manifest should be
> >>>> 'application/vnd.docker.distribution.manifest.v2+json' not
> >>>> 'application/vnd.oci.image.manifest.v1+json'
> >>>>
> >>>> Remove this job to avoid wasting maintenance and CI ressources.
> >>>
> >>> Does the same thing hold for the opensbi job ?
> >>
> >> Cc'ing Bin, I have no idea.
> >>
> >
> > The OpenSBI job now builds fine. I have no preference on keeping vs.
> > removing it.
> >
> > I remember our previous goal was to create CI jobs for every pc-bios
> > image but apparently that never happened.
>
> Yes, and I don't see interest in the community (neither worry that
> pc-bios/ images committed are built on each maintainer workstations).
>
> If it isn't consumed by QEMU, then better remove that job and save
> precious CI minutes. I presume OpenSBI itself is already tested
> by its mainstream project.
Not sure what does "consumed" here mean?
QEMU uses OpenSBI images on RISC-V machines by default.
Regards,
Bin
Re: [PATCH-for-8.0] gitlab-ci: Remove job building EDK2 firmware binaries
On Fri, Mar 10, 2023 at 9:50 PM Philippe Mathieu-Daudé
wrote:
>
> On 10/3/23 14:38, Peter Maydell wrote:
> > On Fri, 10 Mar 2023 at 13:33, Philippe Mathieu-Daudé
> > wrote:
> >>
> >> When we introduced this Gitlab-CI job in commit 71920809ce
> >> ("gitlab-ci.yml: Add jobs to build EDK2 firmware binaries"),
> >> the naive plan was to have reproducible binaries by downloading
> >> what this job would build, testing it and eventually committing
> >> it. With retrospective, nothing happened 3 years later and this
> >> job is just bitrotting:
> >>
> >>Step 1/3 : FROM ubuntu:18.04
> >>18.04: Pulling from library/ubuntu
> >>mediaType in manifest should be
> >>'application/vnd.docker.distribution.manifest.v2+json' not
> >>'application/vnd.oci.image.manifest.v1+json'
> >>
> >> Remove this job to avoid wasting maintenance and CI ressources.
> >
> > Does the same thing hold for the opensbi job ?
>
> Cc'ing Bin, I have no idea.
>
The OpenSBI job now builds fine. I have no preference on keeping vs.
removing it.
I remember our previous goal was to create CI jobs for every pc-bios
image but apparently that never happened.
Regards,
Bin
Re: [PATCH v5 00/16] hw/9pfs: Add 9pfs support for Windows
On Mon, Mar 6, 2023 at 10:15 PM Christian Schoenebeck wrote: > > On Monday, February 20, 2023 11:07:59 AM CET Bin Meng wrote: > > At present there is no Windows support for 9p file system. > > This series adds initial Windows support for 9p file system. > > > > 'local' file system backend driver is supported on Windows, > > including open, read, write, close, rename, remove, etc. > > All security models are supported. The mapped (mapped-xattr) > > security model is implemented using NTFS Alternate Data Stream > > (ADS) so the 9p export path shall be on an NTFS partition. > > > > 'synth' driver is adapted for Windows too so that we can now > > run qtests on Windows for 9p related regression testing. > > > > Example command line to test: > > "-fsdev local,path=c:\msys64,security_model=mapped,id=p9 -device > virtio-9p-pci,fsdev=p9,mount_tag=p9fs" > > > > Changes in v5: > > - rework Windows specific xxxdir() APIs implementation > > I didn't have the chance to look at this v5 yet. > > In general it would help for review to point out in the cover letter which > patch(es) have changed, what decisions you have made and why. > > In this case I guess that's patch 4. > Yes, it's patch 4, and v5 is reworked following your comments regarding patch 4 of v4. Regards, Bin
Re: [PATCH v5 03/16] hw/9pfs: Replace the direct call to xxxdir() APIs with a wrapper
On Mon, Mar 6, 2023 at 5:32 PM Philippe Mathieu-Daudé wrote: > > On 20/2/23 11:08, Bin Meng wrote: > > From: Guohuai Shi > > > > xxxdir() APIs are not safe on Windows host. For future extension to > > Windows, let's replace the direct call to xxxdir() APIs with a wrapper. > > > > Signed-off-by: Guohuai Shi > > Signed-off-by: Bin Meng > > --- > > > > hw/9pfs/9p-util.h | 14 ++ > > hw/9pfs/9p-local.c | 12 ++-- > > 2 files changed, 20 insertions(+), 6 deletions(-) > > > > +#define qemu_opendiropendir_win32 > > +#define qemu_closedir closedir_win32 > > +#define qemu_readdirreaddir_win32 > > +#define qeme_rewinddir rewinddir_win32 > > Typo :) > Ouch! Thanks Philippe :) Regards, Bin
Re: [PATCH v5 00/16] hw/9pfs: Add 9pfs support for Windows
On Mon, Feb 20, 2023 at 6:10 PM Bin Meng wrote: > > At present there is no Windows support for 9p file system. > This series adds initial Windows support for 9p file system. > > 'local' file system backend driver is supported on Windows, > including open, read, write, close, rename, remove, etc. > All security models are supported. The mapped (mapped-xattr) > security model is implemented using NTFS Alternate Data Stream > (ADS) so the 9p export path shall be on an NTFS partition. > > 'synth' driver is adapted for Windows too so that we can now > run qtests on Windows for 9p related regression testing. > > Example command line to test: > "-fsdev local,path=c:\msys64,security_model=mapped,id=p9 -device > virtio-9p-pci,fsdev=p9,mount_tag=p9fs" > > Changes in v5: > - rework Windows specific xxxdir() APIs implementation > > Bin Meng (2): > hw/9pfs: Update helper qemu_stat_rdev() > hw/9pfs: Add a helper qemu_stat_blksize() > > Guohuai Shi (14): > hw/9pfs: Add missing definitions for Windows > hw/9pfs: Implement Windows specific utilities functions for 9pfs > hw/9pfs: Replace the direct call to xxxdir() APIs with a wrapper > hw/9pfs: Implement Windows specific xxxdir() APIs > hw/9pfs: Update the local fs driver to support Windows > hw/9pfs: Support getting current directory offset for Windows > hw/9pfs: Disable unsupported flags and features for Windows > hw/9pfs: Update v9fs_set_fd_limit() for Windows > hw/9pfs: Add Linux error number definition > hw/9pfs: Translate Windows errno to Linux value > fsdev: Disable proxy fs driver on Windows > hw/9pfs: Update synth fs driver for Windows > tests/qtest: virtio-9p-test: Adapt the case for win32 > meson.build: Turn on virtfs for Windows Ping?
Re: [PATCH v2 1/2] gitlab/opensbi: Move to docker:stable
On Sat, Mar 4, 2023 at 4:25 AM Palmer Dabbelt wrote: > > The OpenSBI build has been using docker:19.03.1, which appears to be old > enough that v2 of the manifest is no longer supported. Something has > started serving us those manifests, resulting in errors along the lines > of > > $ docker build --cache-from $IMAGE_TAG --tag > $CI_REGISTRY_IMAGE:$CI_COMMIT_SHA --tag $IMAGE_TAG .gitlab-ci.d/opensbi > Step 1/7 : FROM ubuntu:18.04 > 18.04: Pulling from library/ubuntu > mediaType in manifest should be > 'application/vnd.docker.distribution.manifest.v2+json' not > 'application/vnd.oci.image.manifest.v1+json' > > This moves to docker:stable, as was suggested by the template. It also > adds the python3 package via apt, as OpenSBI requires that to build. > > Signed-off-by: Palmer Dabbelt > --- > .gitlab-ci.d/opensbi.yml| 4 ++-- > .gitlab-ci.d/opensbi/Dockerfile | 1 + > 2 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/.gitlab-ci.d/opensbi.yml b/.gitlab-ci.d/opensbi.yml > index 04ed5a3ea1..9a651465d8 100644 > --- a/.gitlab-ci.d/opensbi.yml > +++ b/.gitlab-ci.d/opensbi.yml > @@ -42,9 +42,9 @@ > docker-opensbi: >extends: .opensbi_job_rules >stage: containers > - image: docker:19.03.1 > + image: docker:stable >services: > -- docker:19.03.1-dind > +- docker:stable-dind >variables: > GIT_DEPTH: 3 > IMAGE_TAG: $CI_REGISTRY_IMAGE:opensbi-cross-build > diff --git a/.gitlab-ci.d/opensbi/Dockerfile b/.gitlab-ci.d/opensbi/Dockerfile > index 4ba8a4de86..2d151a6bc8 100644 > --- a/.gitlab-ci.d/opensbi/Dockerfile > +++ b/.gitlab-ci.d/opensbi/Dockerfile > @@ -16,6 +16,7 @@ RUN apt update \ > git \ > make \ > wget \ > + python3 \ nits: this should be inserted before wget to follow the alphabetical order > && \ > \ > rm -rf /var/lib/apt/lists/* > -- Reviewed-by: Bin Meng
Re: [PATCH v2 1/1] hw/riscv/virt.c: add cbo[mz]-block-size fdt properties
On Thu, Mar 2, 2023 at 5:16 PM Daniel Henrique Barboza wrote: > > From: Anup Patel > > The cbom-block-size fdt property property is used to inform the OS about > the blocksize in bytes for the Zicbom cache operations. Linux documents > it in Documentation/devicetree/bindings/riscv/cpus.yaml > as: > > riscv,cbom-block-size: > $ref: /schemas/types.yaml#/definitions/uint32 > description: > The blocksize in bytes for the Zicbom cache operations. > > cboz-block-size has the same role but for the Zicboz extension, i.e. > informs the size in bytes for Zicboz cache operations. Linux support > for it is under review/approval in [1]. Patch 3 of that series describes > cboz-block-size as: > > riscv,cboz-block-size: > $ref: /schemas/types.yaml#/definitions/uint32 > description: > The blocksize in bytes for the Zicboz cache operations. > > [1] > https://lore.kernel.org/all/20230224162631.405473-1-ajo...@ventanamicro.com/ > > Cc: Andrew Jones > Signed-off-by: Anup Patel > Signed-off-by: Daniel Henrique Barboza > --- > hw/riscv/virt.c | 11 +++ > 1 file changed, 11 insertions(+) > Reviewed-by: Bin Meng
Re: [PATCH 0/4] RISCVCPUConfig related cleanups
Hi Palmer,
On Thu, Mar 2, 2023 at 10:08 AM Palmer Dabbelt wrote:
>
> On Fri, 24 Feb 2023 09:45:16 PST (-0800), dbarb...@ventanamicro.com wrote:
> > Hi,
> >
> > These cleanups were suggested by LIU Zhiwei during the review of the
> > RISCV_FEATURE_* cleanups, currently on version 7 [1].
> >
> > These are dependent on the patch "[PATCH v7 01/10] target/riscv: introduce
> > riscv_cpu_cfg()" from [1] because we use the riscv_cpu_cfg() API.
> >
> >
> > [1] https://lists.gnu.org/archive/html/qemu-devel/2023-02/msg06467.html
> >
> > Daniel Henrique Barboza (4):
> > target/riscv/csr.c: use env_archcpu() in ctr()
> > target/riscv/csr.c: simplify mctr()
> > target/riscv/csr.c: use riscv_cpu_cfg() to avoid env_cpu() pointers
> > target/riscv/csr.c: avoid env_archcpu() usages when reading
> > RISCVCPUConfig
> >
> > target/riscv/csr.c | 90 +-
> > 1 file changed, 24 insertions(+), 66 deletions(-)
>
> I just based these on that patch, which landed as d4ea711704
> ("target/riscv: introduce riscv_cpu_cfg()"). That resulted in a handful
> of merge conflicts, but everything looked pretty mechanical. So it's
> queued up.
>
As Weiwei pointed out in
https://lore.kernel.org/qemu-devel/e40e75ff-37e0-94d3-e9e2-c159b0e2d...@iscas.ac.cn/,
patch#1 should be dropped.
But I see it was landed up in your tree @
https://github.com/palmer-dabbelt/qemu/commit/3c7d54f945f1b5b474ea35c0815a1618927c9384,
while my changes are already in tree @
https://github.com/palmer-dabbelt/qemu/commit/94e297071bc0a5965cc32c497a886f2cf9d32710.
Not sure why git doesn't figure that out ...
Regards,
Bin
Re: [PATCH v2 05/18] target/riscv: gdbstub: Do not generate CSR XML if Zicsr is disabled
On Thu, Mar 2, 2023 at 7:43 AM Palmer Dabbelt wrote: > > On Wed, 01 Mar 2023 01:55:34 PST (-0800), Bin Meng wrote: > > On Wed, Mar 1, 2023 at 5:52 PM LIU Zhiwei > > wrote: > >> > >> > >> On 2023/2/28 18:40, Bin Meng wrote: > >> > There is no need to generate the CSR XML if the Zicsr extension > >> > is not enabled. > >> > >> Should we generate the FPU XML or Vector XML when Zicsr is not enabled? > > > > Good point. I think we should disable that too. > > Seems reasonable. Did you want to do that as part of a v3, or just as a > follow-on fix? > I looked at this further. The FPU / Vector XML is guarded by the " env->misa_ext" check. If Zicsr is disabled while F or V extension is off, QEMU will error out in riscv_cpu_realize() earlier before the gdbstub init. So current patch should be fine. Regards, Bin
Re: [PATCH v6 0/8] net: Pad short frames for network backends
Hi Cédric,
On Wed, Mar 1, 2023 at 5:13 PM Cédric Le Goater wrote:
>
> Hello Bin,
>
> On 3/1/23 10:01, bmeng...@gmail.com wrote:
> > From: Bin Meng
> >
> > The minimum Ethernet frame length is 60 bytes. For short frames with
> > smaller length like ARP packets (only 42 bytes), on a real world NIC
> > it can choose either padding its length to the minimum required 60
> > bytes, or sending it out directly to the wire. Such behavior can be
> > hardcoded or controled by a register bit. Similarly on the receive
> > path, NICs can choose either dropping such short frames directly or
> > handing them over to software to handle.
> >
> > On the other hand, for the network backends like SLiRP/TAP, they
> > don't expose a way to control the short frame behavior. As of today
> > they just send/receive data from/to the other end connected to them,
> > which means any sized packet is acceptable. So they can send and
> > receive short frames without any problem. It is observed that ARP
> > packets sent from SLiRP/TAP are 42 bytes, and SLiRP/TAP just send
> > these ARP packets to the other end which might be a NIC model that
> > does not allow short frames to pass through.
> >
> > To provide better compatibility, for packets sent from QEMU network
> > backends like SLiRP/TAP, we change to pad short frames before sending
> > it out to the other end, if the other end does not forbid it via the
> > nc->do_not_pad flag. This ensures a backend as an Ethernet sender
> > does not violate the spec. But with this change, the behavior of
> > dropping short frames from SLiRP/TAP interfaces in the NIC model
> > cannot be emulated because it always receives a packet that is spec
> > complaint. The capability of sending short frames from NIC models is
> > still supported and short frames can still pass through SLiRP/TAP.
> >
> > This series should be able to fix the issue as reported with some
> > NIC models before, that ARP requests get dropped, preventing the
> > guest from becoming visible on the network. It was workarounded in
> > these NIC models on the receive path, that when a short frame is
> > received, it is padded up to 60 bytes.
>
> I guess we can drop this code in ftgmac100.c also then :
>
> /* TODO : Pad to minimum Ethernet frame length */
> /* handle small packets. */
> if (size < 10) {
> qemu_log_mask(LOG_GUEST_ERROR, "%s: dropped frame of %zd bytes\n",
>__func__, size);
> return size;
> }
>
> Correct ? No need to resend. I can take care of it.
>
Yes, I think so. Thanks!
Regards,
Bin
Re: [PATCH v2 05/18] target/riscv: gdbstub: Do not generate CSR XML if Zicsr is disabled
On Wed, Mar 1, 2023 at 5:52 PM LIU Zhiwei wrote: > > > On 2023/2/28 18:40, Bin Meng wrote: > > There is no need to generate the CSR XML if the Zicsr extension > > is not enabled. > > Should we generate the FPU XML or Vector XML when Zicsr is not enabled? Good point. I think we should disable that too. > > Zhiwei > Regards, Bin
[PATCH v2 16/18] target/riscv: Allow debugger to access sstc CSRs
From: Bin Meng
At present with a debugger attached sstc CSRs can only be accssed
when CPU is in M-mode, or configured correctly.
Fix it by adjusting their predicate() routine logic so that the
static config check comes before the run-time check, as well as
adding a debugger check.
Signed-off-by: Bin Meng
Reviewed-by: Weiwei Li
---
(no changes since v1)
target/riscv/csr.c | 19 ++-
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index a0e70f5ba0..020c3f524f 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -952,6 +952,19 @@ static RISCVException sstc(CPURISCVState *env, int csrno)
return RISCV_EXCP_ILLEGAL_INST;
}
+if ((csrno == CSR_VSTIMECMP) || (csrno == CSR_VSTIMECMPH)) {
+hmode_check = true;
+}
+
+RISCVException ret = hmode_check ? hmode(env, csrno) : smode(env, csrno);
+if (ret != RISCV_EXCP_NONE) {
+return ret;
+}
+
+if (env->debugger) {
+return RISCV_EXCP_NONE;
+}
+
if (env->priv == PRV_M) {
return RISCV_EXCP_NONE;
}
@@ -972,11 +985,7 @@ static RISCVException sstc(CPURISCVState *env, int csrno)
}
}
-if ((csrno == CSR_VSTIMECMP) || (csrno == CSR_VSTIMECMPH)) {
-hmode_check = true;
-}
-
-return hmode_check ? hmode(env, csrno) : smode(env, csrno);
+return RISCV_EXCP_NONE;
}
static RISCVException sstc_32(CPURISCVState *env, int csrno)
--
2.25.1
[PATCH v2 18/18] target/riscv: Group all predicate() routines together
From: Bin Meng
Move sstc()/sstc32() to where all predicate() routines live, and
smstateen_acc_ok() to near {read,write}_xenvcfg().
Signed-off-by: Bin Meng
Reviewed-by: Weiwei Li
---
Changes in v2:
- move smstateen_acc_ok() to near {read,write}_xenvcfg()
target/riscv/csr.c | 177 ++---
1 file changed, 87 insertions(+), 90 deletions(-)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 785f6f4d45..3a7e0217e2 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -40,42 +40,6 @@ void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops)
csr_ops[csrno & (CSR_TABLE_SIZE - 1)] = *ops;
}
-/* Predicates */
-#if !defined(CONFIG_USER_ONLY)
-static RISCVException smstateen_acc_ok(CPURISCVState *env, int index,
- uint64_t bit)
-{
-bool virt = riscv_cpu_virt_enabled(env);
-RISCVCPU *cpu = env_archcpu(env);
-
-if (env->priv == PRV_M || !cpu->cfg.ext_smstateen) {
-return RISCV_EXCP_NONE;
-}
-
-if (!(env->mstateen[index] & bit)) {
-return RISCV_EXCP_ILLEGAL_INST;
-}
-
-if (virt) {
-if (!(env->hstateen[index] & bit)) {
-return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
-}
-
-if (env->priv == PRV_U && !(env->sstateen[index] & bit)) {
-return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
-}
-}
-
-if (env->priv == PRV_U && riscv_has_ext(env, RVS)) {
-if (!(env->sstateen[index] & bit)) {
-return RISCV_EXCP_ILLEGAL_INST;
-}
-}
-
-return RISCV_EXCP_NONE;
-}
-#endif
-
static RISCVException fs(CPURISCVState *env, int csrno)
{
#if !defined(CONFIG_USER_ONLY)
@@ -399,6 +363,60 @@ static RISCVException sstateen(CPURISCVState *env, int
csrno)
return RISCV_EXCP_NONE;
}
+static RISCVException sstc(CPURISCVState *env, int csrno)
+{
+RISCVCPU *cpu = env_archcpu(env);
+bool hmode_check = false;
+
+if (!cpu->cfg.ext_sstc || !env->rdtime_fn) {
+return RISCV_EXCP_ILLEGAL_INST;
+}
+
+if ((csrno == CSR_VSTIMECMP) || (csrno == CSR_VSTIMECMPH)) {
+hmode_check = true;
+}
+
+RISCVException ret = hmode_check ? hmode(env, csrno) : smode(env, csrno);
+if (ret != RISCV_EXCP_NONE) {
+return ret;
+}
+
+if (env->debugger) {
+return RISCV_EXCP_NONE;
+}
+
+if (env->priv == PRV_M) {
+return RISCV_EXCP_NONE;
+}
+
+/*
+ * No need of separate function for rv32 as menvcfg stores both menvcfg
+ * menvcfgh for RV32.
+ */
+if (!(get_field(env->mcounteren, COUNTEREN_TM) &&
+ get_field(env->menvcfg, MENVCFG_STCE))) {
+return RISCV_EXCP_ILLEGAL_INST;
+}
+
+if (riscv_cpu_virt_enabled(env)) {
+if (!(get_field(env->hcounteren, COUNTEREN_TM) &&
+ get_field(env->henvcfg, HENVCFG_STCE))) {
+return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
+}
+}
+
+return RISCV_EXCP_NONE;
+}
+
+static RISCVException sstc_32(CPURISCVState *env, int csrno)
+{
+if (riscv_cpu_mxl(env) != MXL_RV32) {
+return RISCV_EXCP_ILLEGAL_INST;
+}
+
+return sstc(env, csrno);
+}
+
/* Checks if PointerMasking registers could be accessed */
static RISCVException pointer_masking(CPURISCVState *env, int csrno)
{
@@ -943,60 +961,6 @@ static RISCVException read_timeh(CPURISCVState *env, int
csrno,
return RISCV_EXCP_NONE;
}
-static RISCVException sstc(CPURISCVState *env, int csrno)
-{
-RISCVCPU *cpu = env_archcpu(env);
-bool hmode_check = false;
-
-if (!cpu->cfg.ext_sstc || !env->rdtime_fn) {
-return RISCV_EXCP_ILLEGAL_INST;
-}
-
-if ((csrno == CSR_VSTIMECMP) || (csrno == CSR_VSTIMECMPH)) {
-hmode_check = true;
-}
-
-RISCVException ret = hmode_check ? hmode(env, csrno) : smode(env, csrno);
-if (ret != RISCV_EXCP_NONE) {
-return ret;
-}
-
-if (env->debugger) {
-return RISCV_EXCP_NONE;
-}
-
-if (env->priv == PRV_M) {
-return RISCV_EXCP_NONE;
-}
-
-/*
- * No need of separate function for rv32 as menvcfg stores both menvcfg
- * menvcfgh for RV32.
- */
-if (!(get_field(env->mcounteren, COUNTEREN_TM) &&
- get_field(env->menvcfg, MENVCFG_STCE))) {
-return RISCV_EXCP_ILLEGAL_INST;
-}
-
-if (riscv_cpu_virt_enabled(env)) {
-if (!(get_field(env->hcounteren, COUNTEREN_TM) &&
- get_field(env->henvcfg, HENVCFG_STCE))) {
-return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
-}
-}
-
-return RISCV_EXCP_NONE;
-}
-
-static RISCVException sstc_32(CPURISCVState *env, int csrno)
-{
-if (riscv_cpu_mxl(env) != MXL_RV32) {
-return RISCV_EXCP_ILLEGAL_INST;
-}
-
-return sstc(env, csrno);
-}
-
static RISCVException read_vstimecmp(CPURISCVState *env, int csrno,
[PATCH v2 12/18] target/riscv: gdbstub: Drop the vector CSRs in riscv-vector.xml
From: Bin Meng
It's worth noting that the vector CSR predicate() has a similar
run-time check logic to the FPU CSR. With the previous patch our
gdbstub can correctly report these vector CSRs via the CSR xml.
Commit 719d3561b269 ("target/riscv: gdb: support vector registers for rv64 &
rv32")
inserted these vector CSRs in an ad-hoc, non-standard way in the
riscv-vector.xml. Now we can treat these CSRs no different from
other CSRs.
Signed-off-by: Bin Meng
Reviewed-by: Weiwei Li
Reviewed-by: LIU Zhiwei
---
(no changes since v1)
target/riscv/gdbstub.c | 75 --
1 file changed, 75 deletions(-)
diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
index ef52f41460..6048541606 100644
--- a/target/riscv/gdbstub.c
+++ b/target/riscv/gdbstub.c
@@ -127,40 +127,6 @@ static int riscv_gdb_set_fpu(CPURISCVState *env, uint8_t
*mem_buf, int n)
return 0;
}
-/*
- * Convert register index number passed by GDB to the correspond
- * vector CSR number. Vector CSRs are defined after vector registers
- * in dynamic generated riscv-vector.xml, thus the starting register index
- * of vector CSRs is 32.
- * Return 0 if register index number is out of range.
- */
-static int riscv_gdb_vector_csrno(int num_regs)
-{
-/*
- * The order of vector CSRs in the switch case
- * should match with the order defined in csr_ops[].
- */
-switch (num_regs) {
-case 32:
-return CSR_VSTART;
-case 33:
-return CSR_VXSAT;
-case 34:
-return CSR_VXRM;
-case 35:
-return CSR_VCSR;
-case 36:
-return CSR_VL;
-case 37:
-return CSR_VTYPE;
-case 38:
-return CSR_VLENB;
-default:
-/* Unknown register. */
-return 0;
-}
-}
-
static int riscv_gdb_get_vector(CPURISCVState *env, GByteArray *buf, int n)
{
uint16_t vlenb = env_archcpu(env)->cfg.vlen >> 3;
@@ -174,19 +140,6 @@ static int riscv_gdb_get_vector(CPURISCVState *env,
GByteArray *buf, int n)
return cnt;
}
-int csrno = riscv_gdb_vector_csrno(n);
-
-if (!csrno) {
-return 0;
-}
-
-target_ulong val = 0;
-int result = riscv_csrrw_debug(env, csrno, &val, 0, 0);
-
-if (result == RISCV_EXCP_NONE) {
-return gdb_get_regl(buf, val);
-}
-
return 0;
}
@@ -201,19 +154,6 @@ static int riscv_gdb_set_vector(CPURISCVState *env,
uint8_t *mem_buf, int n)
return vlenb;
}
-int csrno = riscv_gdb_vector_csrno(n);
-
-if (!csrno) {
-return 0;
-}
-
-target_ulong val = ldtul_p(mem_buf);
-int result = riscv_csrrw_debug(env, csrno, NULL, val, -1);
-
-if (result == RISCV_EXCP_NONE) {
-return sizeof(target_ulong);
-}
-
return 0;
}
@@ -361,21 +301,6 @@ static int ricsv_gen_dynamic_vector_xml(CPUState *cs, int
base_reg)
num_regs++;
}
-/* Define vector CSRs */
-const char *vector_csrs[7] = {
-"vstart", "vxsat", "vxrm", "vcsr",
-"vl", "vtype", "vlenb"
-};
-
-for (i = 0; i < 7; i++) {
-g_string_append_printf(s,
- "",
- vector_csrs[i], TARGET_LONG_BITS, base_reg++);
-num_regs++;
-}
-
g_string_append_printf(s, "");
cpu->dyn_vreg_xml = g_string_free(s, false);
--
2.25.1
[PATCH v2 17/18] target/riscv: Drop priv level check in mseccfg predicate()
From: Bin Meng
riscv_csrrw_check() already does the generic privilege level check
hence there is no need to do the specific M-mode access check in
the mseccfg predicate().
With this change debugger can access the mseccfg CSR anytime.
Signed-off-by: Bin Meng
Reviewed-by: Weiwei Li
---
(no changes since v1)
target/riscv/csr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 020c3f524f..785f6f4d45 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -451,7 +451,7 @@ static RISCVException pmp(CPURISCVState *env, int csrno)
static RISCVException epmp(CPURISCVState *env, int csrno)
{
-if (env->priv == PRV_M && riscv_cpu_cfg(env)->epmp) {
+if (riscv_cpu_cfg(env)->epmp) {
return RISCV_EXCP_NONE;
}
--
2.25.1
[PATCH v2 15/18] target/riscv: Allow debugger to access {h, s}stateen CSRs
From: Bin Meng
At present {h,s}stateen CSRs are not reported in the CSR XML
hence gdb cannot access them.
Fix it by adjusting their predicate() routine logic so that the
static config check comes before the run-time check, as well as
adding a debugger check.
Signed-off-by: Bin Meng
Reviewed-by: Weiwei Li
---
(no changes since v1)
target/riscv/csr.c | 22 --
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 15b23b9b5a..a0e70f5ba0 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -337,13 +337,22 @@ static RISCVException hstateen_pred(CPURISCVState *env,
int csrno, int base)
return RISCV_EXCP_ILLEGAL_INST;
}
+RISCVException ret = hmode(env, csrno);
+if (ret != RISCV_EXCP_NONE) {
+return ret;
+}
+
+if (env->debugger) {
+return RISCV_EXCP_NONE;
+}
+
if (env->priv < PRV_M) {
if (!(env->mstateen[csrno - base] & SMSTATEEN_STATEEN)) {
return RISCV_EXCP_ILLEGAL_INST;
}
}
-return hmode(env, csrno);
+return RISCV_EXCP_NONE;
}
static RISCVException hstateen(CPURISCVState *env, int csrno)
@@ -366,6 +375,15 @@ static RISCVException sstateen(CPURISCVState *env, int
csrno)
return RISCV_EXCP_ILLEGAL_INST;
}
+RISCVException ret = smode(env, csrno);
+if (ret != RISCV_EXCP_NONE) {
+return ret;
+}
+
+if (env->debugger) {
+return RISCV_EXCP_NONE;
+}
+
if (env->priv < PRV_M) {
if (!(env->mstateen[index] & SMSTATEEN_STATEEN)) {
return RISCV_EXCP_ILLEGAL_INST;
@@ -378,7 +396,7 @@ static RISCVException sstateen(CPURISCVState *env, int
csrno)
}
}
-return smode(env, csrno);
+return RISCV_EXCP_NONE;
}
/* Checks if PointerMasking registers could be accessed */
--
2.25.1
[PATCH v2 13/18] target/riscv: Allow debugger to access user timer and counter CSRs
From: Bin Meng
At present user timer and counter CSRs are not reported in the
CSR XML hence gdb cannot access them.
Fix it by adding a debugger check in their predicate() routine.
Signed-off-by: Bin Meng
Reviewed-by: Weiwei Li
Reviewed-by: LIU Zhiwei
---
(no changes since v1)
target/riscv/csr.c | 4
1 file changed, 4 insertions(+)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 7284fd8a0d..10ae5df5e6 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -131,6 +131,10 @@ static RISCVException ctr(CPURISCVState *env, int csrno)
skip_ext_pmu_check:
+if (env->debugger) {
+return RISCV_EXCP_NONE;
+}
+
if (env->priv < PRV_M && !get_field(env->mcounteren, ctr_mask)) {
return RISCV_EXCP_ILLEGAL_INST;
}
--
2.25.1
[PATCH v2 14/18] target/riscv: Allow debugger to access seed CSR
From: Bin Meng
At present seed CSR is not reported in the CSR XML hence gdb cannot
access it.
Fix it by adding a debugger check in its predicate() routine.
Signed-off-by: Bin Meng
Reviewed-by: Weiwei Li
Reviewed-by: LIU Zhiwei
---
(no changes since v1)
target/riscv/csr.c | 4
1 file changed, 4 insertions(+)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 10ae5df5e6..15b23b9b5a 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -459,6 +459,10 @@ static RISCVException seed(CPURISCVState *env, int csrno)
}
#if !defined(CONFIG_USER_ONLY)
+if (env->debugger) {
+return RISCV_EXCP_NONE;
+}
+
/*
* With a CSR read-write instruction:
* 1) The seed CSR is always available in machine mode as normal.
--
2.25.1
[PATCH v2 09/18] target/riscv: Simplify getting RISCVCPU pointer from env
Use env_archcpu() to get RISCVCPU pointer from env directly.
Signed-off-by: Bin Meng
Reviewed-by: Weiwei Li
Reviewed-by: LIU Zhiwei
---
(no changes since v1)
target/riscv/csr.c | 36
1 file changed, 12 insertions(+), 24 deletions(-)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index a3e0e5755c..8e827362cc 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -46,8 +46,7 @@ static RISCVException smstateen_acc_ok(CPURISCVState *env,
int index,
uint64_t bit)
{
bool virt = riscv_cpu_virt_enabled(env);
-CPUState *cs = env_cpu(env);
-RISCVCPU *cpu = RISCV_CPU(cs);
+RISCVCPU *cpu = env_archcpu(env);
if (env->priv == PRV_M || !cpu->cfg.ext_smstateen) {
return RISCV_EXCP_NONE;
@@ -90,8 +89,7 @@ static RISCVException fs(CPURISCVState *env, int csrno)
static RISCVException vs(CPURISCVState *env, int csrno)
{
-CPUState *cs = env_cpu(env);
-RISCVCPU *cpu = RISCV_CPU(cs);
+RISCVCPU *cpu = env_archcpu(env);
if (env->misa_ext & RVV ||
cpu->cfg.ext_zve32f || cpu->cfg.ext_zve64f) {
@@ -108,8 +106,7 @@ static RISCVException vs(CPURISCVState *env, int csrno)
static RISCVException ctr(CPURISCVState *env, int csrno)
{
#if !defined(CONFIG_USER_ONLY)
-CPUState *cs = env_cpu(env);
-RISCVCPU *cpu = RISCV_CPU(cs);
+RISCVCPU *cpu = env_archcpu(env);
int ctr_index;
target_ulong ctr_mask;
int base_csrno = CSR_CYCLE;
@@ -166,8 +163,7 @@ static RISCVException ctr32(CPURISCVState *env, int csrno)
#if !defined(CONFIG_USER_ONLY)
static RISCVException mctr(CPURISCVState *env, int csrno)
{
-CPUState *cs = env_cpu(env);
-RISCVCPU *cpu = RISCV_CPU(cs);
+RISCVCPU *cpu = env_archcpu(env);
int ctr_index;
int base_csrno = CSR_MHPMCOUNTER3;
@@ -195,8 +191,7 @@ static RISCVException mctr32(CPURISCVState *env, int csrno)
static RISCVException sscofpmf(CPURISCVState *env, int csrno)
{
-CPUState *cs = env_cpu(env);
-RISCVCPU *cpu = RISCV_CPU(cs);
+RISCVCPU *cpu = env_archcpu(env);
if (!cpu->cfg.ext_sscofpmf) {
return RISCV_EXCP_ILLEGAL_INST;
@@ -321,8 +316,7 @@ static RISCVException umode32(CPURISCVState *env, int csrno)
static RISCVException mstateen(CPURISCVState *env, int csrno)
{
-CPUState *cs = env_cpu(env);
-RISCVCPU *cpu = RISCV_CPU(cs);
+RISCVCPU *cpu = env_archcpu(env);
if (!cpu->cfg.ext_smstateen) {
return RISCV_EXCP_ILLEGAL_INST;
@@ -333,8 +327,7 @@ static RISCVException mstateen(CPURISCVState *env, int
csrno)
static RISCVException hstateen_pred(CPURISCVState *env, int csrno, int base)
{
-CPUState *cs = env_cpu(env);
-RISCVCPU *cpu = RISCV_CPU(cs);
+RISCVCPU *cpu = env_archcpu(env);
if (!cpu->cfg.ext_smstateen) {
return RISCV_EXCP_ILLEGAL_INST;
@@ -363,8 +356,7 @@ static RISCVException sstateen(CPURISCVState *env, int
csrno)
{
bool virt = riscv_cpu_virt_enabled(env);
int index = csrno - CSR_SSTATEEN0;
-CPUState *cs = env_cpu(env);
-RISCVCPU *cpu = RISCV_CPU(cs);
+RISCVCPU *cpu = env_archcpu(env);
if (!cpu->cfg.ext_smstateen) {
return RISCV_EXCP_ILLEGAL_INST;
@@ -918,8 +910,7 @@ static RISCVException read_timeh(CPURISCVState *env, int
csrno,
static RISCVException sstc(CPURISCVState *env, int csrno)
{
-CPUState *cs = env_cpu(env);
-RISCVCPU *cpu = RISCV_CPU(cs);
+RISCVCPU *cpu = env_archcpu(env);
bool hmode_check = false;
if (!cpu->cfg.ext_sstc || !env->rdtime_fn) {
@@ -1152,8 +1143,7 @@ static RISCVException write_ignore(CPURISCVState *env,
int csrno,
static RISCVException read_mvendorid(CPURISCVState *env, int csrno,
target_ulong *val)
{
-CPUState *cs = env_cpu(env);
-RISCVCPU *cpu = RISCV_CPU(cs);
+RISCVCPU *cpu = env_archcpu(env);
*val = cpu->cfg.mvendorid;
return RISCV_EXCP_NONE;
@@ -1162,8 +1152,7 @@ static RISCVException read_mvendorid(CPURISCVState *env,
int csrno,
static RISCVException read_marchid(CPURISCVState *env, int csrno,
target_ulong *val)
{
-CPUState *cs = env_cpu(env);
-RISCVCPU *cpu = RISCV_CPU(cs);
+RISCVCPU *cpu = env_archcpu(env);
*val = cpu->cfg.marchid;
return RISCV_EXCP_NONE;
@@ -1172,8 +1161,7 @@ static RISCVException read_marchid(CPURISCVState *env,
int csrno,
static RISCVException read_mimpid(CPURISCVState *env, int csrno,
target_ulong *val)
{
-CPUState *cs = env_cpu(env);
-RISCVCPU *cpu = RISCV_CPU(cs);
+RISCVCPU *cpu = env_archcpu(env);
*val = cpu->cfg.mimpid;
return RISCV_EXCP_NONE;
--
2.25.1
[PATCH v2 11/18] target/riscv: gdbstub: Turn on debugger mode before calling CSR predicate()
Since commit 94452ac4cf26 ("target/riscv: remove fflags, frm, and fcsr from
riscv-*-fpu.xml")
the 3 FPU CSRs are removed from the XML target decription. The
original intent of that commit was based on the assumption that
the 3 FPU CSRs will show up in the riscv-csr.xml so the ones in
riscv-*-fpu.xml are redundant. But unforuantely that is not true.
As the FPU CSR predicate() has a run-time check on MSTATUS.FS,
at the time when CSR XML is generated MSTATUS.FS is unset, hence
no FPU CSRs will be reported.
The FPU CSR predicate() already considered such a case of being
accessed by a debugger. All we need to do is to turn on debugger
mode before calling predicate().
Signed-off-by: Bin Meng
Reviewed-by: Weiwei Li
Reviewed-by: LIU Zhiwei
---
(no changes since v1)
target/riscv/gdbstub.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
index 294f0ceb1c..ef52f41460 100644
--- a/target/riscv/gdbstub.c
+++ b/target/riscv/gdbstub.c
@@ -280,6 +280,10 @@ static int riscv_gen_dynamic_csr_xml(CPUState *cs, int
base_reg)
int bitsize = 16 << env->misa_mxl_max;
int i;
+#if !defined(CONFIG_USER_ONLY)
+env->debugger = true;
+#endif
+
/* Until gdb knows about 128-bit registers */
if (bitsize > 64) {
bitsize = 64;
@@ -308,6 +312,11 @@ static int riscv_gen_dynamic_csr_xml(CPUState *cs, int
base_reg)
g_string_append_printf(s, "");
cpu->dyn_csr_xml = g_string_free(s, false);
+
+#if !defined(CONFIG_USER_ONLY)
+env->debugger = false;
+#endif
+
return CSR_TABLE_SIZE;
}
--
2.25.1
[PATCH v2 08/18] target/riscv: Simplify {read, write}_pmpcfg() a little bit
Use the register index that has already been calculated in the
pmpcfg_csr_{read,write} call.
Signed-off-by: Bin Meng
Reviewed-by: Weiwei Li
Reviewed-by: LIU Zhiwei
---
(no changes since v1)
target/riscv/csr.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 9264db6110..a3e0e5755c 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -3360,7 +3360,7 @@ static RISCVException read_pmpcfg(CPURISCVState *env, int
csrno,
if (!check_pmp_reg_index(env, reg_index)) {
return RISCV_EXCP_ILLEGAL_INST;
}
-*val = pmpcfg_csr_read(env, csrno - CSR_PMPCFG0);
+*val = pmpcfg_csr_read(env, reg_index);
return RISCV_EXCP_NONE;
}
@@ -3372,7 +3372,7 @@ static RISCVException write_pmpcfg(CPURISCVState *env,
int csrno,
if (!check_pmp_reg_index(env, reg_index)) {
return RISCV_EXCP_ILLEGAL_INST;
}
-pmpcfg_csr_write(env, csrno - CSR_PMPCFG0, val);
+pmpcfg_csr_write(env, reg_index, val);
return RISCV_EXCP_NONE;
}
--
2.25.1
[PATCH v2 07/18] target/riscv: Use 'bool' type for read_only
The read_only variable is currently declared as an 'int', but it
should really be a 'bool'.
Signed-off-by: Bin Meng
Reviewed-by: Weiwei Li
Reviewed-by: LIU Zhiwei
---
(no changes since v1)
target/riscv/csr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 6a82628749..9264db6110 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -3775,7 +3775,7 @@ static inline RISCVException
riscv_csrrw_check(CPURISCVState *env,
RISCVCPU *cpu)
{
/* check privileges and return RISCV_EXCP_ILLEGAL_INST if check fails */
-int read_only = get_field(csrno, 0xC00) == 3;
+bool read_only = get_field(csrno, 0xC00) == 3;
int csr_min_priv = csr_ops[csrno].min_priv_ver;
/* ensure the CSR extension is enabled */
--
2.25.1
[PATCH v2 10/18] target/riscv: Avoid reporting odd-numbered pmpcfgX in the CSR XML for RV64
At present the odd-numbered PMP configuration registers for RV64 are
reported in the CSR XML by QEMU gdbstub. However these registers do
not exist on RV64 so trying to access them from gdb results in 'E14'.
Move the pmpcfgX index check from the actual read/write routine to
the PMP CSR predicate() routine, so that non-existent pmpcfgX won't
be reported in the CSR XML for RV64.
Signed-off-by: Bin Meng
Reviewed-by: Weiwei Li
Reviewed-by: LIU Zhiwei
---
Changes in v2:
- keep the 'RV128 restriction check' todo comment
target/riscv/csr.c | 24 +---
1 file changed, 9 insertions(+), 15 deletions(-)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 8e827362cc..7284fd8a0d 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -412,6 +412,15 @@ static int aia_hmode32(CPURISCVState *env, int csrno)
static RISCVException pmp(CPURISCVState *env, int csrno)
{
if (riscv_cpu_cfg(env)->pmp) {
+if (csrno <= CSR_PMPCFG3) {
+uint32_t reg_index = csrno - CSR_PMPCFG0;
+
+/* TODO: RV128 restriction check */
+if ((reg_index & 1) && (riscv_cpu_mxl(env) == MXL_RV64)) {
+return RISCV_EXCP_ILLEGAL_INST;
+}
+}
+
return RISCV_EXCP_NONE;
}
@@ -3331,23 +3340,11 @@ static RISCVException write_mseccfg(CPURISCVState *env,
int csrno,
return RISCV_EXCP_NONE;
}
-static bool check_pmp_reg_index(CPURISCVState *env, uint32_t reg_index)
-{
-/* TODO: RV128 restriction check */
-if ((reg_index & 1) && (riscv_cpu_mxl(env) == MXL_RV64)) {
-return false;
-}
-return true;
-}
-
static RISCVException read_pmpcfg(CPURISCVState *env, int csrno,
target_ulong *val)
{
uint32_t reg_index = csrno - CSR_PMPCFG0;
-if (!check_pmp_reg_index(env, reg_index)) {
-return RISCV_EXCP_ILLEGAL_INST;
-}
*val = pmpcfg_csr_read(env, reg_index);
return RISCV_EXCP_NONE;
}
@@ -3357,9 +3354,6 @@ static RISCVException write_pmpcfg(CPURISCVState *env,
int csrno,
{
uint32_t reg_index = csrno - CSR_PMPCFG0;
-if (!check_pmp_reg_index(env, reg_index)) {
-return RISCV_EXCP_ILLEGAL_INST;
-}
pmpcfg_csr_write(env, reg_index, val);
return RISCV_EXCP_NONE;
}
--
2.25.1
[PATCH v2 06/18] target/riscv: Coding style fixes in csr.c
Fix various places that violate QEMU coding style:
- correct multi-line comment format
- indent to opening parenthesis
Signed-off-by: Bin Meng
Reviewed-by: Weiwei Li
Reviewed-by: LIU Zhiwei
---
(no changes since v1)
target/riscv/csr.c | 62 --
1 file changed, 32 insertions(+), 30 deletions(-)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index cfd7ffc5c2..6a82628749 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -963,7 +963,7 @@ static RISCVException sstc_32(CPURISCVState *env, int csrno)
}
static RISCVException read_vstimecmp(CPURISCVState *env, int csrno,
-target_ulong *val)
+ target_ulong *val)
{
*val = env->vstimecmp;
@@ -971,7 +971,7 @@ static RISCVException read_vstimecmp(CPURISCVState *env,
int csrno,
}
static RISCVException read_vstimecmph(CPURISCVState *env, int csrno,
-target_ulong *val)
+ target_ulong *val)
{
*val = env->vstimecmp >> 32;
@@ -979,7 +979,7 @@ static RISCVException read_vstimecmph(CPURISCVState *env,
int csrno,
}
static RISCVException write_vstimecmp(CPURISCVState *env, int csrno,
-target_ulong val)
+ target_ulong val)
{
RISCVCPU *cpu = env_archcpu(env);
@@ -996,7 +996,7 @@ static RISCVException write_vstimecmp(CPURISCVState *env,
int csrno,
}
static RISCVException write_vstimecmph(CPURISCVState *env, int csrno,
-target_ulong val)
+ target_ulong val)
{
RISCVCPU *cpu = env_archcpu(env);
@@ -1020,7 +1020,7 @@ static RISCVException read_stimecmp(CPURISCVState *env,
int csrno,
}
static RISCVException read_stimecmph(CPURISCVState *env, int csrno,
-target_ulong *val)
+ target_ulong *val)
{
if (riscv_cpu_virt_enabled(env)) {
*val = env->vstimecmp >> 32;
@@ -1032,7 +1032,7 @@ static RISCVException read_stimecmph(CPURISCVState *env,
int csrno,
}
static RISCVException write_stimecmp(CPURISCVState *env, int csrno,
-target_ulong val)
+ target_ulong val)
{
RISCVCPU *cpu = env_archcpu(env);
@@ -1055,7 +1055,7 @@ static RISCVException write_stimecmp(CPURISCVState *env,
int csrno,
}
static RISCVException write_stimecmph(CPURISCVState *env, int csrno,
-target_ulong val)
+ target_ulong val)
{
RISCVCPU *cpu = env_archcpu(env);
@@ -1342,7 +1342,8 @@ static RISCVException write_misa(CPURISCVState *env, int
csrno,
/* 'E' excludes all other extensions */
if (val & RVE) {
-/* when we support 'E' we can do "val = RVE;" however
+/*
+ * when we support 'E' we can do "val = RVE;" however
* for now we just drop writes if 'E' is present.
*/
return RISCV_EXCP_NONE;
@@ -1361,7 +1362,8 @@ static RISCVException write_misa(CPURISCVState *env, int
csrno,
val &= ~RVD;
}
-/* Suppress 'C' if next instruction is not aligned
+/*
+ * Suppress 'C' if next instruction is not aligned
* TODO: this should check next_pc
*/
if ((val & RVC) && (GETPC() & ~3) != 0) {
@@ -1830,28 +1832,28 @@ static RISCVException write_mscratch(CPURISCVState
*env, int csrno,
}
static RISCVException read_mepc(CPURISCVState *env, int csrno,
- target_ulong *val)
+target_ulong *val)
{
*val = env->mepc;
return RISCV_EXCP_NONE;
}
static RISCVException write_mepc(CPURISCVState *env, int csrno,
- target_ulong val)
+ target_ulong val)
{
env->mepc = val;
return RISCV_EXCP_NONE;
}
static RISCVException read_mcause(CPURISCVState *env, int csrno,
- target_ulong *val)
+ target_ulong *val)
{
*val = env->mcause;
return RISCV_EXCP_NONE;
}
static RISCVException write_mcause(CPURISCVState *env, int csrno,
- target_ulong val)
+ target_ulong val)
{
env->mcause = val;
return RISCV_EXCP_NONE;
@@ -1873,14 +1875,14 @@ static RISCVException write_mtval(CPURISCVState *env,
int csrno,
/* Execution environment configuration setup */
static RISCVException read_menvcfg(CPURISCVState *env, int csrno,
- target_ulong *val)
+ target_ulon
[PATCH v2 05/18] target/riscv: gdbstub: Do not generate CSR XML if Zicsr is disabled
There is no need to generate the CSR XML if the Zicsr extension
is not enabled.
Signed-off-by: Bin Meng
Reviewed-by: Weiwei Li
Reviewed-by: LIU Zhiwei
---
(no changes since v1)
target/riscv/gdbstub.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
index 704f3d6922..294f0ceb1c 100644
--- a/target/riscv/gdbstub.c
+++ b/target/riscv/gdbstub.c
@@ -406,7 +406,10 @@ void riscv_cpu_register_gdb_regs_for_features(CPUState *cs)
g_assert_not_reached();
}
-gdb_register_coprocessor(cs, riscv_gdb_get_csr, riscv_gdb_set_csr,
- riscv_gen_dynamic_csr_xml(cs, cs->gdb_num_regs),
- "riscv-csr.xml", 0);
+if (cpu->cfg.ext_icsr) {
+int base_reg = cs->gdb_num_regs;
+gdb_register_coprocessor(cs, riscv_gdb_get_csr, riscv_gdb_set_csr,
+ riscv_gen_dynamic_csr_xml(cs, base_reg),
+ "riscv-csr.xml", 0);
+}
}
--
2.25.1
[PATCH v2 04/18] target/riscv: gdbstub: Minor change for better readability
Use a variable 'base_reg' to represent cs->gdb_num_regs so that
the call to ricsv_gen_dynamic_vector_xml() can be placed in one
single line for better readability.
Signed-off-by: Bin Meng
Reviewed-by: Weiwei Li
Reviewed-by: LIU Zhiwei
---
(no changes since v1)
target/riscv/gdbstub.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
index e57372db38..704f3d6922 100644
--- a/target/riscv/gdbstub.c
+++ b/target/riscv/gdbstub.c
@@ -385,9 +385,9 @@ void riscv_cpu_register_gdb_regs_for_features(CPUState *cs)
32, "riscv-32bit-fpu.xml", 0);
}
if (env->misa_ext & RVV) {
+int base_reg = cs->gdb_num_regs;
gdb_register_coprocessor(cs, riscv_gdb_get_vector,
riscv_gdb_set_vector,
- ricsv_gen_dynamic_vector_xml(cs,
-
cs->gdb_num_regs),
+ ricsv_gen_dynamic_vector_xml(cs, base_reg),
"riscv-vector.xml", 0);
}
switch (env->misa_mxl_max) {
--
2.25.1
[PATCH v2 03/18] target/riscv: Use g_assert() for the predicate() NULL check
At present riscv_csrrw_check() checks the CSR predicate() against
NULL and throws RISCV_EXCP_ILLEGAL_INST if it is NULL. But this is
a pure software check, and has nothing to do with the emulation of
the hardware behavior, thus it is inappropriate to return illegal
instruction exception when software forgets to install the hook.
Change to use g_assert() instead.
Signed-off-by: Bin Meng
---
Changes in v2:
- new patch: Use assert() for the predicate() NULL check
target/riscv/csr.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 4cc2c6370f..cfd7ffc5c2 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -3786,11 +3786,6 @@ static inline RISCVException
riscv_csrrw_check(CPURISCVState *env,
return RISCV_EXCP_ILLEGAL_INST;
}
-/* check predicate */
-if (!csr_ops[csrno].predicate) {
-return RISCV_EXCP_ILLEGAL_INST;
-}
-
/* read / write check */
if (write_mask && read_only) {
return RISCV_EXCP_ILLEGAL_INST;
@@ -3803,6 +3798,7 @@ static inline RISCVException
riscv_csrrw_check(CPURISCVState *env,
* illegal instruction exception should be triggered instead of virtual
* instruction exception. Hence this comes after the read / write check.
*/
+g_assert(csr_ops[csrno].predicate != NULL);
RISCVException ret = csr_ops[csrno].predicate(env, csrno);
if (ret != RISCV_EXCP_NONE) {
return ret;
--
2.25.1
[PATCH v2 02/18] target/riscv: Add some comments to clarify the priority policy of riscv_csrrw_check()
The priority policy of riscv_csrrw_check() was once adjusted in
commit eacaf4401956 ("target/riscv: Fix priority of csr related check in
riscv_csrrw_check")
whose commit message says the CSR existence check should come before
the access control check, but the code changes did not agree with
the commit message, that the predicate() check actually came after
the read / write check.
In fact this was intentional. Add some comments there so that people
won't bother trying to change it without a solid reason.
Signed-off-by: Bin Meng
---
Changes in v2:
- Keep the original priority policy, instead add some comments for clarification
target/riscv/csr.c | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 75a540bfcb..4cc2c6370f 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -3776,11 +3776,12 @@ static inline RISCVException
riscv_csrrw_check(CPURISCVState *env,
int read_only = get_field(csrno, 0xC00) == 3;
int csr_min_priv = csr_ops[csrno].min_priv_ver;
-/* ensure the CSR extension is enabled. */
+/* ensure the CSR extension is enabled */
if (!cpu->cfg.ext_icsr) {
return RISCV_EXCP_ILLEGAL_INST;
}
+/* privileged spec version check */
if (env->priv_ver < csr_min_priv) {
return RISCV_EXCP_ILLEGAL_INST;
}
@@ -3790,10 +3791,18 @@ static inline RISCVException
riscv_csrrw_check(CPURISCVState *env,
return RISCV_EXCP_ILLEGAL_INST;
}
+/* read / write check */
if (write_mask && read_only) {
return RISCV_EXCP_ILLEGAL_INST;
}
+/*
+ * The predicate() not only does existence check but also does some
+ * access control check which triggers for example virtual instruction
+ * exception in some cases. When writing read-only CSRs in those cases
+ * illegal instruction exception should be triggered instead of virtual
+ * instruction exception. Hence this comes after the read / write check.
+ */
RISCVException ret = csr_ops[csrno].predicate(env, csrno);
if (ret != RISCV_EXCP_NONE) {
return ret;
--
2.25.1
[PATCH v2 01/18] target/riscv: gdbstub: Check priv spec version before reporting CSR
The gdbstub CSR XML is dynamically generated according to the result
of the CSR predicate() result. This has been working fine until
commit 7100fe6c2441 ("target/riscv: Enable privileged spec version 1.12")
introduced the privilege spec version check in riscv_csrrw_check().
When debugging the 'sifive_u' machine whose priv spec is at 1.10,
gdbstub reports priv spec 1.12 CSRs like menvcfg in the XML, hence
we see "remote failure reply 'E14'" message when examining all CSRs
via "info register system" from gdb.
Add the priv spec version check in the CSR XML generation logic to
fix this issue.
Fixes: 7100fe6c2441 ("target/riscv: Enable privileged spec version 1.12")
Signed-off-by: Bin Meng
Reviewed-by: Weiwei Li
Reviewed-by: LIU Zhiwei
---
(no changes since v1)
target/riscv/gdbstub.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
index 6e7bbdbd5e..e57372db38 100644
--- a/target/riscv/gdbstub.c
+++ b/target/riscv/gdbstub.c
@@ -290,6 +290,9 @@ static int riscv_gen_dynamic_csr_xml(CPUState *cs, int
base_reg)
g_string_append_printf(s, "");
for (i = 0; i < CSR_TABLE_SIZE; i++) {
+if (env->priv_ver < csr_ops[i].min_priv_ver) {
+continue;
+}
predicate = csr_ops[i].predicate;
if (predicate && (predicate(env, i) == RISCV_EXCP_NONE)) {
if (csr_ops[i].name) {
--
2.25.1
