Re: [PATCH v1 1/1] aspeed/hace: Support AST1030 HACE
On 5/3/22 04:27, Steven Lee wrote:
Per ast1030_v7.pdf, AST1030 HACE engine is identical to AST2600's HACE
engine.
Signed-off-by: Steven Lee
Reviewed-by: Cédric Le Goater
Thanks
C.
---
hw/misc/aspeed_hace.c | 20
include/hw/misc/aspeed_hace.h | 2 ++
2 files changed, 22 insertions(+)
diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
index 10f00e65f4..91f3c0b208 100644
--- a/hw/misc/aspeed_hace.c
+++ b/hw/misc/aspeed_hace.c
@@ -378,11 +378,31 @@ static const TypeInfo aspeed_ast2600_hace_info = {
.class_init = aspeed_ast2600_hace_class_init,
};
+static void aspeed_ast1030_hace_class_init(ObjectClass *klass, void *data)
+{
+DeviceClass *dc = DEVICE_CLASS(klass);
+AspeedHACEClass *ahc = ASPEED_HACE_CLASS(klass);
+
+dc->desc = "AST1030 Hash and Crypto Engine";
+
+ahc->src_mask = 0x7FFF;
+ahc->dest_mask = 0x7FF8;
+ahc->key_mask = 0x7FF8;
+ahc->hash_mask = 0x00147FFF;
+}
+
+static const TypeInfo aspeed_ast1030_hace_info = {
+.name = TYPE_ASPEED_AST1030_HACE,
+.parent = TYPE_ASPEED_HACE,
+.class_init = aspeed_ast1030_hace_class_init,
+};
+
static void aspeed_hace_register_types(void)
{
type_register_static(_ast2400_hace_info);
type_register_static(_ast2500_hace_info);
type_register_static(_ast2600_hace_info);
+type_register_static(_ast1030_hace_info);
type_register_static(_hace_info);
}
diff --git a/include/hw/misc/aspeed_hace.h b/include/hw/misc/aspeed_hace.h
index 94d5ada95f..e9d3563a05 100644
--- a/include/hw/misc/aspeed_hace.h
+++ b/include/hw/misc/aspeed_hace.h
@@ -15,6 +15,8 @@
#define TYPE_ASPEED_AST2400_HACE TYPE_ASPEED_HACE "-ast2400"
#define TYPE_ASPEED_AST2500_HACE TYPE_ASPEED_HACE "-ast2500"
#define TYPE_ASPEED_AST2600_HACE TYPE_ASPEED_HACE "-ast2600"
+#define TYPE_ASPEED_AST1030_HACE TYPE_ASPEED_HACE "-ast1030"
+
OBJECT_DECLARE_TYPE(AspeedHACEState, AspeedHACEClass, ASPEED_HACE)
#define ASPEED_HACE_NR_REGS (0x64 >> 2)
Re: [PATCH v2 1/2] hw/core: Sync uboot_image.h from U-Boot v2022.01
On Thu, Apr 28, 2022 at 4:43 PM Bin Meng wrote: > > On Fri, Apr 22, 2022 at 11:00 AM Bin Meng wrote: > > > > +Richard > > > > On Wed, Apr 20, 2022 at 4:16 PM Bin Meng wrote: > > > > > > On Tue, Apr 12, 2022 at 9:11 AM Bin Meng wrote: > > > > > > > > On Thu, Mar 24, 2022 at 9:48 PM Bin Meng wrote: > > > > > > > > > > From: Bin Meng > > > > > > > > > > Sync uboot_image.h from upstream U-Boot v2022.01 release [1]. > > > > > > > > > > [1] > > > > > https://source.denx.de/u-boot/u-boot/-/blob/v2022.01/include/image.h > > > > > > > > > > Signed-off-by: Bin Meng > > > > > --- > > > > > > > > > > (no changes since v1) > > > > > > > > > > hw/core/uboot_image.h | 213 > > > > > -- > > > > > 1 file changed, 142 insertions(+), 71 deletions(-) > > > > > > > > > > > > > Ping? > > > > > > Ping? > > > > Richard, is that you to pick up this series? > > > > Ping? Ping? Can you please indicate who is the right person to pick up this series? Thanks. Regards, Bin
Re: [PATCH 0/9] 9pfs: Add 9pfs support for Windows host
On Tue, Apr 26, 2022 at 9:41 AM Bin Meng wrote: > > +Mark > > > On Mon, Apr 25, 2022 at 10:27 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' backend driver is updated only to build sucessuflly > > in a Windows build. Funtionalities are not tested as test > > cases do not run on Windows at present. > > > > Example command line to test: > > > > "-fsdev local,path=c:\msys64,security_model=mapped,id=p9 -device > > virtio-9p-pci,fsdev=p9,mount_tag=p9fs" > > Any comments to v1 before we send a v2 which will be rebased on master? Regards, Bin
[PATCH v1 0/1] aspeed/hace: Support AST1030 HACE
Per ast1030_v7.pdf, AST1030 HACE engine is identical to AST2600's HACE engine. Please help to review. Thanks, Steven Based-on: 20220426021120.28255-3-steven_...@aspeedtech.com ([v6,2/3] aspeed/hace: Support AST2600 HACE) Steven Lee (1): aspeed/hace: Support AST1030 HACE hw/misc/aspeed_hace.c | 20 include/hw/misc/aspeed_hace.h | 2 ++ 2 files changed, 22 insertions(+) -- 2.17.1
[PATCH v1 1/1] aspeed/hace: Support AST1030 HACE
Per ast1030_v7.pdf, AST1030 HACE engine is identical to AST2600's HACE
engine.
Signed-off-by: Steven Lee
---
hw/misc/aspeed_hace.c | 20
include/hw/misc/aspeed_hace.h | 2 ++
2 files changed, 22 insertions(+)
diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
index 10f00e65f4..91f3c0b208 100644
--- a/hw/misc/aspeed_hace.c
+++ b/hw/misc/aspeed_hace.c
@@ -378,11 +378,31 @@ static const TypeInfo aspeed_ast2600_hace_info = {
.class_init = aspeed_ast2600_hace_class_init,
};
+static void aspeed_ast1030_hace_class_init(ObjectClass *klass, void *data)
+{
+DeviceClass *dc = DEVICE_CLASS(klass);
+AspeedHACEClass *ahc = ASPEED_HACE_CLASS(klass);
+
+dc->desc = "AST1030 Hash and Crypto Engine";
+
+ahc->src_mask = 0x7FFF;
+ahc->dest_mask = 0x7FF8;
+ahc->key_mask = 0x7FF8;
+ahc->hash_mask = 0x00147FFF;
+}
+
+static const TypeInfo aspeed_ast1030_hace_info = {
+.name = TYPE_ASPEED_AST1030_HACE,
+.parent = TYPE_ASPEED_HACE,
+.class_init = aspeed_ast1030_hace_class_init,
+};
+
static void aspeed_hace_register_types(void)
{
type_register_static(_ast2400_hace_info);
type_register_static(_ast2500_hace_info);
type_register_static(_ast2600_hace_info);
+type_register_static(_ast1030_hace_info);
type_register_static(_hace_info);
}
diff --git a/include/hw/misc/aspeed_hace.h b/include/hw/misc/aspeed_hace.h
index 94d5ada95f..e9d3563a05 100644
--- a/include/hw/misc/aspeed_hace.h
+++ b/include/hw/misc/aspeed_hace.h
@@ -15,6 +15,8 @@
#define TYPE_ASPEED_AST2400_HACE TYPE_ASPEED_HACE "-ast2400"
#define TYPE_ASPEED_AST2500_HACE TYPE_ASPEED_HACE "-ast2500"
#define TYPE_ASPEED_AST2600_HACE TYPE_ASPEED_HACE "-ast2600"
+#define TYPE_ASPEED_AST1030_HACE TYPE_ASPEED_HACE "-ast1030"
+
OBJECT_DECLARE_TYPE(AspeedHACEState, AspeedHACEClass, ASPEED_HACE)
#define ASPEED_HACE_NR_REGS (0x64 >> 2)
--
2.17.1
Re: [PULL 06/11] QIOChannelSocket: Implement io_writev zero copy flag & io_flush for CONFIG_LINUX
On Mon, May 02, 2022 at 09:12:53PM -0300, Leonardo Bras Soares Passos wrote: > Hello Peter, > > On Mon, May 2, 2022 at 8:52 PM Peter Xu wrote: > > > > Leo, > > > > On Fri, Apr 29, 2022 at 11:40:44PM -0300, Leonardo Bras Soares Passos wrote: > > > Does anyone else have any further suggestions, or know how this kind of > > > issue > > > is generally solved in qemu? > > > > I've no solid idea why it can't see MSG_ZEROCOPY defined in the specific > > environment, but when I was looking at bits/socket.h I saw this: > > > > #ifndef _SYS_SOCKET_H > > # error "Never include directly; use > > instead." > > #endif > > > > Maybe worth a shot to do a replacement in all cases? > > > > Sure, no problem with this, I will update for v11. > (Or should I send a different patch since Dave has already merged in his > tree?) > > But it should not interfere in MSG_ZEROCOPY definition: > > > > I tracked down how the test happened, downloaded the same docker image > > > from the > > > tests(opensuse-leap-15.2), and took a look at the filesystem for the > > > MSG_ZEROCOPY define, which I could not find anywhere. > > By this, I mean I did a 'grep MSG_ZEROCOPY -r /' and could not find anything, > so > it's probably not defined anywhere in the fs. What you described gives me the feeling that the distro seems to have had mismatched versions of asm-generic/socket.h (who should define SO_ZEROCOPY), and bits/socket.h (who should define MSG_ZEROCOPY). Let's first replace it with sys/socket.h, then one trick you could consider play with (even if any env could have broken headers) that I thought of, is you can put your code into: #if defined(MSG_ZEROCOPY) && defined(SO_ZEROCOPY) ... #endif Blocks. Just to avoid assuming CONFIG_LINUX will be the same. -- Peter Xu
Re: [PATCH] hw/dma: Add Xilinx AXI CDMA
On Mon, May 2, 2022 at 6:29 PM Edgar E. Iglesias
wrote:
> On Thu, Apr 28, 2022 at 5:43 PM wrote:
>
>> From: Frank Chang
>>
>> Add Xilinx AXI CDMA model, which follows
>> AXI Central Direct Memory Access v4.1 spec:
>> https://docs.xilinx.com/v/u/en-US/pg034-axi-cdma
>>
>> Supports both Simple DMA and Scatter Gather modes.
>>
>
> Hi Frank,
>
> Thanks for modeling this! I have a couple of questions.
>
Hi Edgar,
Thanks for reviewing.
>
> Do you plan to submit a machine that uses this DMA?
>
Currently, Xilinx CDMA is used in our internal platform only, which is not
upstream.
Do you have any suggestions for the existing machine that I can add Xilinx
CDMA to?
Or perhaps, ARM virt machine?
>
> The CDMA has a 32-bit AXI4-Lite port for register accesses (see page 6 and
> 8 in the spec you referenced), so axicdma_ops.impl.max should be 4 and you
> shouldn't need the read/write q versions.
>
Okay, that's something I was not aware of.
However, I have a question regarding the 64-bit address space.
For 64-bit address space, i.e. xlnx,addrwidth = 64.
The CDMA spec says that:
"TAILDESC_PNTR[_MSB] register causes the AXI CDMA SG Engine
to start fetching descriptors starting from the CURDESC_PNTR register
value."
It seems that DMA will start the transfer if either TAILDESC_PNTR or
TAILDESC_PNTR_MSB is written.
Then how can we guarantee that the full 64-bit address pointer is written
before the DMA transfer is started if we can't write both TAILDESC_PNTR and
TAILDESC_PNTR_MSB
at the same time?
I'm also awarded that Xilinx CDMA Linux driver also has separate 32-bit
writes for a 64-bit address.
But wouldn't that cause, e.g. dmatest to be failed?
Regards,
Frank Chang
>
> Best regards,
> Edgar
>
>
>
>>
>> Signed-off-by: Frank Chang
>> Reviewed-by: Jim Shu
>> ---
>> hw/dma/meson.build | 2 +-
>> hw/dma/xilinx_axicdma.c | 792
>> include/hw/dma/xilinx_axicdma.h | 72 +++
>> 3 files changed, 865 insertions(+), 1 deletion(-)
>> create mode 100644 hw/dma/xilinx_axicdma.c
>> create mode 100644 include/hw/dma/xilinx_axicdma.h
>>
>> diff --git a/hw/dma/meson.build b/hw/dma/meson.build
>> index f3f0661bc3..85edf80b82 100644
>> --- a/hw/dma/meson.build
>> +++ b/hw/dma/meson.build
>> @@ -3,7 +3,7 @@ softmmu_ss.add(when: 'CONFIG_PL080', if_true:
>> files('pl080.c'))
>> softmmu_ss.add(when: 'CONFIG_PL330', if_true: files('pl330.c'))
>> softmmu_ss.add(when: 'CONFIG_I82374', if_true: files('i82374.c'))
>> softmmu_ss.add(when: 'CONFIG_I8257', if_true: files('i8257.c'))
>> -softmmu_ss.add(when: 'CONFIG_XILINX_AXI', if_true:
>> files('xilinx_axidma.c'))
>> +softmmu_ss.add(when: 'CONFIG_XILINX_AXI', if_true:
>> files('xilinx_axidma.c', 'xilinx_axicdma.c'))
>> softmmu_ss.add(when: 'CONFIG_ZYNQ_DEVCFG', if_true:
>> files('xlnx-zynq-devcfg.c'))
>> softmmu_ss.add(when: 'CONFIG_ETRAXFS', if_true: files('etraxfs_dma.c'))
>> softmmu_ss.add(when: 'CONFIG_STP2000', if_true: files('sparc32_dma.c'))
>> diff --git a/hw/dma/xilinx_axicdma.c b/hw/dma/xilinx_axicdma.c
>> new file mode 100644
>> index 00..50aec3ec45
>> --- /dev/null
>> +++ b/hw/dma/xilinx_axicdma.c
>> @@ -0,0 +1,792 @@
>> +/*
>> + * QEMU model of Xilinx AXI-CDMA block.
>> + *
>> + * Copyright (c) 2022 Frank Chang .
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining
>> a copy
>> + * of this software and associated documentation files (the "Software"),
>> to deal
>> + * in the Software without restriction, including without limitation the
>> rights
>> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or
>> sell
>> + * copies of the Software, and to permit persons to whom the Software is
>> + * furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice shall be
>> included in
>> + * all copies or substantial portions of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>> EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>> MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT
>> SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
>> OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
>> ARISING FROM,
>> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
>> DEALINGS IN
>> + * THE SOFTWARE.
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "hw/hw.h"
>> +#include "hw/irq.h"
>> +#include "hw/ptimer.h"
>> +#include "hw/qdev-properties.h"
>> +#include "hw/sysbus.h"
>> +#include "qapi/error.h"
>> +#include "qemu/log.h"
>> +#include "qemu/module.h"
>> +#include "qemu/timer.h"
>> +#include "qom/object.h"
>> +#include "sysemu/dma.h"
>> +#include "hw/dma/xilinx_axicdma.h"
>> +
>> +#define R_CDMACR0x00
>> +#define R_CDMASR0x04
>> +#define R_CURDESC 0x08
>> +#define
Re: [PULL 06/11] QIOChannelSocket: Implement io_writev zero copy flag & io_flush for CONFIG_LINUX
Hello Peter, On Mon, May 2, 2022 at 8:52 PM Peter Xu wrote: > > Leo, > > On Fri, Apr 29, 2022 at 11:40:44PM -0300, Leonardo Bras Soares Passos wrote: > > Does anyone else have any further suggestions, or know how this kind of > > issue > > is generally solved in qemu? > > I've no solid idea why it can't see MSG_ZEROCOPY defined in the specific > environment, but when I was looking at bits/socket.h I saw this: > > #ifndef _SYS_SOCKET_H > # error "Never include directly; use instead." > #endif > > Maybe worth a shot to do a replacement in all cases? > Sure, no problem with this, I will update for v11. (Or should I send a different patch since Dave has already merged in his tree?) But it should not interfere in MSG_ZEROCOPY definition: > > I tracked down how the test happened, downloaded the same docker image from > > the > > tests(opensuse-leap-15.2), and took a look at the filesystem for the > > MSG_ZEROCOPY define, which I could not find anywhere. By this, I mean I did a 'grep MSG_ZEROCOPY -r /' and could not find anything, so it's probably not defined anywhere in the fs. > -- > Peter Xu > Thanks Peter! Best regards, Leo
Re: [PATCH] hw/openrisc: use right OMPIC size variable
On 5/2/22 16:28, Jason A. Donenfeld wrote: This appears to be a copy and paste error. The UART size was used instead of the much smaller OMPIC size. Signed-off-by: Jason A. Donenfeld --- hw/openrisc/openrisc_sim.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Richard Henderson r~
Re: [PATCH] target/openrisc: implement shutdown and reset helpers
On 5/2/22 15:52, Jason A. Donenfeld wrote: OpenRISC defines various nop instructions in or1k as meaning shutdown or reset. Implement these in TCG. This has been tested with Linux and confirmed to work. No, OpenRISC does not define various nop instructions, etc. OpenRISC defines a Power Management Register to handle doze and suspend; there is no specification for shutdown or reset. See https://openrisc.io/architecture r~
Re: [PULL 06/11] QIOChannelSocket: Implement io_writev zero copy flag & io_flush for CONFIG_LINUX
Leo, On Fri, Apr 29, 2022 at 11:40:44PM -0300, Leonardo Bras Soares Passos wrote: > Does anyone else have any further suggestions, or know how this kind of issue > is generally solved in qemu? I've no solid idea why it can't see MSG_ZEROCOPY defined in the specific environment, but when I was looking at bits/socket.h I saw this: #ifndef _SYS_SOCKET_H # error "Never include directly; use instead." #endif Maybe worth a shot to do a replacement in all cases? -- Peter Xu
[PATCH] hw/openrisc: use right OMPIC size variable
This appears to be a copy and paste error. The UART size was used
instead of the much smaller OMPIC size.
Signed-off-by: Jason A. Donenfeld
---
hw/openrisc/openrisc_sim.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/openrisc/openrisc_sim.c b/hw/openrisc/openrisc_sim.c
index 99b14940f4..bf434e9737 100644
--- a/hw/openrisc/openrisc_sim.c
+++ b/hw/openrisc/openrisc_sim.c
@@ -410,7 +410,7 @@ static void openrisc_sim_init(MachineState *machine)
if (smp_cpus > 1) {
openrisc_sim_ompic_init(state, or1ksim_memmap[OR1KSIM_OMPIC].base,
-or1ksim_memmap[OR1KSIM_UART].size,
+or1ksim_memmap[OR1KSIM_OMPIC].size,
smp_cpus, cpus, OR1KSIM_OMPIC_IRQ);
}
--
2.35.1
[PATCH] hw/openrisc: support 4 serial ports in or1ksim
The 8250 serial controller supports 4 serial ports, so wire them all up,
so that we can have more than one basic I/O channel.
Cc: Stafford Horne
Signed-off-by: Jason A. Donenfeld
---
hw/openrisc/openrisc_sim.c | 21 +++--
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/hw/openrisc/openrisc_sim.c b/hw/openrisc/openrisc_sim.c
index bf434e9737..fe07db1cc5 100644
--- a/hw/openrisc/openrisc_sim.c
+++ b/hw/openrisc/openrisc_sim.c
@@ -71,6 +71,10 @@ enum {
OR1KSIM_ETHOC_IRQ = 4,
};
+enum {
+OR1KSIM_UART_COUNT = 4
+};
+
static const struct MemmapEntry {
hwaddr base;
hwaddr size;
@@ -239,11 +243,13 @@ static void openrisc_sim_ompic_init(Or1ksimState *state,
hwaddr base,
static void openrisc_sim_serial_init(Or1ksimState *state, hwaddr base,
hwaddr size, int num_cpus,
- OpenRISCCPU *cpus[], int irq_pin)
+ OpenRISCCPU *cpus[], int irq_pin,
+ int uart_idx)
{
void *fdt = state->fdt;
char *nodename;
qemu_irq serial_irq;
+char alias[sizeof("uart0")];
int i;
if (num_cpus > 1) {
@@ -258,7 +264,7 @@ static void openrisc_sim_serial_init(Or1ksimState *state,
hwaddr base,
serial_irq = get_cpu_irq(cpus, 0, irq_pin);
}
serial_mm_init(get_system_memory(), base, 0, serial_irq, 115200,
- serial_hd(0), DEVICE_NATIVE_ENDIAN);
+ serial_hd(OR1KSIM_UART_COUNT - uart_idx - 1),
DEVICE_NATIVE_ENDIAN);
/* Add device tree node for serial. */
nodename = g_strdup_printf("/serial@%" HWADDR_PRIx, base);
@@ -271,7 +277,8 @@ static void openrisc_sim_serial_init(Or1ksimState *state,
hwaddr base,
/* The /chosen node is created during fdt creation. */
qemu_fdt_setprop_string(fdt, "/chosen", "stdout-path", nodename);
-qemu_fdt_setprop_string(fdt, "/aliases", "uart0", nodename);
+snprintf(alias, sizeof(alias), "uart%d", uart_idx);
+qemu_fdt_setprop_string(fdt, "/aliases", alias, nodename);
g_free(nodename);
}
@@ -414,9 +421,11 @@ static void openrisc_sim_init(MachineState *machine)
smp_cpus, cpus, OR1KSIM_OMPIC_IRQ);
}
-openrisc_sim_serial_init(state, or1ksim_memmap[OR1KSIM_UART].base,
- or1ksim_memmap[OR1KSIM_UART].size, smp_cpus, cpus,
- OR1KSIM_UART_IRQ);
+for (n = 0; n < OR1KSIM_UART_COUNT; ++n)
+openrisc_sim_serial_init(state, or1ksim_memmap[OR1KSIM_UART].base +
+or1ksim_memmap[OR1KSIM_UART].size
* n,
+ or1ksim_memmap[OR1KSIM_UART].size,
smp_cpus, cpus,
+ OR1KSIM_UART_IRQ, n);
load_addr = openrisc_load_kernel(ram_size, kernel_filename);
if (load_addr > 0) {
--
2.35.1
[PATCH] target/openrisc: implement shutdown and reset helpers
OpenRISC defines various nop instructions in or1k as meaning shutdown or
reset. Implement these in TCG. This has been tested with Linux and
confirmed to work.
Cc: Stafford Horne
Signed-off-by: Jason A. Donenfeld
---
target/openrisc/helper.h | 1 +
target/openrisc/sys_helper.c | 18 ++
target/openrisc/translate.c | 1 +
3 files changed, 20 insertions(+)
diff --git a/target/openrisc/helper.h b/target/openrisc/helper.h
index d847814a28..ea3557b3f9 100644
--- a/target/openrisc/helper.h
+++ b/target/openrisc/helper.h
@@ -64,3 +64,4 @@ DEF_HELPER_FLAGS_1(rfe, 0, void, env)
/* sys */
DEF_HELPER_FLAGS_3(mtspr, 0, void, env, tl, tl)
DEF_HELPER_FLAGS_3(mfspr, TCG_CALL_NO_WG, tl, env, tl, tl)
+DEF_HELPER_1(nop, void, i32)
diff --git a/target/openrisc/sys_helper.c b/target/openrisc/sys_helper.c
index 48674231e7..f6249896fb 100644
--- a/target/openrisc/sys_helper.c
+++ b/target/openrisc/sys_helper.c
@@ -19,6 +19,7 @@
*/
#include "qemu/osdep.h"
+#include "sysemu/runstate.h"
#include "cpu.h"
#include "exec/exec-all.h"
#include "exec/helper-proto.h"
@@ -314,3 +315,20 @@ target_ulong HELPER(mfspr)(CPUOpenRISCState *env,
target_ulong rd,
/* for rd is passed in, if rd unchanged, just keep it back. */
return rd;
}
+
+void HELPER(nop)(uint32_t arg)
+{
+#ifndef CONFIG_USER_ONLY
+ switch (arg) {
+ case 0x001: /* NOP_EXIT */
+ case 0x00c: /* NOP_EXIT_SILENT */
+ qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
+ break;
+ case 0x00d: /* NOP_RESET */
+ qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
+ break;
+ default:
+ break;
+ }
+#endif
+}
diff --git a/target/openrisc/translate.c b/target/openrisc/translate.c
index 7b8ad43d5f..2e4f3759d4 100644
--- a/target/openrisc/translate.c
+++ b/target/openrisc/translate.c
@@ -780,6 +780,7 @@ static bool trans_l_sh(DisasContext *dc, arg_store *a)
static bool trans_l_nop(DisasContext *dc, arg_l_nop *a)
{
+gen_helper_nop(cpu_R(dc, a->k));
return true;
}
--
2.35.1
Re: [PATCH v2 20/21] target/ppc: Add unused msr bits FIELDs
On 5/2/22 07:39, Víctor Colombo wrote: Add FIELDs macros for msr bits that had an unused msr_* before. Signed-off-by: Víctor Colombo Acked-by: Richard Henderson r~
Re: [PATCH v2 17/21] target/ppc: Remove msr_fe0 and msr_fe1 macros
On 5/2/22 07:39, Víctor Colombo wrote:
msr_fe0 and msr_fe1 macros hide the usage of env->msr, which is a bad
behavior. Substitute it with FIELD_EX64 calls that explicitly use
env->msr as a parameter.
Suggested-by: Richard Henderson
Signed-off-by: Víctor Colombo
Reviewed-by: Richard Henderson
-if ((msr_fe0 == 0 && msr_fe1 == 0) ||
+if ((!FIELD_EX64(env->msr, MSR, FE0) &&
+!FIELD_EX64(env->msr, MSR, FE1)) ||
!FIELD_EX64(env->msr, MSR, FP)) {
This has 6 repetitions. Perhaps extract to a helper for conciseness?
r~
Re: [PATCH v2 18/21] target/ppc: Remove msr_ts macro
On 5/2/22 07:39, Víctor Colombo wrote: msr_ts macro hides the usage of env->msr, which is a bad behavior. Substitute it with FIELD_EX64 calls that explicitly use env->msr as a parameter. Suggested-by: Richard Henderson Signed-off-by: Víctor Colombo Reviewed-by: Richard Henderson r~
Re: [PATCH v2 12/21] target/ppc: Remove msr_fp macro
On 5/2/22 07:39, Víctor Colombo wrote: msr_fp macro hides the usage of env->msr, which is a bad behavior Substitute it with FIELD_EX64 calls that explicitly use env->msr as a parameter. Suggested-by: Richard Henderson Signed-off-by: Víctor Colombo Reviewed-by: Richard Henderson r~
Re: [PATCH v2 13/21] target/ppc: Remove msr_me macro
On 5/2/22 14:38, Richard Henderson wrote:
On 5/2/22 07:39, Víctor Colombo wrote:
msr_me macro hides the usage of env->msr, which is a bad behavior
Substitute it with FIELD_EX64 calls that explicitly use env->msr
as a parameter.
Suggested-by: Richard Henderson
Signed-off-by: Víctor Colombo
---
v2: Remove M_MSR_CM and use FIELD_EX64 instead
Signed-off-by: Víctor Colombo
---
target/ppc/cpu.h | 2 +-
target/ppc/mmu_common.c | 2 +-
target/ppc/mmu_helper.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 294c61c9dc..8e652691cf 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -354,6 +354,7 @@ typedef enum {
#define MSR_RI 1 /* Recoverable interrupt 1
*/
#define MSR_LE 0 /* Little-endian mode 1 hflags
*/
+FIELD(MSR, CM, MSR_CM, 1)
Patch title is wrong.
Otherwise,
Reviewed-by: Richard Henderson
r~
Re: [PATCH v2 10/21] target/ppc: Remove msr_me macro
On 5/2/22 07:39, Víctor Colombo wrote: msr_me macro hides the usage of env->msr, which is a bad behavior Substitute it with FIELD_EX64 calls that explicitly use env->msr as a parameter. Suggested-by: Richard Henderson Signed-off-by: Víctor Colombo Reviewed-by: Richard Henderson r~
Re: [PATCH v2 09/21] target/ppc: Remove msr_pow macro
On 5/2/22 07:39, Víctor Colombo wrote: msr_pow macro hides the usage of env->msr, which is a bad behavior Substitute it with FIELD_EX64 calls that explicitly use env->msr as a parameter. Suggested-by: Richard Henderson Signed-off-by: Víctor Colombo Reviewed-by: Richard Henderson r~
Re: [PATCH v2 06/21] target/ppc: Remove msr_ile macro
On 5/2/22 07:39, Víctor Colombo wrote: msr_ile macro hides the usage of env->msr, which is a bad behavior Substitute it with FIELD_EX64 calls that explicitly use env->msr as a parameter. Suggested-by: Richard Henderson Signed-off-by: Víctor Colombo Reviewed-by: Richard Henderson r~
Re: [PATCH v2 04/21] target/ppc: Remove msr_le macro
On 5/2/22 07:39, Víctor Colombo wrote: msr_le macro hides the usage of env->msr, which is a bad behavior Substitute it with FIELD_EX64 calls that explicitly use env->msr as a parameter. Suggested-by: Richard Henderson Signed-off-by: Víctor Colombo Reviewed-by: Richard Henderson r~
Re: [PATCH v2 07/21] target/ppc: Remove msr_ee macro
On 5/2/22 07:39, Víctor Colombo wrote: msr_ee macro hides the usage of env->msr, which is a bad behavior Substitute it with FIELD_EX64 calls that explicitly use env->msr as a parameter. Suggested-by: Richard Henderson Signed-off-by: Víctor Colombo Reviewed-by: Richard Henderson r~
Re: [PATCH v2 08/21] target/ppc: Remove msr_ce macro
On 5/2/22 07:39, Víctor Colombo wrote: msr_ce macro hides the usage of env->msr, which is a bad behavior Substitute it with FIELD_EX64 calls that explicitly use env->msr as a parameter. Suggested-by: Richard Henderson Signed-off-by: Víctor Colombo Reviewed-by: Richard Henderson r~
Re: [PATCH v2 05/21] target/ppc: Remove msr_ds macro
On 5/2/22 07:39, Víctor Colombo wrote: msr_ds macro hides the usage of env->msr, which is a bad behavior Substitute it with FIELD_EX64 calls that explicitly use env->msr as a parameter. Suggested-by: Richard Henderson Signed-off-by: Víctor Colombo Reviewed-by: Richard Henderson r~
Re: [PATCH v2 03/21] target/ppc: Remove msr_pr macro
On 5/2/22 07:39, Víctor Colombo wrote: msr_pr macro hides the usage of env->msr, which is a bad behavior Substitute it with FIELD_EX64 calls that explicitly use env->msr as a parameter. Suggested-by: Richard Henderson Signed-off-by: Víctor Colombo --- v2: Remove M_MSR_PR and use FIELD_EX64 instead Signed-off-by: Víctor Colombo --- Reviewed-by: Richard Henderson r~
Re: [PATCH v2 19/21] target/ppc: Remove msr_hv macro
On 5/2/22 07:39, Víctor Colombo wrote: msr_hv macro hides the usage of env->msr, which is a bad behavior. Substitute it with FIELD_EX64 calls that explicitly use env->msr as a parameter. Suggested-by: Richard Henderson Signed-off-by: Víctor Colombo --- v2: Remove M_MSR_HV and use FIELD_EX64 instead. In this patch I'm having some problems on how to use FIELD in the 'not defined TARGET_PPC64' case. IIUC in case the target is 32 bits then the mask should be zero. However the mask is created in MAKE_64BIT_MASK, and MAKE_64BIT_MASK(_, 0) triggers undefined behavior as it would try to do a 64 bit right shift. Am I missing an easy way to do this? Sadly not. Reviewed-by: Richard Henderson r~
Re: [PATCH v2 16/21] target/ppc: Remove msr_ep macro
On 5/2/22 07:39, Víctor Colombo wrote:
-if (unlikely((value >> MSR_EP) & 1) != msr_ep) {
-env->excp_prefix = ((value >> MSR_EP) & 1) * 0xFFF0;
+if (unlikely(!(value & env->msr & R_MSR_EP_MASK))) {
Needs xor.
+env->excp_prefix = FIELD_EX64(env->msr, MSR, EP) * 0xFFF0;
Needs extract from value.
r~
Re: [PATCH v2 15/21] target/ppc: Remove msr_dr macro
On 5/2/22 07:39, Víctor Colombo wrote:
if (!(value & env->msr & R_MSR_IR_MASK) ||
-((value >> MSR_DR) & 1) != msr_dr) {
+!(value & env->msr & R_MSR_DR_MASK)) {
Xor, but then this becomes
(value ^ env->msr) & (R_MSR_IR_MASK | R_MSR_DR_MASK)
r~
Re: [PATCH v2 14/21] target/ppc: Remove msr_ir macro
On 5/2/22 07:39, Víctor Colombo wrote: -if (((value >> MSR_IR) & 1) != msr_ir || +if (!(value & env->msr & R_MSR_IR_MASK) || Likewise needs xor. r~
Re: [PATCH v2 11/21] target/ppc: Remove msr_gs macro
On 5/2/22 07:39, Víctor Colombo wrote:
-((value >> MSR_GS) & 1) != msr_gs) {
+!(value & env->msr & R_MSR_GS_MASK)) {
This isn't right. I think you wanted
(value ^ env->msr) & R_MSR_GS_MASK
r~
Re: [PATCH v2 13/21] target/ppc: Remove msr_me macro
On 5/2/22 07:39, Víctor Colombo wrote:
msr_me macro hides the usage of env->msr, which is a bad behavior
Substitute it with FIELD_EX64 calls that explicitly use env->msr
as a parameter.
Suggested-by: Richard Henderson
Signed-off-by: Víctor Colombo
---
v2: Remove M_MSR_CM and use FIELD_EX64 instead
Signed-off-by: Víctor Colombo
---
target/ppc/cpu.h| 2 +-
target/ppc/mmu_common.c | 2 +-
target/ppc/mmu_helper.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 294c61c9dc..8e652691cf 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -354,6 +354,7 @@ typedef enum {
#define MSR_RI 1 /* Recoverable interrupt1
*/
#define MSR_LE 0 /* Little-endian mode 1 hflags
*/
+FIELD(MSR, CM, MSR_CM, 1)
Patch title is wrong.
r~
Re: [PATCH v3 3/3] nbd/server: Allow MULTI_CONN for shared writable exports
On Fri, Apr 29, 2022 at 02:49:35PM +0200, Kevin Wolf wrote: ... > > Or a multi-pathed connection to network storage, where one QEMU > > process accesses the network device, but those accesses may > > round-robin which server they reach, and where any caching at an > > individual server may be inconsistent with what is seen on another > > server unless flushing is used to force the round-robin access to > > synchronize between the multi-path views. > > I don't think this is a realistic scenario. It would mean that you > successfully write data to the storage, and when you then read the same > location, you get different data back. This would be inconsistent even > with a single client. So I'd call this broken storage that should be > replaced as soon as possible. > > I could imagine problems of this kind with two separate connections to > the network storage, but here all the NBD clients share a single > BlockBackend, so for the storage they are a single connection. I like that chain of reasoning. > > > > In fact, I don't think we even need the flush restriction from the NBD > > > spec. All clients see the same state (that of the NBD server > > > BlockBackend) even without anyone issuing any flush. The flush is only > > > needed to make sure that cached data is written to the backing storage > > > when writeback caches are involved. > > > > > > Please correct me if I'm misunderstanding something here. > > > > Likewise me, if I'm being overly cautious. > > > > I can certainly write a simpler v4 that just always advertises > > MULTI_CONN if we allow more than one client, without any knob to > > override it; it's just that it is harder to write a commit message > > justifying why I think it is safe to do so. > > Having an explicit option doesn't hurt, but it's the reasoning in the > commit message that feels wrong to me. > > We could consider changing "auto" to advertise MULTI_CONN even for > writable exports. There might still be a good reason not to do this by > default, though, because of the NBD clients. I'm quite sure that the > backend won't make any trouble, but client might if someone else is > writing to the same image (this is why we require an explicit > share-rw=on for guest devices in the same case). If your worry is about a client trying to determine if writing to an NBD server is going to race with some external process writing to the direct image, I don't see how not advertising MULTI_CONN will make things safer - the NBD client to qemu-nbd will still be going through a single backend, and that race is present even if there is only one NBD client. The point of MULTI_CONN is informing the client that it can open multiple sockets and see a consistent view across all of them, and in your scenario of the server competing with some external process over the underlying data file, that competition is not controlled by how many NBD clients connect to the server, but by the external process having access at the same time the server has access through a single BlockBackend (and would be just as risky as if MULTI_CONN were not advertised and the client limits itself to one NBD connection). If we can argue that our single BlockBackend point of access is safe enough to default to advertising MULTI_CONN for writable connections (when we support parallel clients), then exposing an OnOffAuto knob is overkill. I'm not even sure I can envision a case where needing to not advertise the bit would matter to a client (clients are supposed to ignore unknown feature bits). -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [PATCH v5 7/9] s390x/pci: enable adapter event notification for interpreted devices
On 5/2/22 7:30 AM, Pierre Morel wrote:
On 5/2/22 11:19, Niklas Schnelle wrote:
On Mon, 2022-05-02 at 09:48 +0200, Pierre Morel wrote:
On 4/22/22 14:10, Matthew Rosato wrote:
On 4/22/22 5:39 AM, Pierre Morel wrote:
On 4/4/22 20:17, Matthew Rosato wrote:
Use the associated kvm ioctl operation to enable adapter event
notification
and forwarding for devices when requested. This feature will be
set up
with or without firmware assist based upon the 'forwarding_assist'
setting.
Signed-off-by: Matthew Rosato
---
hw/s390x/s390-pci-bus.c | 20 ++---
hw/s390x/s390-pci-inst.c | 40
+++--
hw/s390x/s390-pci-kvm.c | 30 +
include/hw/s390x/s390-pci-bus.h | 1 +
include/hw/s390x/s390-pci-kvm.h | 14
5 files changed, 100 insertions(+), 5 deletions(-)
diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index 9c02d31250..47918d2ce9 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -190,7 +190,10 @@ void s390_pci_sclp_deconfigure(SCCB *sccb)
rc = SCLP_RC_NO_ACTION_REQUIRED;
break;
default:
- if (pbdev->summary_ind) {
+ if (pbdev->interp && (pbdev->fh & FH_MASK_ENABLE)) {
+ /* Interpreted devices were using interrupt
forwarding */
+ s390_pci_kvm_aif_disable(pbdev);
Same remark as for the kernel part.
The VFIO device is already initialized and the action is on this
device, Shouldn't we use the VFIO device interface instead of the KVM
interface?
I don't necessarily disagree, but in v3 of the kernel series I was told
not to use VFIO ioctls to accomplish tasks that are unique to KVM (e.g.
AEN interpretation) and to instead use a KVM ioctl.
VFIO_DEVICE_SET_IRQS won't work as-is for reasons described in the
kernel series (e.g. we don't see any of the config space notifiers
because of instruction interpretation) -- as far as I can figure we
could add our own s390 code to QEMU to issue VFIO_DEVICE_SET_IRQS
directly for an interpreted device, but I think would also need
s390-specific changes to VFIO_DEVICE_SET_IRQS accommodate this (e.g.
maybe something like a VFIO_IRQ_SET_DATA_S390AEN where we can then
specify the aen information in vfio_irq_set.data -- or something else I
Hi,
yes this in VFIO_DEVICE_SET_IRQS is what I think should be done.
haven't though of yet) -- I can try to look at this some more and
see if
I get a good idea.
I understood that the demand was concerning the IOMMU but I may be
wrong.
The IOMMU was an issue, but the request to move the ioctl out of vfio to
kvm was specifically because these ioctl operations were only relevant
for VMs and are not applicable to vfio uses cases outside of virtualization.
https://lore.kernel.org/kvm/20220208185141.gh4...@nvidia.com/
For my opinion, the handling of AEN is not specific to KVM but specific
to the device, for example the code should be the same if Z ever decide
to use XEN or another hypervizor, except for the GISA part but this part
is already implemented in KVM in a way it can be used from a device like
in VFIO AP.
Fundamentally, these operations are valid only when you have _both_ a
virtual machine and vfio device. (Yes, you could swap in a new
hypervisor with a new GISA implementation, but at the end of it the
hypervisor must still provide the GISA designation for this to work)
If fh lookup is a concern, one idea that Jason floated was passing the
vfio device fd as an argument to the kvm ioctl (so pass this down on a
kvm ioctl from userspace instead of a fh) and then using a new vfio
external API to get the relevant device from the provided fd.
https://lore.kernel.org/kvm/20220208195117.gi4...@nvidia.com/
@Alex, what do you think?
Regards,
Pierre
As I understand it the question isn't if it is specific to KVM but
rather if it is specific to virtualization. As vfio-pci is also used
for non virtualization purposes such as with DPDK/SPDK or a fully
emulating QEMU, it should only be in VFIO if it is relevant for these
kinds of user-space PCI accesses too. I'm not an AEN expert but as I
understand it, this does forwarding interrupts into a SIE context which
only makes sense for virtualization not for general user-space PCI.
Right, AEN forwarding is only relevant for virtual machines.
Being in VFIO kernel part does not mean that this part should be called
from any user of VFIO in userland.
That is a reason why I did propose an extension and not using the
current implementation of VFIO_DEVICE_SET_IRQS as is.
The reason behind is that the AEN hardware handling is device specific:
we need the Function Handle to program AEN.
You also need the GISA designation which is provided by the kvm or you
also can't program AEN. So you ultimately need both a function handle
that is 'owned' by the device (vfio device fd) and the GISA designation
that is 'owned' by kvm (kvm fd). So there are 2 different
Re: [PATCH 0/7] qapi: Primarily whitespace tweaks
Andrea Bolognani writes: > On Mon, May 02, 2022 at 02:43:52PM +0200, Markus Armbruster wrote: >> PATCH 1-5 queued, because no-brainers :) > > Thanks! > > How do you want me to handle respinning 6/7 and 7/7? Send out the > entire series again with those two patches tweaked, wait for your > pull request to make it into the tree, something else... ? If you like, I can post a pull request tomorrow morning. Respinning the complete series is fine with me. > What about the changes you suggested to the commit message of 3/7? If you respin the complete series, you get to can amend it. Else I do.
Re: [PATCH 6/7] qapi: Drop unnecessary horizontal spacing in comments
Andrea Bolognani writes: > On Mon, May 02, 2022 at 10:50:07AM +0200, Markus Armbruster wrote: >> Andrea Bolognani writes: >> > -# @writeback: true if writeback mode is enabled >> > -# @direct: true if the host page cache is bypassed (O_DIRECT) >> > -# @no-flush:true if flush requests are ignored for the device >> > +# @writeback: true if writeback mode is enabled >> > +# @direct:true if the host page cache is bypassed (O_DIRECT) >> > +# @no-flush: true if flush requests are ignored for the device >> >> I'm no fan of horizontally aligning descriptions, because when you add a >> longer name, you either realign (I hate the churn) or live with the >> inconsistency (I hate that, too). > > We seem to be in violent agreement on the topic, but it's apparent > that other people feel diffently :) > >> I doubt changing to a different alignment now is useful. The next >> patch, however, drops the alignment entirely. Possibly useful. >> >> Thoughts? > > My rationale for splitting things the way I did is that, if dropping > the horizontal alignment entirely was not considered desirable, we > could at least get rid of the extra whitespace. Understood. > But if you think that > the benefit from the half measure doesn't offset the cost of the > churn it causes, I'm happy to drop these hunks and go straight from > the current status to no horizontal alignment at all in one fell > swoop with the next patch. Show us the patches, and then we can decide whether the improvement is worth the churn. >> > -# Since: 0.14 >> > +# Since: 0.14 >> >> This one is TAG: TEXT, whereas the one above is a multiple @NAME: >> DESCRIPTION. Extra space in the latter can provide alignment. Extra >> space in the former is always redundant. I'd take a patch dropping >> these obviously redundant spaces without debate :) > > Okay, I'll respin this so that the first patch drops all extra > whitespace in contexts where horizontal alignment is either not > attempted or not possible, and the second one implements the more > controversial changes. The first one is another no-brainer.
[PATCH] tests/tcg/s390x: Use a different PCRel32 notation in branch-relative-long.c
Binutils >=2.37 and Clang do not accept (. - 0x1) PCRel32
constants. While this looks like a bug that needs fixing, use a
different notation (-0x1) as a workaround.
Reported-by: Thomas Huth
Signed-off-by: Ilya Leoshkevich
---
tests/tcg/s390x/branch-relative-long.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/tcg/s390x/branch-relative-long.c
b/tests/tcg/s390x/branch-relative-long.c
index 94219afcad..8ce9f1c2e5 100644
--- a/tests/tcg/s390x/branch-relative-long.c
+++ b/tests/tcg/s390x/branch-relative-long.c
@@ -13,8 +13,8 @@
#_name "_end:\n");
DEFINE_ASM(br_r14, "br %r14");
-DEFINE_ASM(brasl_r0, "brasl %r0,.-0x1");
-DEFINE_ASM(brcl_0xf, "brcl 0xf,.-0x1");
+DEFINE_ASM(brasl_r0, "brasl %r0,-0x1");
+DEFINE_ASM(brcl_0xf, "brcl 0xf,-0x1");
struct test {
const char *code;
--
2.35.1
Re: [PATCH v6 00/13] s390x/tcg: Implement Vector-Enhancements Facility 2
There was also the patch that had them as .insn in the other series of emails. On Mon, May 2, 2022 at 11:52 AM David Hildenbrand wrote: > > On 02.05.22 09:20, Thomas Huth wrote: > > On 28/04/2022 11.46, David Hildenbrand wrote: > >> Implement Vector-Enhancements Facility 2 for s390x > >> > >> resolves: https://gitlab.com/qemu-project/qemu/-/issues/738 > >> > >> implements: > >> VECTOR LOAD ELEMENTS REVERSED (VLER) > >> VECTOR LOAD BYTE REVERSED ELEMENTS (VLBR) > >> VECTOR LOAD BYTE REVERSED ELEMENT (VLEBRH, VLEBRF, VLEBRG) > >> VECTOR LOAD BYTE REVERSED ELEMENT AND ZERO (VLLEBRZ) > >> VECTOR LOAD BYTE REVERSED ELEMENT AND REPLICATE (VLBRREP) > >> VECTOR STORE ELEMENTS REVERSED (VSTER) > >> VECTOR STORE BYTE REVERSED ELEMENTS (VSTBR) > >> VECTOR STORE BYTE REVERSED ELEMENTS (VSTEBRH, VSTEBRF, > >> VSTEBRG) > >> VECTOR SHIFT LEFT DOUBLE BY BIT (VSLD) > >> VECTOR SHIFT RIGHT DOUBLE BY BIT(VSRD) > >> VECTOR STRING SEARCH(VSTRS) > >> > >> modifies: > >> VECTOR FP CONVERT FROM FIXED(VCFPS) > >> VECTOR FP CONVERT FROM LOGICAL (VCFPL) > >> VECTOR FP CONVERT TO FIXED (VCSFP) > >> VECTOR FP CONVERT TO LOGICAL(VCLFP) > >> VECTOR SHIFT LEFT (VSL) > >> VECTOR SHIFT RIGHT ARITHMETIC (VSRA) > >> VECTOR SHIFT RIGHT LOGICAL (VSRL) > > > > Thanks, queued to my s390x-next branch now: > > > > https://gitlab.com/thuth/qemu/-/commits/s390x-next/ > > > Thanks for fixing up. At this point I would have suggested to exclude > the tests for now. > > -- > Thanks, > > David / dhildenb >
Re: [PATCH] hw/gpio/aspeed_gpio: Fix QOM pin property
> On May 2, 2022, at 8:09 AM, Cédric Le Goater wrote:
>
> On 5/2/22 10:08, Peter Delevoryas wrote:
>> I was setting gpioV4-7 to "1110" using the QOM pin property handler and
>> noticed that lowering gpioV7 was inadvertently lowering gpioV4-6 too.
>> (qemu) qom-set /machine/soc/gpio gpioV4 true
>> (qemu) qom-set /machine/soc/gpio gpioV5 true
>> (qemu) qom-set /machine/soc/gpio gpioV6 true
>> (qemu) qom-get /machine/soc/gpio gpioV4
>> true
>> (qemu) qom-set /machine/soc/gpio gpioV7 false
>> (qemu) qom-get /machine/soc/gpio gpioV4
>> false
>> An expression in aspeed_gpio_set_pin_level was using a logical NOT
>> operator instead of a bitwise NOT operator:
>> value &= !pin_mask;
>> The original author probably intended to make a bitwise NOT expression
>> "~", but mistakenly used a logical NOT operator "!" instead. Some
>> programming languages like Rust use "!" for both purposes.
>> Fixes: 4b7f956862dc ("hw/gpio: Add basic Aspeed GPIO model for AST2400 and
>> AST2500")
>> Signed-off-by: Peter Delevoryas
>
> Nice catch !
>
> Reviewed-by: Cédric Le Goater
>
> I was going to send a PR but I will wait a bit to include this fix.
That’s great, thanks!
>
> Thanks,
>
> C.
>
>
>> ---
>> hw/gpio/aspeed_gpio.c | 2 +-
>> tests/qtest/aspeed_gpio-test.c | 87 ++
>> tests/qtest/meson.build | 3 +-
>> 3 files changed, 90 insertions(+), 2 deletions(-)
>> create mode 100644 tests/qtest/aspeed_gpio-test.c
>> diff --git a/hw/gpio/aspeed_gpio.c b/hw/gpio/aspeed_gpio.c
>> index c63634d3d3..9b736e7a9f 100644
>> --- a/hw/gpio/aspeed_gpio.c
>> +++ b/hw/gpio/aspeed_gpio.c
>> @@ -312,7 +312,7 @@ static void aspeed_gpio_set_pin_level(AspeedGPIOState
>> *s, uint32_t set_idx,
>> if (level) {
>> value |= pin_mask;
>> } else {
>> - value &= !pin_mask;
>> + value &= ~pin_mask;
>> }
>> aspeed_gpio_update(s, >sets[set_idx], value);
>> diff --git a/tests/qtest/aspeed_gpio-test.c b/tests/qtest/aspeed_gpio-test.c
>> new file mode 100644
>> index 00..c1003f2d1b
>> --- /dev/null
>> +++ b/tests/qtest/aspeed_gpio-test.c
>> @@ -0,0 +1,87 @@
>> +/*
>> + * QTest testcase for the Aspeed GPIO Controller.
>> + *
>> + * Copyright (c) Meta Platforms, Inc. and affiliates. (http://www.meta.com)
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a
>> copy
>> + * of this software and associated documentation files (the "Software"), to
>> deal
>> + * in the Software without restriction, including without limitation the
>> rights
>> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
>> + * copies of the Software, and to permit persons to whom the Software is
>> + * furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice shall be included
>> in
>> + * all copies or substantial portions of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
>> OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
>> OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
>> FROM,
>> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
>> + * THE SOFTWARE.
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "qemu/bitops.h"
>> +#include "qemu/timer.h"
>> +#include "qapi/qmp/qdict.h"
>> +#include "libqtest-single.h"
>> +
>> +static bool qom_get_bool(QTestState *s, const char *path, const char
>> *property)
>> +{
>> + QDict *r;
>> + bool b;
>> +
>> + r = qtest_qmp(s, "{ 'execute': 'qom-get', 'arguments': "
>> + "{ 'path': %s, 'property': %s } }", path, property);
>> + b = qdict_get_bool(r, "return");
>> + qobject_unref(r);
>> +
>> + return b;
>> +}
>> +
>> +static void qom_set_bool(QTestState *s, const char *path, const char
>> *property,
>> + bool value)
>> +{
>> + QDict *r;
>> +
>> + r = qtest_qmp(s, "{ 'execute': 'qom-set', 'arguments': "
>> + "{ 'path': %s, 'property': %s, 'value': %i } }",
>> + path, property, value);
>> + qobject_unref(r);
>> +}
>> +
>> +static void test_set_colocated_pins(const void *data)
>> +{
>> + QTestState *s = (QTestState *)data;
>> +
>> + /*
>> + * gpioV4-7 occupy bits within a single 32-bit value, so we want to make
>> + * sure that modifying one doesn't affect the other.
>> + */
>> + qom_set_bool(s, "/machine/soc/gpio", "gpioV4", true);
>> + qom_set_bool(s, "/machine/soc/gpio", "gpioV5", false);
>> + qom_set_bool(s, "/machine/soc/gpio", "gpioV6", true);
>> + qom_set_bool(s, "/machine/soc/gpio", "gpioV7", false);
>> + g_assert(qom_get_bool(s, "/machine/soc/gpio", "gpioV4"));
>> + g_assert(!qom_get_bool(s, "/machine/soc/gpio", "gpioV5"));
>> + g_assert(qom_get_bool(s, "/machine/soc/gpio", "gpioV6"));
>> + g_assert(!qom_get_bool(s, "/machine/soc/gpio", "gpioV7"));
>> +}
>> +
>> +int
Re: [PATCH v6 00/13] s390x/tcg: Implement Vector-Enhancements Facility 2
On 02.05.22 09:20, Thomas Huth wrote: > On 28/04/2022 11.46, David Hildenbrand wrote: >> Implement Vector-Enhancements Facility 2 for s390x >> >> resolves: https://gitlab.com/qemu-project/qemu/-/issues/738 >> >> implements: >> VECTOR LOAD ELEMENTS REVERSED (VLER) >> VECTOR LOAD BYTE REVERSED ELEMENTS (VLBR) >> VECTOR LOAD BYTE REVERSED ELEMENT (VLEBRH, VLEBRF, VLEBRG) >> VECTOR LOAD BYTE REVERSED ELEMENT AND ZERO (VLLEBRZ) >> VECTOR LOAD BYTE REVERSED ELEMENT AND REPLICATE (VLBRREP) >> VECTOR STORE ELEMENTS REVERSED (VSTER) >> VECTOR STORE BYTE REVERSED ELEMENTS (VSTBR) >> VECTOR STORE BYTE REVERSED ELEMENTS (VSTEBRH, VSTEBRF, VSTEBRG) >> VECTOR SHIFT LEFT DOUBLE BY BIT (VSLD) >> VECTOR SHIFT RIGHT DOUBLE BY BIT(VSRD) >> VECTOR STRING SEARCH(VSTRS) >> >> modifies: >> VECTOR FP CONVERT FROM FIXED(VCFPS) >> VECTOR FP CONVERT FROM LOGICAL (VCFPL) >> VECTOR FP CONVERT TO FIXED (VCSFP) >> VECTOR FP CONVERT TO LOGICAL(VCLFP) >> VECTOR SHIFT LEFT (VSL) >> VECTOR SHIFT RIGHT ARITHMETIC (VSRA) >> VECTOR SHIFT RIGHT LOGICAL (VSRL) > > Thanks, queued to my s390x-next branch now: > > https://gitlab.com/thuth/qemu/-/commits/s390x-next/ > Thanks for fixing up. At this point I would have suggested to exclude the tests for now. -- Thanks, David / dhildenb
Re: [PATCH] hw/gpio/aspeed_gpio: Fix QOM pin property
On 5/2/22 10:08, Peter Delevoryas wrote:
I was setting gpioV4-7 to "1110" using the QOM pin property handler and
noticed that lowering gpioV7 was inadvertently lowering gpioV4-6 too.
(qemu) qom-set /machine/soc/gpio gpioV4 true
(qemu) qom-set /machine/soc/gpio gpioV5 true
(qemu) qom-set /machine/soc/gpio gpioV6 true
(qemu) qom-get /machine/soc/gpio gpioV4
true
(qemu) qom-set /machine/soc/gpio gpioV7 false
(qemu) qom-get /machine/soc/gpio gpioV4
false
An expression in aspeed_gpio_set_pin_level was using a logical NOT
operator instead of a bitwise NOT operator:
value &= !pin_mask;
The original author probably intended to make a bitwise NOT expression
"~", but mistakenly used a logical NOT operator "!" instead. Some
programming languages like Rust use "!" for both purposes.
Fixes: 4b7f956862dc ("hw/gpio: Add basic Aspeed GPIO model for AST2400 and
AST2500")
Signed-off-by: Peter Delevoryas
Nice catch !
Reviewed-by: Cédric Le Goater
I was going to send a PR but I will wait a bit to include this fix.
Thanks,
C.
---
hw/gpio/aspeed_gpio.c | 2 +-
tests/qtest/aspeed_gpio-test.c | 87 ++
tests/qtest/meson.build| 3 +-
3 files changed, 90 insertions(+), 2 deletions(-)
create mode 100644 tests/qtest/aspeed_gpio-test.c
diff --git a/hw/gpio/aspeed_gpio.c b/hw/gpio/aspeed_gpio.c
index c63634d3d3..9b736e7a9f 100644
--- a/hw/gpio/aspeed_gpio.c
+++ b/hw/gpio/aspeed_gpio.c
@@ -312,7 +312,7 @@ static void aspeed_gpio_set_pin_level(AspeedGPIOState *s,
uint32_t set_idx,
if (level) {
value |= pin_mask;
} else {
-value &= !pin_mask;
+value &= ~pin_mask;
}
aspeed_gpio_update(s, >sets[set_idx], value);
diff --git a/tests/qtest/aspeed_gpio-test.c b/tests/qtest/aspeed_gpio-test.c
new file mode 100644
index 00..c1003f2d1b
--- /dev/null
+++ b/tests/qtest/aspeed_gpio-test.c
@@ -0,0 +1,87 @@
+/*
+ * QTest testcase for the Aspeed GPIO Controller.
+ *
+ * Copyright (c) Meta Platforms, Inc. and affiliates. (http://www.meta.com)
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/bitops.h"
+#include "qemu/timer.h"
+#include "qapi/qmp/qdict.h"
+#include "libqtest-single.h"
+
+static bool qom_get_bool(QTestState *s, const char *path, const char *property)
+{
+QDict *r;
+bool b;
+
+r = qtest_qmp(s, "{ 'execute': 'qom-get', 'arguments': "
+ "{ 'path': %s, 'property': %s } }", path, property);
+b = qdict_get_bool(r, "return");
+qobject_unref(r);
+
+return b;
+}
+
+static void qom_set_bool(QTestState *s, const char *path, const char *property,
+ bool value)
+{
+QDict *r;
+
+r = qtest_qmp(s, "{ 'execute': 'qom-set', 'arguments': "
+ "{ 'path': %s, 'property': %s, 'value': %i } }",
+ path, property, value);
+qobject_unref(r);
+}
+
+static void test_set_colocated_pins(const void *data)
+{
+QTestState *s = (QTestState *)data;
+
+/*
+ * gpioV4-7 occupy bits within a single 32-bit value, so we want to make
+ * sure that modifying one doesn't affect the other.
+ */
+qom_set_bool(s, "/machine/soc/gpio", "gpioV4", true);
+qom_set_bool(s, "/machine/soc/gpio", "gpioV5", false);
+qom_set_bool(s, "/machine/soc/gpio", "gpioV6", true);
+qom_set_bool(s, "/machine/soc/gpio", "gpioV7", false);
+g_assert(qom_get_bool(s, "/machine/soc/gpio", "gpioV4"));
+g_assert(!qom_get_bool(s, "/machine/soc/gpio", "gpioV5"));
+g_assert(qom_get_bool(s, "/machine/soc/gpio", "gpioV6"));
+g_assert(!qom_get_bool(s, "/machine/soc/gpio", "gpioV7"));
+}
+
+int main(int argc, char **argv)
+{
+QTestState *s;
+int r;
+
+g_test_init(, , NULL);
+
+s = qtest_init("-machine ast2600-evb");
+qtest_add_data_func("/ast2600/gpio/set_colocated_pins", s,
+
[PATCH v2 21/21] target/ppc: Change MSR_* to follow POWER ISA numbering convention
Today we have the issue where MSR_* values are the 'inverted order'
bit numbers from what the ISA specifies. e.g. MSR_LE is bit 63 but
is defined as 0 in QEMU.
Add a macro to be used to convert from QEMU order to ISA order.
This solution requires less changes than to use the already defined
PPC_BIT macro, which would turn MSR_* in masks instead of the numbers
itself.
Signed-off-by: Víctor Colombo
---
It'll also allow other places in cpu.h to be changed, like FSCR_*,
TEXASR_*, and FPSCR_*.
Signed-off-by: Víctor Colombo
---
target/ppc/cpu.h | 87
1 file changed, 44 insertions(+), 43 deletions(-)
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 5cd9d88555..9ed2dbb7bd 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -38,6 +38,7 @@
#define PPC_ELF_MACHINE EM_PPC
#endif
+#define PPC_BIT_NR(bit) (63 - (bit))
#define PPC_BIT(bit)(0x8000ULL >> (bit))
#define PPC_BIT32(bit) (0x8000 >> (bit))
#define PPC_BIT8(bit) (0x80 >> (bit))
@@ -310,49 +311,49 @@ typedef enum {
/*/
/* Machine state register bits definition*/
-#define MSR_SF 63 /* Sixty-four-bit modehflags */
-#define MSR_TAG 62 /* Tag-active mode (POWERx ?)*/
-#define MSR_ISF 61 /* Sixty-four-bit interrupt mode on 630 */
-#define MSR_HV 60 /* hypervisor state hflags */
-#define MSR_TS0 34 /* Transactional state, 2 bits (Book3s) */
-#define MSR_TS1 33
-#define MSR_TM 32 /* Transactional Memory Available (Book3s) */
-#define MSR_CM 31 /* Computation mode for BookE hflags */
-#define MSR_ICM 30 /* Interrupt computation mode for BookE */
-#define MSR_GS 28 /* guest state for BookE */
-#define MSR_UCLE 26 /* User-mode cache lock enable for BookE */
-#define MSR_VR 25 /* altivec availablex hflags */
-#define MSR_SPE 25 /* SPE enable for BookE x hflags */
-#define MSR_VSX 23 /* Vector Scalar Extension (ISA 2.06 and later) x hflags */
-#define MSR_S22 /* Secure state */
-#define MSR_KEY 19 /* key bit on 603e */
-#define MSR_POW 18 /* Power management */
-#define MSR_WE 18 /* Wait State Enable on 405 */
-#define MSR_TGPR 17 /* TGPR usage on 602/603x*/
-#define MSR_CE 17 /* Critical interrupt enable on embedded PowerPC x */
-#define MSR_ILE 16 /* Interrupt little-endian mode */
-#define MSR_EE 15 /* External interrupt enable */
-#define MSR_PR 14 /* Problem state hflags */
-#define MSR_FP 13 /* Floating point available hflags */
-#define MSR_ME 12 /* Machine check interrupt enable*/
-#define MSR_FE0 11 /* Floating point exception mode 0 */
-#define MSR_SE 10 /* Single-step trace enable x hflags */
-#define MSR_DWE 10 /* Debug wait enable on 405 x*/
-#define MSR_UBLE 10 /* User BTB lock enable on e500 x*/
-#define MSR_BE 9 /* Branch trace enable x hflags */
-#define MSR_DE 9 /* Debug interrupts enable on embedded PowerPC x*/
-#define MSR_FE1 8 /* Floating point exception mode 1 */
-#define MSR_AL 7 /* AL bit on POWER */
-#define MSR_EP 6 /* Exception prefix on 601 */
-#define MSR_IR 5 /* Instruction relocate */
-#define MSR_DR 4 /* Data relocate */
-#define MSR_IS 5 /* Instruction address space (BookE) */
-#define MSR_DS 4 /* Data address space (BookE)*/
-#define MSR_PE 3 /* Protection enable on 403 */
-#define MSR_PX 2 /* Protection exclusive on 403 x*/
-#define MSR_PMM 2 /* Performance monitor mark on POWERx*/
-#define MSR_RI 1 /* Recoverable interrupt1*/
-#define MSR_LE 0 /* Little-endian mode 1 hflags */
+#define MSR_SF PPC_BIT_NR(0) /* Sixty-four-bit modehflags */
+#define MSR_TAG PPC_BIT_NR(1) /* Tag-active mode (POWERx ?)*/
+#define MSR_ISF PPC_BIT_NR(2) /* Sixty-four-bit interrupt mode on 630 */
+#define MSR_HV PPC_BIT_NR(3) /* hypervisor state
[PATCH v2 19/21] target/ppc: Remove msr_hv macro
msr_hv macro hides the usage of env->msr, which is a bad
behavior. Substitute it with FIELD_EX64 calls that explicitly use
env->msr as a parameter.
Suggested-by: Richard Henderson
Signed-off-by: Víctor Colombo
---
v2: Remove M_MSR_HV and use FIELD_EX64 instead.
In this patch I'm having some problems on how to use FIELD in the
'not defined TARGET_PPC64' case. IIUC in case the target is 32 bits
then the mask should be zero. However the mask is created in
MAKE_64BIT_MASK, and MAKE_64BIT_MASK(_, 0) triggers undefined behavior
as it would try to do a 64 bit right shift. Am I missing an easy way
to do this?
Signed-off-by: Víctor Colombo
---
target/ppc/cpu.h | 12 ++--
target/ppc/cpu_init.c| 6 --
target/ppc/excp_helper.c | 8
target/ppc/mem_helper.c | 4 ++--
target/ppc/misc_helper.c | 2 +-
target/ppc/mmu-radix64.c | 6 +++---
6 files changed, 20 insertions(+), 18 deletions(-)
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 0b861660ae..9b765af4db 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -354,6 +354,12 @@ typedef enum {
#define MSR_RI 1 /* Recoverable interrupt1*/
#define MSR_LE 0 /* Little-endian mode 1 hflags */
+#if defined(TARGET_PPC64)
+FIELD(MSR, HV, MSR_HV, 1)
+#define FIELD_EX64_HV(storage) FIELD_EX64(storage, MSR, HV)
+#else
+#define FIELD_EX64_HV(storage) 0
+#endif
FIELD(MSR, TS, MSR_TS0, 2)
FIELD(MSR, CM, MSR_CM, 1)
FIELD(MSR, GS, MSR_GS, 1)
@@ -483,12 +489,6 @@ FIELD(MSR, LE, MSR_LE, 1)
#define HFSCR_MSGP PPC_BIT(53) /* Privileged Message Send Facilities */
#define HFSCR_IC_MSGP 0xA
-#if defined(TARGET_PPC64)
-#define msr_hv ((env->msr >> MSR_HV) & 1)
-#else
-#define msr_hv (0)
-#endif
-
#define DBCR0_ICMP (1 << 27)
#define DBCR0_BRT (1 << 26)
#define DBSR_ICMP (1 << 27)
diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index 10e7c41bc9..d4c7813de5 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -6305,7 +6305,8 @@ static bool cpu_has_work_POWER9(CPUState *cs)
if ((env->pending_interrupts & (1u << PPC_INTERRUPT_EXT)) &&
(env->spr[SPR_LPCR] & LPCR_EEE)) {
bool heic = !!(env->spr[SPR_LPCR] & LPCR_HEIC);
-if (!heic || !msr_hv || FIELD_EX64(env->msr, MSR, PR)) {
+if (!heic || !FIELD_EX64_HV(env->msr) ||
+FIELD_EX64(env->msr, MSR, PR)) {
return true;
}
}
@@ -6520,7 +6521,8 @@ static bool cpu_has_work_POWER10(CPUState *cs)
if ((env->pending_interrupts & (1u << PPC_INTERRUPT_EXT)) &&
(env->spr[SPR_LPCR] & LPCR_EEE)) {
bool heic = !!(env->spr[SPR_LPCR] & LPCR_HEIC);
-if (!heic || !msr_hv || FIELD_EX64(env->msr, MSR, PR)) {
+if (!heic || !FIELD_EX64_HV(env->msr) ||
+FIELD_EX64(env->msr, MSR, PR)) {
return true;
}
}
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 549b86cf0b..2c6ec30c09 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -1727,7 +1727,7 @@ static void ppc_hw_interrupt(CPUPPCState *env)
if (env->pending_interrupts & (1 << PPC_INTERRUPT_HDECR)) {
/* LPCR will be clear when not supported so this will work */
bool hdice = !!(env->spr[SPR_LPCR] & LPCR_HDICE);
-if ((async_deliver || msr_hv == 0) && hdice) {
+if ((async_deliver || !FIELD_EX64_HV(env->msr)) && hdice) {
/* HDEC clears on delivery */
env->pending_interrupts &= ~(1 << PPC_INTERRUPT_HDECR);
powerpc_excp(cpu, POWERPC_EXCP_HDECR);
@@ -1739,7 +1739,7 @@ static void ppc_hw_interrupt(CPUPPCState *env)
if (env->pending_interrupts & (1 << PPC_INTERRUPT_HVIRT)) {
/* LPCR will be clear when not supported so this will work */
bool hvice = !!(env->spr[SPR_LPCR] & LPCR_HVICE);
-if ((async_deliver || msr_hv == 0) && hvice) {
+if ((async_deliver || !FIELD_EX64_HV(env->msr)) && hvice) {
powerpc_excp(cpu, POWERPC_EXCP_HVIRT);
return;
}
@@ -1750,9 +1750,9 @@ static void ppc_hw_interrupt(CPUPPCState *env)
bool lpes0 = !!(env->spr[SPR_LPCR] & LPCR_LPES0);
bool heic = !!(env->spr[SPR_LPCR] & LPCR_HEIC);
/* HEIC blocks delivery to the hypervisor */
-if ((async_deliver && !(heic && msr_hv &&
+if ((async_deliver && !(heic && FIELD_EX64_HV(env->msr) &&
!FIELD_EX64(env->msr, MSR, PR))) ||
-(env->has_hv_mode && msr_hv == 0 && !lpes0)) {
+(env->has_hv_mode && !FIELD_EX64_HV(env->msr) && !lpes0)) {
if (books_vhyp_promotes_external_to_hvirt(cpu)) {
powerpc_excp(cpu, POWERPC_EXCP_HVIRT);
} else {
diff --git a/target/ppc/mem_helper.c b/target/ppc/mem_helper.c
index 9af135e88e..d1163f316c 100644
--- a/target/ppc/mem_helper.c
+++
[PATCH v2 18/21] target/ppc: Remove msr_ts macro
msr_ts macro hides the usage of env->msr, which is a bad
behavior. Substitute it with FIELD_EX64 calls that explicitly use
env->msr as a parameter.
Suggested-by: Richard Henderson
Signed-off-by: Víctor Colombo
---
v2: Remove M_MSR_TS* and use FIELD_EX64 instead.
Signed-off-by: Víctor Colombo
---
target/ppc/cpu.h | 2 +-
target/ppc/kvm.c | 4 ++--
target/ppc/machine.c | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 5fce8f00da..0b861660ae 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -354,6 +354,7 @@ typedef enum {
#define MSR_RI 1 /* Recoverable interrupt1*/
#define MSR_LE 0 /* Little-endian mode 1 hflags */
+FIELD(MSR, TS, MSR_TS0, 2)
FIELD(MSR, CM, MSR_CM, 1)
FIELD(MSR, GS, MSR_GS, 1)
FIELD(MSR, POW, MSR_POW, 1)
@@ -487,7 +488,6 @@ FIELD(MSR, LE, MSR_LE, 1)
#else
#define msr_hv (0)
#endif
-#define msr_ts ((env->msr >> MSR_TS1) & 3)
#define DBCR0_ICMP (1 << 27)
#define DBCR0_BRT (1 << 26)
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index 8276326de9..59db1b9227 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -973,7 +973,7 @@ int kvm_arch_put_registers(CPUState *cs, int level)
}
#ifdef TARGET_PPC64
-if (msr_ts) {
+if (FIELD_EX64(env->msr, MSR, TS)) {
for (i = 0; i < ARRAY_SIZE(env->tm_gpr); i++) {
kvm_set_one_reg(cs, KVM_REG_PPC_TM_GPR(i), >tm_gpr[i]);
}
@@ -1281,7 +1281,7 @@ int kvm_arch_get_registers(CPUState *cs)
}
#ifdef TARGET_PPC64
-if (msr_ts) {
+if (FIELD_EX64(env->msr, MSR, TS)) {
for (i = 0; i < ARRAY_SIZE(env->tm_gpr); i++) {
kvm_get_one_reg(cs, KVM_REG_PPC_TM_GPR(i), >tm_gpr[i]);
}
diff --git a/target/ppc/machine.c b/target/ppc/machine.c
index e673944597..7104a5c67e 100644
--- a/target/ppc/machine.c
+++ b/target/ppc/machine.c
@@ -417,7 +417,7 @@ static bool tm_needed(void *opaque)
{
PowerPCCPU *cpu = opaque;
CPUPPCState *env = >env;
-return msr_ts;
+return FIELD_EX64(env->msr, MSR, TS);
}
static const VMStateDescription vmstate_tm = {
--
2.25.1
[PATCH v2 17/21] target/ppc: Remove msr_fe0 and msr_fe1 macros
msr_fe0 and msr_fe1 macros hide the usage of env->msr, which is a bad
behavior. Substitute it with FIELD_EX64 calls that explicitly use
env->msr as a parameter.
Suggested-by: Richard Henderson
Signed-off-by: Víctor Colombo
---
v2: Remove M_MSR_FE* and use FIELD_EX64 instead. As the bit numbers
for FE0 and FE1 are not continuous, we can't benefit from a unified
FIELD for them :(
Signed-off-by: Víctor Colombo
---
target/ppc/cpu.h | 5 +++--
target/ppc/excp_helper.c | 18 --
2 files changed, 15 insertions(+), 8 deletions(-)
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 9683e6a359..5fce8f00da 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -363,6 +363,9 @@ FIELD(MSR, EE, MSR_EE, 1)
FIELD(MSR, PR, MSR_PR, 1)
FIELD(MSR, FP, MSR_FP, 1)
FIELD(MSR, ME, MSR_ME, 1)
+/* MSR_FE0 and MSR_FE1 are not side-by-side so we can't combine them */
+FIELD(MSR, FE0, MSR_FE0, 1)
+FIELD(MSR, FE1, MSR_FE1, 1)
FIELD(MSR, EP, MSR_EP, 1)
FIELD(MSR, IR, MSR_IR, 1)
FIELD(MSR, DR, MSR_DR, 1)
@@ -484,8 +487,6 @@ FIELD(MSR, LE, MSR_LE, 1)
#else
#define msr_hv (0)
#endif
-#define msr_fe0 ((env->msr >> MSR_FE0) & 1)
-#define msr_fe1 ((env->msr >> MSR_FE1) & 1)
#define msr_ts ((env->msr >> MSR_TS1) & 3)
#define DBCR0_ICMP (1 << 27)
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 30baad0489..549b86cf0b 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -478,7 +478,8 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
case POWERPC_EXCP_PROGRAM: /* Program exception*/
switch (env->error_code & ~0xF) {
case POWERPC_EXCP_FP:
-if ((msr_fe0 == 0 && msr_fe1 == 0) ||
+if ((!FIELD_EX64(env->msr, MSR, FE0) &&
+!FIELD_EX64(env->msr, MSR, FE1)) ||
!FIELD_EX64(env->msr, MSR, FP)) {
trace_ppc_excp_fp_ignore();
powerpc_reset_excp_state(cpu);
@@ -616,7 +617,8 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp)
case POWERPC_EXCP_PROGRAM: /* Program exception*/
switch (env->error_code & ~0xF) {
case POWERPC_EXCP_FP:
-if ((msr_fe0 == 0 && msr_fe1 == 0) ||
+if ((!FIELD_EX64(env->msr, MSR, FE0) &&
+!FIELD_EX64(env->msr, MSR, FE1)) ||
!FIELD_EX64(env->msr, MSR, FP)) {
trace_ppc_excp_fp_ignore();
powerpc_reset_excp_state(cpu);
@@ -790,7 +792,8 @@ static void powerpc_excp_7xx(PowerPCCPU *cpu, int excp)
case POWERPC_EXCP_PROGRAM: /* Program exception*/
switch (env->error_code & ~0xF) {
case POWERPC_EXCP_FP:
-if ((msr_fe0 == 0 && msr_fe1 == 0) ||
+if ((!FIELD_EX64(env->msr, MSR, FE0) &&
+!FIELD_EX64(env->msr, MSR, FE1)) ||
!FIELD_EX64(env->msr, MSR, FP)) {
trace_ppc_excp_fp_ignore();
powerpc_reset_excp_state(cpu);
@@ -976,7 +979,8 @@ static void powerpc_excp_74xx(PowerPCCPU *cpu, int excp)
case POWERPC_EXCP_PROGRAM: /* Program exception*/
switch (env->error_code & ~0xF) {
case POWERPC_EXCP_FP:
-if ((msr_fe0 == 0 && msr_fe1 == 0) ||
+if ((!FIELD_EX64(env->msr, MSR, FE0) &&
+!FIELD_EX64(env->msr, MSR, FE1)) ||
!FIELD_EX64(env->msr, MSR, FP)) {
trace_ppc_excp_fp_ignore();
powerpc_reset_excp_state(cpu);
@@ -1175,7 +1179,8 @@ static void powerpc_excp_booke(PowerPCCPU *cpu, int excp)
case POWERPC_EXCP_PROGRAM: /* Program exception*/
switch (env->error_code & ~0xF) {
case POWERPC_EXCP_FP:
-if ((msr_fe0 == 0 && msr_fe1 == 0) ||
+if ((!FIELD_EX64(env->msr, MSR, FE0) &&
+!FIELD_EX64(env->msr, MSR, FE1)) ||
!FIELD_EX64(env->msr, MSR, FP)) {
trace_ppc_excp_fp_ignore();
powerpc_reset_excp_state(cpu);
@@ -1439,7 +1444,8 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
case POWERPC_EXCP_PROGRAM: /* Program exception*/
switch (env->error_code & ~0xF) {
case POWERPC_EXCP_FP:
-if ((msr_fe0 == 0 && msr_fe1 == 0) ||
+if ((!FIELD_EX64(env->msr, MSR, FE0) &&
+!FIELD_EX64(env->msr, MSR, FE1)) ||
!FIELD_EX64(env->msr, MSR, FP)) {
trace_ppc_excp_fp_ignore();
powerpc_reset_excp_state(cpu);
--
2.25.1
[PATCH v2 16/21] target/ppc: Remove msr_ep macro
msr_ep macro hides the usage of env->msr, which is a bad behavior
Substitute it with FIELD_EX64 calls that explicitly use env->msr
as a parameter.
Also, this macro was called in a specific place where it was being
used 'kinda' like a mask: (value >> MSR_EP) & 1) != msr_ep. The setup
to use FIELD_EX64 (calling FIELD macro in cpu.h) also adds a
R_MSR_EP_MASK which is equivalent to (1ull << MSR_EP). So, use
this mask to simplify the code.
Suggested-by: Richard Henderson
Signed-off-by: Víctor Colombo
---
v2: Remove M_MSR_EP and use FIELD_EX64 instead
Signed-off-by: Víctor Colombo
---
target/ppc/cpu.h | 2 +-
target/ppc/helper_regs.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index ff52eef304..9683e6a359 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -363,6 +363,7 @@ FIELD(MSR, EE, MSR_EE, 1)
FIELD(MSR, PR, MSR_PR, 1)
FIELD(MSR, FP, MSR_FP, 1)
FIELD(MSR, ME, MSR_ME, 1)
+FIELD(MSR, EP, MSR_EP, 1)
FIELD(MSR, IR, MSR_IR, 1)
FIELD(MSR, DR, MSR_DR, 1)
FIELD(MSR, DS, MSR_DS, 1)
@@ -485,7 +486,6 @@ FIELD(MSR, LE, MSR_LE, 1)
#endif
#define msr_fe0 ((env->msr >> MSR_FE0) & 1)
#define msr_fe1 ((env->msr >> MSR_FE1) & 1)
-#define msr_ep ((env->msr >> MSR_EP) & 1)
#define msr_ts ((env->msr >> MSR_TS1) & 3)
#define DBCR0_ICMP (1 << 27)
diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
index 8c2b8acb60..1c593686d4 100644
--- a/target/ppc/helper_regs.c
+++ b/target/ppc/helper_regs.c
@@ -241,8 +241,8 @@ int hreg_store_msr(CPUPPCState *env, target_ulong value,
int alter_hv)
/* Swap temporary saved registers with GPRs */
hreg_swap_gpr_tgpr(env);
}
-if (unlikely((value >> MSR_EP) & 1) != msr_ep) {
-env->excp_prefix = ((value >> MSR_EP) & 1) * 0xFFF0;
+if (unlikely(!(value & env->msr & R_MSR_EP_MASK))) {
+env->excp_prefix = FIELD_EX64(env->msr, MSR, EP) * 0xFFF0;
}
/*
* If PR=1 then EE, IR and DR must be 1
--
2.25.1
[PATCH v2 14/21] target/ppc: Remove msr_ir macro
msr_ir macro hides the usage of env->msr, which is a bad behavior
Substitute it with FIELD_EX64 calls that explicitly use env->msr
as a parameter.
Suggested-by: Richard Henderson
Signed-off-by: Víctor Colombo
---
v2: Remove M_MSR_IR and use FIELD_EX64 instead
Signed-off-by: Víctor Colombo
---
target/ppc/cpu.h | 2 +-
target/ppc/helper_regs.c | 2 +-
target/ppc/mmu_common.c | 11 ++-
3 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 8e652691cf..18d41e7af4 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -363,6 +363,7 @@ FIELD(MSR, EE, MSR_EE, 1)
FIELD(MSR, PR, MSR_PR, 1)
FIELD(MSR, FP, MSR_FP, 1)
FIELD(MSR, ME, MSR_ME, 1)
+FIELD(MSR, IR, MSR_IR, 1)
FIELD(MSR, DS, MSR_DS, 1)
FIELD(MSR, LE, MSR_LE, 1)
@@ -484,7 +485,6 @@ FIELD(MSR, LE, MSR_LE, 1)
#define msr_fe0 ((env->msr >> MSR_FE0) & 1)
#define msr_fe1 ((env->msr >> MSR_FE1) & 1)
#define msr_ep ((env->msr >> MSR_EP) & 1)
-#define msr_ir ((env->msr >> MSR_IR) & 1)
#define msr_dr ((env->msr >> MSR_DR) & 1)
#define msr_ts ((env->msr >> MSR_TS1) & 3)
diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
index 88fcc01589..7f3f6ce5c1 100644
--- a/target/ppc/helper_regs.c
+++ b/target/ppc/helper_regs.c
@@ -227,7 +227,7 @@ int hreg_store_msr(CPUPPCState *env, target_ulong value,
int alter_hv)
value &= ~MSR_HVB;
value |= env->msr & MSR_HVB;
}
-if (((value >> MSR_IR) & 1) != msr_ir ||
+if (!(value & env->msr & R_MSR_IR_MASK) ||
((value >> MSR_DR) & 1) != msr_dr) {
cpu_interrupt_exittb(cs);
}
diff --git a/target/ppc/mmu_common.c b/target/ppc/mmu_common.c
index 031bb4493b..30deca0425 100644
--- a/target/ppc/mmu_common.c
+++ b/target/ppc/mmu_common.c
@@ -388,7 +388,7 @@ static int get_segment_6xx_tlb(CPUPPCState *env, mmu_ctx_t
*ctx,
" nip=" TARGET_FMT_lx " lr=" TARGET_FMT_lx
" ir=%d dr=%d pr=%d %d t=%d\n",
eaddr, (int)(eaddr >> 28), sr, env->nip, env->lr,
- (int)msr_ir, (int)msr_dr, pr ? 1 : 0,
+ (int)FIELD_EX64(env->msr, MSR, IR), (int)msr_dr, pr ? 1 : 0,
access_type == MMU_DATA_STORE, type);
pgidx = (eaddr & ~SEGMENT_MASK_256M) >> target_page_bits;
hash = vsid ^ pgidx;
@@ -626,7 +626,8 @@ found_tlb:
}
/* Check the address space */
-if ((access_type == MMU_INST_FETCH ? msr_ir : msr_dr) != (tlb->attr & 1)) {
+if ((access_type == MMU_INST_FETCH ?
+FIELD_EX64(env->msr, MSR, IR) : msr_dr) != (tlb->attr & 1)) {
qemu_log_mask(CPU_LOG_MMU, "%s: AS doesn't match\n", __func__);
return -1;
}
@@ -839,7 +840,7 @@ found_tlb:
if (access_type == MMU_INST_FETCH) {
/* There is no way to fetch code using epid load */
assert(!use_epid);
-as = msr_ir;
+as = FIELD_EX64(env->msr, MSR, IR);
}
if (as != ((tlb->mas1 & MAS1_TS) >> MAS1_TS_SHIFT)) {
@@ -1169,7 +1170,7 @@ int get_physical_address_wtlb(CPUPPCState *env, mmu_ctx_t
*ctx,
int mmu_idx)
{
int ret = -1;
-bool real_mode = (type == ACCESS_CODE && msr_ir == 0)
+bool real_mode = (type == ACCESS_CODE && !FIELD_EX64(env->msr, MSR, IR))
|| (type != ACCESS_CODE && msr_dr == 0);
switch (env->mmu_model) {
@@ -1231,7 +1232,7 @@ static void booke206_update_mas_tlb_miss(CPUPPCState
*env, target_ulong address,
bool use_epid = mmubooke206_get_as(env, mmu_idx, , , );
if (access_type == MMU_INST_FETCH) {
-as = msr_ir;
+as = FIELD_EX64(env->msr, MSR, IR);
}
env->spr[SPR_BOOKE_MAS0] = env->spr[SPR_BOOKE_MAS4] & MAS4_TLBSELD_MASK;
env->spr[SPR_BOOKE_MAS1] = env->spr[SPR_BOOKE_MAS4] & MAS4_TSIZED_MASK;
--
2.25.1
[PATCH v2 15/21] target/ppc: Remove msr_dr macro
msr_dr macro hides the usage of env->msr, which is a bad behavior
Substitute it with FIELD_EX64 calls that explicitly use env->msr
as a parameter.
Suggested-by: Richard Henderson
Signed-off-by: Víctor Colombo
---
v2: Remove M_MSR_DR and use FIELD_EX64 instead
Signed-off-by: Víctor Colombo
---
target/ppc/cpu.h | 2 +-
target/ppc/helper_regs.c | 2 +-
target/ppc/mmu_common.c | 10 ++
3 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 18d41e7af4..ff52eef304 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -364,6 +364,7 @@ FIELD(MSR, PR, MSR_PR, 1)
FIELD(MSR, FP, MSR_FP, 1)
FIELD(MSR, ME, MSR_ME, 1)
FIELD(MSR, IR, MSR_IR, 1)
+FIELD(MSR, DR, MSR_DR, 1)
FIELD(MSR, DS, MSR_DS, 1)
FIELD(MSR, LE, MSR_LE, 1)
@@ -485,7 +486,6 @@ FIELD(MSR, LE, MSR_LE, 1)
#define msr_fe0 ((env->msr >> MSR_FE0) & 1)
#define msr_fe1 ((env->msr >> MSR_FE1) & 1)
#define msr_ep ((env->msr >> MSR_EP) & 1)
-#define msr_dr ((env->msr >> MSR_DR) & 1)
#define msr_ts ((env->msr >> MSR_TS1) & 3)
#define DBCR0_ICMP (1 << 27)
diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
index 7f3f6ce5c1..8c2b8acb60 100644
--- a/target/ppc/helper_regs.c
+++ b/target/ppc/helper_regs.c
@@ -228,7 +228,7 @@ int hreg_store_msr(CPUPPCState *env, target_ulong value,
int alter_hv)
value |= env->msr & MSR_HVB;
}
if (!(value & env->msr & R_MSR_IR_MASK) ||
-((value >> MSR_DR) & 1) != msr_dr) {
+!(value & env->msr & R_MSR_DR_MASK)) {
cpu_interrupt_exittb(cs);
}
if ((env->mmu_model == POWERPC_MMU_BOOKE ||
diff --git a/target/ppc/mmu_common.c b/target/ppc/mmu_common.c
index 30deca0425..89107a6af2 100644
--- a/target/ppc/mmu_common.c
+++ b/target/ppc/mmu_common.c
@@ -388,7 +388,8 @@ static int get_segment_6xx_tlb(CPUPPCState *env, mmu_ctx_t
*ctx,
" nip=" TARGET_FMT_lx " lr=" TARGET_FMT_lx
" ir=%d dr=%d pr=%d %d t=%d\n",
eaddr, (int)(eaddr >> 28), sr, env->nip, env->lr,
- (int)FIELD_EX64(env->msr, MSR, IR), (int)msr_dr, pr ? 1 : 0,
+ (int)FIELD_EX64(env->msr, MSR, IR),
+ (int)FIELD_EX64(env->msr, MSR, DR), pr ? 1 : 0,
access_type == MMU_DATA_STORE, type);
pgidx = (eaddr & ~SEGMENT_MASK_256M) >> target_page_bits;
hash = vsid ^ pgidx;
@@ -627,7 +628,8 @@ found_tlb:
/* Check the address space */
if ((access_type == MMU_INST_FETCH ?
-FIELD_EX64(env->msr, MSR, IR) : msr_dr) != (tlb->attr & 1)) {
+FIELD_EX64(env->msr, MSR, IR) :
+FIELD_EX64(env->msr, MSR, DR)) != (tlb->attr & 1)) {
qemu_log_mask(CPU_LOG_MMU, "%s: AS doesn't match\n", __func__);
return -1;
}
@@ -1170,8 +1172,8 @@ int get_physical_address_wtlb(CPUPPCState *env, mmu_ctx_t
*ctx,
int mmu_idx)
{
int ret = -1;
-bool real_mode = (type == ACCESS_CODE && !FIELD_EX64(env->msr, MSR, IR))
-|| (type != ACCESS_CODE && msr_dr == 0);
+bool real_mode = (type == ACCESS_CODE && !FIELD_EX64(env->msr, MSR, IR)) ||
+ (type != ACCESS_CODE && !FIELD_EX64(env->msr, MSR, DR));
switch (env->mmu_model) {
case POWERPC_MMU_SOFT_6xx:
--
2.25.1
Re: [PATCH v2 00/21] target/ppc: Remove hidden usages of *env
On 5/2/22 16:39, Víctor Colombo wrote: By running the grep command `git grep -nr 'define \(fpscr\|msr\)_[a-z0-9]\+\>'` we can find multiple macros that use `env->fpscr` and `env->msr` but doesn't take *env as a parameter. Richard Henderson said [1] that these macros hiding the usage of *env "are evil". This patch series remove them and substitute with an explicit usage of *env by using registerfields API. Patch 20 (target/ppc: Add unused msr bits FIELDs) declares unused FIELDs, the same that were removed in patch 02 (target/ppc: Remove unused msr_* macros). I did that to keep the changes consistent with what was already present before. Patch 21 (target/ppc: Change MSR_* to follow POWER ISA numbering convention) changes the MSR_* bit number to match POWER ISA by adding a new macro to 'invert' the ordering. (added in v2) [1]: https://lists.gnu.org/archive/html/qemu-ppc/2021-11/msg00280.html Patches requiring review: Patch 3 and after Hello everyone, thanks for your kind reviews in v1, What do you think of this new approach I did for v2? It looks real good. I hope we can queue it for the next PR. Thanks, C. v2: - Abandon the ideia to add an M_MSR_* macro - Instead, use registerfields API as suggested by Richard - Add patch 21 to invert MSR_* values to match ISA ordering Víctor Colombo (21): target/ppc: Remove fpscr_* macros from cpu.h target/ppc: Remove unused msr_* macros target/ppc: Remove msr_pr macro target/ppc: Remove msr_le macro target/ppc: Remove msr_ds macro target/ppc: Remove msr_ile macro target/ppc: Remove msr_ee macro target/ppc: Remove msr_ce macro target/ppc: Remove msr_pow macro target/ppc: Remove msr_me macro target/ppc: Remove msr_gs macro target/ppc: Remove msr_fp macro target/ppc: Remove msr_me macro target/ppc: Remove msr_ir macro target/ppc: Remove msr_dr macro target/ppc: Remove msr_ep macro target/ppc: Remove msr_fe0 and msr_fe1 macros target/ppc: Remove msr_ts macro target/ppc: Remove msr_hv macro target/ppc: Add unused msr bits FIELDs target/ppc: Change MSR_* to follow POWER ISA numbering convention hw/ppc/pegasos2.c| 2 +- hw/ppc/spapr.c | 2 +- target/ppc/cpu.c | 2 +- target/ppc/cpu.h | 214 ++- target/ppc/cpu_init.c| 23 +++-- target/ppc/excp_helper.c | 66 +++- target/ppc/fpu_helper.c | 28 ++--- target/ppc/gdbstub.c | 2 +- target/ppc/helper_regs.c | 12 +-- target/ppc/kvm.c | 7 +- target/ppc/machine.c | 2 +- target/ppc/mem_helper.c | 23 +++-- target/ppc/misc_helper.c | 2 +- target/ppc/mmu-radix64.c | 11 +- target/ppc/mmu_common.c | 40 target/ppc/mmu_helper.c | 6 +- 16 files changed, 225 insertions(+), 217 deletions(-)
[PATCH v2 11/21] target/ppc: Remove msr_gs macro
msr_gs macro hides the usage of env->msr, which is a bad behavior
Substitute it with FIELD_EX64 calls that explicitly use env->msr
as a parameter.
Suggested-by: Richard Henderson
Signed-off-by: Víctor Colombo
---
v2: Remove M_MSR_GS and use FIELD_EX64 instead
Signed-off-by: Víctor Colombo
---
target/ppc/cpu.h | 2 +-
target/ppc/helper_regs.c | 2 +-
target/ppc/mmu_helper.c | 4 ++--
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 059a00ed65..4b69cd666d 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -354,6 +354,7 @@ typedef enum {
#define MSR_RI 1 /* Recoverable interrupt1*/
#define MSR_LE 0 /* Little-endian mode 1 hflags */
+FIELD(MSR, GS, MSR_GS, 1)
FIELD(MSR, POW, MSR_POW, 1)
FIELD(MSR, CE, MSR_CE, 1)
FIELD(MSR, ILE, MSR_ILE, 1)
@@ -479,7 +480,6 @@ FIELD(MSR, LE, MSR_LE, 1)
#define msr_hv (0)
#endif
#define msr_cm ((env->msr >> MSR_CM) & 1)
-#define msr_gs ((env->msr >> MSR_GS) & 1)
#define msr_fp ((env->msr >> MSR_FP) & 1)
#define msr_fe0 ((env->msr >> MSR_FE0) & 1)
#define msr_fe1 ((env->msr >> MSR_FE1) & 1)
diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
index 940f0207a0..88fcc01589 100644
--- a/target/ppc/helper_regs.c
+++ b/target/ppc/helper_regs.c
@@ -233,7 +233,7 @@ int hreg_store_msr(CPUPPCState *env, target_ulong value,
int alter_hv)
}
if ((env->mmu_model == POWERPC_MMU_BOOKE ||
env->mmu_model == POWERPC_MMU_BOOKE206) &&
-((value >> MSR_GS) & 1) != msr_gs) {
+!(value & env->msr & R_MSR_GS_MASK)) {
cpu_interrupt_exittb(cs);
}
if (unlikely((env->flags & POWERPC_FLAG_TGPR) &&
diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
index 142a717255..5bb5c71038 100644
--- a/target/ppc/mmu_helper.c
+++ b/target/ppc/mmu_helper.c
@@ -935,7 +935,7 @@ void helper_booke206_tlbwe(CPUPPCState *env)
}
if (((env->spr[SPR_BOOKE_MAS0] & MAS0_ATSEL) == MAS0_ATSEL_LRAT) &&
-!msr_gs) {
+!FIELD_EX64(env->msr, MSR, GS)) {
/* XXX we don't support direct LRAT setting yet */
fprintf(stderr, "cpu: don't support LRAT setting yet\n");
return;
@@ -962,7 +962,7 @@ void helper_booke206_tlbwe(CPUPPCState *env)
POWERPC_EXCP_INVAL_INVAL, GETPC());
}
-if (msr_gs) {
+if (FIELD_EX64(env->msr, MSR, GS)) {
cpu_abort(env_cpu(env), "missing HV implementation\n");
}
--
2.25.1
[PATCH v2 13/21] target/ppc: Remove msr_me macro
msr_me macro hides the usage of env->msr, which is a bad behavior
Substitute it with FIELD_EX64 calls that explicitly use env->msr
as a parameter.
Suggested-by: Richard Henderson
Signed-off-by: Víctor Colombo
---
v2: Remove M_MSR_CM and use FIELD_EX64 instead
Signed-off-by: Víctor Colombo
---
target/ppc/cpu.h| 2 +-
target/ppc/mmu_common.c | 2 +-
target/ppc/mmu_helper.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 294c61c9dc..8e652691cf 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -354,6 +354,7 @@ typedef enum {
#define MSR_RI 1 /* Recoverable interrupt1*/
#define MSR_LE 0 /* Little-endian mode 1 hflags */
+FIELD(MSR, CM, MSR_CM, 1)
FIELD(MSR, GS, MSR_GS, 1)
FIELD(MSR, POW, MSR_POW, 1)
FIELD(MSR, CE, MSR_CE, 1)
@@ -480,7 +481,6 @@ FIELD(MSR, LE, MSR_LE, 1)
#else
#define msr_hv (0)
#endif
-#define msr_cm ((env->msr >> MSR_CM) & 1)
#define msr_fe0 ((env->msr >> MSR_FE0) & 1)
#define msr_fe1 ((env->msr >> MSR_FE1) & 1)
#define msr_ep ((env->msr >> MSR_EP) & 1)
diff --git a/target/ppc/mmu_common.c b/target/ppc/mmu_common.c
index 7e77b9b84a..031bb4493b 100644
--- a/target/ppc/mmu_common.c
+++ b/target/ppc/mmu_common.c
@@ -692,7 +692,7 @@ int ppcmas_tlb_check(CPUPPCState *env, ppcmas_tlb_t *tlb,
hwaddr mask;
uint32_t tlb_pid;
-if (!msr_cm) {
+if (!FIELD_EX64(env->msr, MSR, CM)) {
/* In 32bit mode we can only address 32bit EAs */
address = (uint32_t)address;
}
diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
index 5bb5c71038..15239dc95b 100644
--- a/target/ppc/mmu_helper.c
+++ b/target/ppc/mmu_helper.c
@@ -1003,7 +1003,7 @@ void helper_booke206_tlbwe(CPUPPCState *env)
/* Add a mask for page attributes */
mask |= MAS2_ACM | MAS2_VLE | MAS2_W | MAS2_I | MAS2_M | MAS2_G | MAS2_E;
-if (!msr_cm) {
+if (!FIELD_EX64(env->msr, MSR, CM)) {
/*
* Executing a tlbwe instruction in 32-bit mode will set bits
* 0:31 of the TLB EPN field to zero.
--
2.25.1
[PATCH v2 10/21] target/ppc: Remove msr_me macro
msr_me macro hides the usage of env->msr, which is a bad behavior
Substitute it with FIELD_EX64 calls that explicitly use env->msr
as a parameter.
Suggested-by: Richard Henderson
Signed-off-by: Víctor Colombo
---
v2: Remove M_MSR_ME and use FIELD_EX64 instead
Signed-off-by: Víctor Colombo
---
target/ppc/cpu.h | 2 +-
target/ppc/excp_helper.c | 12 ++--
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index f283292863..059a00ed65 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -359,6 +359,7 @@ FIELD(MSR, CE, MSR_CE, 1)
FIELD(MSR, ILE, MSR_ILE, 1)
FIELD(MSR, EE, MSR_EE, 1)
FIELD(MSR, PR, MSR_PR, 1)
+FIELD(MSR, ME, MSR_ME, 1)
FIELD(MSR, DS, MSR_DS, 1)
FIELD(MSR, LE, MSR_LE, 1)
@@ -480,7 +481,6 @@ FIELD(MSR, LE, MSR_LE, 1)
#define msr_cm ((env->msr >> MSR_CM) & 1)
#define msr_gs ((env->msr >> MSR_GS) & 1)
#define msr_fp ((env->msr >> MSR_FP) & 1)
-#define msr_me ((env->msr >> MSR_ME) & 1)
#define msr_fe0 ((env->msr >> MSR_FE0) & 1)
#define msr_fe1 ((env->msr >> MSR_FE1) & 1)
#define msr_ep ((env->msr >> MSR_EP) & 1)
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index ee63641dd0..e254ae806c 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -444,7 +444,7 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
srr1 = SPR_40x_SRR3;
break;
case POWERPC_EXCP_MCHECK:/* Machine check exception */
-if (msr_me == 0) {
+if (!FIELD_EX64(env->msr, MSR, ME)) {
/*
* Machine check exception is not enabled. Enter
* checkstop state.
@@ -575,7 +575,7 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp)
case POWERPC_EXCP_CRITICAL:/* Critical input */
break;
case POWERPC_EXCP_MCHECK:/* Machine check exception */
-if (msr_me == 0) {
+if (!FIELD_EX64(env->msr, MSR, ME)) {
/*
* Machine check exception is not enabled. Enter
* checkstop state.
@@ -748,7 +748,7 @@ static void powerpc_excp_7xx(PowerPCCPU *cpu, int excp)
switch (excp) {
case POWERPC_EXCP_MCHECK:/* Machine check exception */
-if (msr_me == 0) {
+if (!FIELD_EX64(env->msr, MSR, ME)) {
/*
* Machine check exception is not enabled. Enter
* checkstop state.
@@ -933,7 +933,7 @@ static void powerpc_excp_74xx(PowerPCCPU *cpu, int excp)
switch (excp) {
case POWERPC_EXCP_MCHECK:/* Machine check exception */
-if (msr_me == 0) {
+if (!FIELD_EX64(env->msr, MSR, ME)) {
/*
* Machine check exception is not enabled. Enter
* checkstop state.
@@ -1128,7 +1128,7 @@ static void powerpc_excp_booke(PowerPCCPU *cpu, int excp)
srr1 = SPR_BOOKE_CSRR1;
break;
case POWERPC_EXCP_MCHECK:/* Machine check exception */
-if (msr_me == 0) {
+if (!FIELD_EX64(env->msr, MSR, ME)) {
/*
* Machine check exception is not enabled. Enter
* checkstop state.
@@ -1366,7 +1366,7 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
switch (excp) {
case POWERPC_EXCP_MCHECK:/* Machine check exception */
-if (msr_me == 0) {
+if (!FIELD_EX64(env->msr, MSR, ME)) {
/*
* Machine check exception is not enabled. Enter
* checkstop state.
--
2.25.1
[PATCH v2 12/21] target/ppc: Remove msr_fp macro
msr_fp macro hides the usage of env->msr, which is a bad behavior
Substitute it with FIELD_EX64 calls that explicitly use env->msr
as a parameter.
Suggested-by: Richard Henderson
Signed-off-by: Víctor Colombo
---
v2: Remove M_MSR_FP and use FIELD_EX64 instead
Signed-off-by: Víctor Colombo
---
target/ppc/cpu.h | 2 +-
target/ppc/excp_helper.c | 18 --
2 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 4b69cd666d..294c61c9dc 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -360,6 +360,7 @@ FIELD(MSR, CE, MSR_CE, 1)
FIELD(MSR, ILE, MSR_ILE, 1)
FIELD(MSR, EE, MSR_EE, 1)
FIELD(MSR, PR, MSR_PR, 1)
+FIELD(MSR, FP, MSR_FP, 1)
FIELD(MSR, ME, MSR_ME, 1)
FIELD(MSR, DS, MSR_DS, 1)
FIELD(MSR, LE, MSR_LE, 1)
@@ -480,7 +481,6 @@ FIELD(MSR, LE, MSR_LE, 1)
#define msr_hv (0)
#endif
#define msr_cm ((env->msr >> MSR_CM) & 1)
-#define msr_fp ((env->msr >> MSR_FP) & 1)
#define msr_fe0 ((env->msr >> MSR_FE0) & 1)
#define msr_fe1 ((env->msr >> MSR_FE1) & 1)
#define msr_ep ((env->msr >> MSR_EP) & 1)
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index e254ae806c..30baad0489 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -478,7 +478,8 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
case POWERPC_EXCP_PROGRAM: /* Program exception*/
switch (env->error_code & ~0xF) {
case POWERPC_EXCP_FP:
-if ((msr_fe0 == 0 && msr_fe1 == 0) || msr_fp == 0) {
+if ((msr_fe0 == 0 && msr_fe1 == 0) ||
+!FIELD_EX64(env->msr, MSR, FP)) {
trace_ppc_excp_fp_ignore();
powerpc_reset_excp_state(cpu);
return;
@@ -615,7 +616,8 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp)
case POWERPC_EXCP_PROGRAM: /* Program exception*/
switch (env->error_code & ~0xF) {
case POWERPC_EXCP_FP:
-if ((msr_fe0 == 0 && msr_fe1 == 0) || msr_fp == 0) {
+if ((msr_fe0 == 0 && msr_fe1 == 0) ||
+!FIELD_EX64(env->msr, MSR, FP)) {
trace_ppc_excp_fp_ignore();
powerpc_reset_excp_state(cpu);
return;
@@ -788,7 +790,8 @@ static void powerpc_excp_7xx(PowerPCCPU *cpu, int excp)
case POWERPC_EXCP_PROGRAM: /* Program exception*/
switch (env->error_code & ~0xF) {
case POWERPC_EXCP_FP:
-if ((msr_fe0 == 0 && msr_fe1 == 0) || msr_fp == 0) {
+if ((msr_fe0 == 0 && msr_fe1 == 0) ||
+!FIELD_EX64(env->msr, MSR, FP)) {
trace_ppc_excp_fp_ignore();
powerpc_reset_excp_state(cpu);
return;
@@ -973,7 +976,8 @@ static void powerpc_excp_74xx(PowerPCCPU *cpu, int excp)
case POWERPC_EXCP_PROGRAM: /* Program exception*/
switch (env->error_code & ~0xF) {
case POWERPC_EXCP_FP:
-if ((msr_fe0 == 0 && msr_fe1 == 0) || msr_fp == 0) {
+if ((msr_fe0 == 0 && msr_fe1 == 0) ||
+!FIELD_EX64(env->msr, MSR, FP)) {
trace_ppc_excp_fp_ignore();
powerpc_reset_excp_state(cpu);
return;
@@ -1171,7 +1175,8 @@ static void powerpc_excp_booke(PowerPCCPU *cpu, int excp)
case POWERPC_EXCP_PROGRAM: /* Program exception*/
switch (env->error_code & ~0xF) {
case POWERPC_EXCP_FP:
-if ((msr_fe0 == 0 && msr_fe1 == 0) || msr_fp == 0) {
+if ((msr_fe0 == 0 && msr_fe1 == 0) ||
+!FIELD_EX64(env->msr, MSR, FP)) {
trace_ppc_excp_fp_ignore();
powerpc_reset_excp_state(cpu);
return;
@@ -1434,7 +1439,8 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
case POWERPC_EXCP_PROGRAM: /* Program exception*/
switch (env->error_code & ~0xF) {
case POWERPC_EXCP_FP:
-if ((msr_fe0 == 0 && msr_fe1 == 0) || msr_fp == 0) {
+if ((msr_fe0 == 0 && msr_fe1 == 0) ||
+!FIELD_EX64(env->msr, MSR, FP)) {
trace_ppc_excp_fp_ignore();
powerpc_reset_excp_state(cpu);
return;
--
2.25.1
[PATCH v2 06/21] target/ppc: Remove msr_ile macro
msr_ile macro hides the usage of env->msr, which is a bad behavior
Substitute it with FIELD_EX64 calls that explicitly use env->msr
as a parameter.
Suggested-by: Richard Henderson
Signed-off-by: Víctor Colombo
---
v2: Remove M_MSR_ILE and use FIELD_EX64 instead
Signed-off-by: Víctor Colombo
---
target/ppc/cpu.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 8aff6f1faf..d4a0d7e082 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -354,6 +354,7 @@ typedef enum {
#define MSR_RI 1 /* Recoverable interrupt1*/
#define MSR_LE 0 /* Little-endian mode 1 hflags */
+FIELD(MSR, ILE, MSR_ILE, 1)
FIELD(MSR, PR, MSR_PR, 1)
FIELD(MSR, DS, MSR_DS, 1)
FIELD(MSR, LE, MSR_LE, 1)
@@ -477,7 +478,6 @@ FIELD(MSR, LE, MSR_LE, 1)
#define msr_gs ((env->msr >> MSR_GS) & 1)
#define msr_pow ((env->msr >> MSR_POW) & 1)
#define msr_ce ((env->msr >> MSR_CE) & 1)
-#define msr_ile ((env->msr >> MSR_ILE) & 1)
#define msr_ee ((env->msr >> MSR_EE) & 1)
#define msr_fp ((env->msr >> MSR_FP) & 1)
#define msr_me ((env->msr >> MSR_ME) & 1)
@@ -2678,7 +2678,7 @@ static inline bool
ppc_interrupts_little_endian(PowerPCCPU *cpu, bool hv)
} else if (pcc->lpcr_mask & LPCR_ILE) {
ile = !!(env->spr[SPR_LPCR] & LPCR_ILE);
} else {
-ile = !!(msr_ile);
+ile = FIELD_EX64(env->msr, MSR, ILE);
}
return ile;
--
2.25.1
[PATCH v2 09/21] target/ppc: Remove msr_pow macro
msr_pow macro hides the usage of env->msr, which is a bad behavior
Substitute it with FIELD_EX64 calls that explicitly use env->msr
as a parameter.
Suggested-by: Richard Henderson
Signed-off-by: Víctor Colombo
---
v2: Remove M_MSR_POW and use FIELD_EX64 instead
Signed-off-by: Víctor Colombo
---
target/ppc/cpu.h | 2 +-
target/ppc/excp_helper.c | 12 ++--
target/ppc/helper_regs.c | 2 +-
3 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 65cb457e6f..f283292863 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -354,6 +354,7 @@ typedef enum {
#define MSR_RI 1 /* Recoverable interrupt1*/
#define MSR_LE 0 /* Little-endian mode 1 hflags */
+FIELD(MSR, POW, MSR_POW, 1)
FIELD(MSR, CE, MSR_CE, 1)
FIELD(MSR, ILE, MSR_ILE, 1)
FIELD(MSR, EE, MSR_EE, 1)
@@ -478,7 +479,6 @@ FIELD(MSR, LE, MSR_LE, 1)
#endif
#define msr_cm ((env->msr >> MSR_CM) & 1)
#define msr_gs ((env->msr >> MSR_GS) & 1)
-#define msr_pow ((env->msr >> MSR_POW) & 1)
#define msr_fp ((env->msr >> MSR_FP) & 1)
#define msr_me ((env->msr >> MSR_ME) & 1)
#define msr_fe0 ((env->msr >> MSR_FE0) & 1)
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index ca80c1ed63..ee63641dd0 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -661,7 +661,7 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp)
case POWERPC_EXCP_ITLB: /* Instruction TLB error*/
break;
case POWERPC_EXCP_RESET: /* System reset exception */
-if (msr_pow) {
+if (FIELD_EX64(env->msr, MSR, POW)) {
cpu_abort(cs, "Trying to deliver power-saving system reset "
"exception %d with no HV support\n", excp);
}
@@ -853,7 +853,7 @@ static void powerpc_excp_7xx(PowerPCCPU *cpu, int excp)
case POWERPC_EXCP_DECR: /* Decrementer exception*/
break;
case POWERPC_EXCP_RESET: /* System reset exception */
-if (msr_pow) {
+if (FIELD_EX64(env->msr, MSR, POW)) {
cpu_abort(cs, "Trying to deliver power-saving system reset "
"exception %d with no HV support\n", excp);
}
@@ -1038,7 +1038,7 @@ static void powerpc_excp_74xx(PowerPCCPU *cpu, int excp)
case POWERPC_EXCP_DECR: /* Decrementer exception*/
break;
case POWERPC_EXCP_RESET: /* System reset exception */
-if (msr_pow) {
+if (FIELD_EX64(env->msr, MSR, POW)) {
cpu_abort(cs, "Trying to deliver power-saving system reset "
"exception %d with no HV support\n", excp);
}
@@ -1248,7 +1248,7 @@ static void powerpc_excp_booke(PowerPCCPU *cpu, int excp)
env->spr[SPR_BOOKE_ESR] = ESR_SPV;
break;
case POWERPC_EXCP_RESET: /* System reset exception */
-if (msr_pow) {
+if (FIELD_EX64(env->msr, MSR, POW)) {
cpu_abort(cs, "Trying to deliver power-saving system reset "
"exception %d with no HV support\n", excp);
}
@@ -1507,7 +1507,7 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
break;
case POWERPC_EXCP_RESET: /* System reset exception */
/* A power-saving exception sets ME, otherwise it is unchanged */
-if (msr_pow) {
+if (FIELD_EX64(env->msr, MSR, POW)) {
/* indicate that we resumed from power save mode */
msr |= 0x1;
new_msr |= ((target_ulong)1 << MSR_ME);
@@ -1519,7 +1519,7 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
*/
new_msr |= (target_ulong)MSR_HVB;
} else {
-if (msr_pow) {
+if (FIELD_EX64(env->msr, MSR, POW)) {
cpu_abort(cs, "Trying to deliver power-saving system reset "
"exception %d with no HV support\n", excp);
}
diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
index 9a691d6833..940f0207a0 100644
--- a/target/ppc/helper_regs.c
+++ b/target/ppc/helper_regs.c
@@ -261,7 +261,7 @@ int hreg_store_msr(CPUPPCState *env, target_ulong value,
int alter_hv)
env->msr = value;
hreg_compute_hflags(env);
#if !defined(CONFIG_USER_ONLY)
-if (unlikely(msr_pow == 1)) {
+if (unlikely(FIELD_EX64(env->msr, MSR, POW))) {
if (!env->pending_interrupts && (*env->check_pow)(env)) {
cs->halted = 1;
excp = EXCP_HALTED;
--
2.25.1
Re: [Patch 1/3] hw/vfio/pci: fix vfio_pci_hot_reset_result trace point
Hi Alex, On 5/2/22 16:35, Alex Williamson wrote: > On Mon, 2 May 2022 02:42:21 -0700 > Yi Liu wrote: > >> From: Eric Auger >> >> Properly output the errno string. > More explanation please, why is it broken and how does this fix it? > Thanks, "%m" format specifier is not interpreted by the trace infrastructure and thus "%m" is output instead of the actual errno string. Fix it by outputting strerror(errno). Thanks Eric > > Alex > >> Signed-off-by: Eric Auger >> Signed-off-by: Yi Liu >> --- >> hw/vfio/pci.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c >> index 9fd9faee1d..4a66376be6 100644 >> --- a/hw/vfio/pci.c >> +++ b/hw/vfio/pci.c >> @@ -2337,7 +2337,7 @@ static int vfio_pci_hot_reset(VFIOPCIDevice *vdev, >> bool single) >> g_free(reset); >> >> trace_vfio_pci_hot_reset_result(vdev->vbasedev.name, >> -ret ? "%m" : "Success"); >> +ret ? strerror(errno) : "Success"); >> >> out: >> /* Re-enable INTx on affected devices */
[PATCH v2 04/21] target/ppc: Remove msr_le macro
msr_le macro hides the usage of env->msr, which is a bad behavior
Substitute it with FIELD_EX64 calls that explicitly use env->msr
as a parameter.
Suggested-by: Richard Henderson
Signed-off-by: Víctor Colombo
---
v2: Remove M_MSR_LE and use FIELD_EX64 instead
Signed-off-by: Víctor Colombo
---
target/ppc/cpu.h| 2 +-
target/ppc/cpu_init.c | 2 +-
target/ppc/gdbstub.c| 2 +-
target/ppc/mem_helper.c | 16
4 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 21d1f14381..932c5f4bdd 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -355,6 +355,7 @@ typedef enum {
#define MSR_LE 0 /* Little-endian mode 1 hflags */
FIELD(MSR, PR, MSR_PR, 1)
+FIELD(MSR, LE, MSR_LE, 1)
/* PMU bits */
#define MMCR0_FC PPC_BIT(32) /* Freeze Counters */
@@ -485,7 +486,6 @@ FIELD(MSR, PR, MSR_PR, 1)
#define msr_ir ((env->msr >> MSR_IR) & 1)
#define msr_dr ((env->msr >> MSR_DR) & 1)
#define msr_ds ((env->msr >> MSR_DS) & 1)
-#define msr_le ((env->msr >> MSR_LE) & 1)
#define msr_ts ((env->msr >> MSR_TS1) & 3)
#define DBCR0_ICMP (1 << 27)
diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index ac16a64846..0c6b83406e 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -7210,7 +7210,7 @@ static bool ppc_cpu_is_big_endian(CPUState *cs)
cpu_synchronize_state(cs);
-return !msr_le;
+return !FIELD_EX64(env->msr, MSR, LE);
}
#ifdef CONFIG_TCG
diff --git a/target/ppc/gdbstub.c b/target/ppc/gdbstub.c
index 1252429a2a..1a0b9ca82c 100644
--- a/target/ppc/gdbstub.c
+++ b/target/ppc/gdbstub.c
@@ -95,7 +95,7 @@ static int ppc_gdb_register_len(int n)
void ppc_maybe_bswap_register(CPUPPCState *env, uint8_t *mem_buf, int len)
{
#ifndef CONFIG_USER_ONLY
-if (!msr_le) {
+if (!FIELD_EX64(env->msr, MSR, LE)) {
/* do nothing */
} else if (len == 4) {
bswap32s((uint32_t *)mem_buf);
diff --git a/target/ppc/mem_helper.c b/target/ppc/mem_helper.c
index fba7f84b7a..9af135e88e 100644
--- a/target/ppc/mem_helper.c
+++ b/target/ppc/mem_helper.c
@@ -33,9 +33,9 @@
static inline bool needs_byteswap(const CPUPPCState *env)
{
#if TARGET_BIG_ENDIAN
- return msr_le;
+ return FIELD_EX64(env->msr, MSR, LE);
#else
- return !msr_le;
+ return !FIELD_EX64(env->msr, MSR, LE);
#endif
}
@@ -470,8 +470,8 @@ uint32_t helper_stqcx_be_parallel(CPUPPCState *env,
target_ulong addr,
#endif
/*
- * We use msr_le to determine index ordering in a vector. However,
- * byteswapping is not simply controlled by msr_le. We also need to
+ * We use MSR_LE to determine index ordering in a vector. However,
+ * byteswapping is not simply controlled by MSR_LE. We also need to
* take into account endianness of the target. This is done for the
* little-endian PPC64 user-mode target.
*/
@@ -484,7 +484,7 @@ uint32_t helper_stqcx_be_parallel(CPUPPCState *env,
target_ulong addr,
int adjust = HI_IDX * (n_elems - 1);\
int sh = sizeof(r->element[0]) >> 1;\
int index = (addr & 0xf) >> sh; \
-if (msr_le) { \
+if (FIELD_EX64(env->msr, MSR, LE)) {\
index = n_elems - index - 1;\
} \
\
@@ -511,7 +511,7 @@ LVE(lvewx, cpu_ldl_data_ra, bswap32, u32)
int adjust = HI_IDX * (n_elems - 1);\
int sh = sizeof(r->element[0]) >> 1;\
int index = (addr & 0xf) >> sh; \
-if (msr_le) { \
+if (FIELD_EX64(env->msr, MSR, LE)) {\
index = n_elems - index - 1;\
} \
\
@@ -545,7 +545,7 @@ void helper_##name(CPUPPCState *env, target_ulong addr,
\
t.s128 = int128_zero(); \
if (nb) { \
nb = (nb >= 16) ? 16 : nb; \
-if (msr_le && !lj) {\
+if (FIELD_EX64(env->msr, MSR, LE) && !lj) { \
for (i = 16; i > 16 - nb; i--) {\
t.VsrB(i - 1) = cpu_ldub_data_ra(env, addr, GETPC()); \
addr = addr_add(env, addr, 1); \
@@ -576,7 +576,7 @@ void helper_##name(CPUPPCState *env, target_ulong addr,
\
}
[PATCH v2 07/21] target/ppc: Remove msr_ee macro
msr_ee macro hides the usage of env->msr, which is a bad behavior
Substitute it with FIELD_EX64 calls that explicitly use env->msr
as a parameter.
Suggested-by: Richard Henderson
Signed-off-by: Víctor Colombo
---
v2: Remove M_MSR_EE and use FIELD_EX64 instead
Signed-off-by: Víctor Colombo
---
target/ppc/cpu.h | 2 +-
target/ppc/cpu_init.c| 15 ++-
target/ppc/excp_helper.c | 2 +-
target/ppc/kvm.c | 3 ++-
4 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index d4a0d7e082..51db67ff92 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -355,6 +355,7 @@ typedef enum {
#define MSR_LE 0 /* Little-endian mode 1 hflags */
FIELD(MSR, ILE, MSR_ILE, 1)
+FIELD(MSR, EE, MSR_EE, 1)
FIELD(MSR, PR, MSR_PR, 1)
FIELD(MSR, DS, MSR_DS, 1)
FIELD(MSR, LE, MSR_LE, 1)
@@ -478,7 +479,6 @@ FIELD(MSR, LE, MSR_LE, 1)
#define msr_gs ((env->msr >> MSR_GS) & 1)
#define msr_pow ((env->msr >> MSR_POW) & 1)
#define msr_ce ((env->msr >> MSR_CE) & 1)
-#define msr_ee ((env->msr >> MSR_EE) & 1)
#define msr_fp ((env->msr >> MSR_FP) & 1)
#define msr_me ((env->msr >> MSR_ME) & 1)
#define msr_fe0 ((env->msr >> MSR_FE0) & 1)
diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index 0c6b83406e..10e7c41bc9 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -5949,7 +5949,8 @@ static bool cpu_has_work_POWER7(CPUState *cs)
}
return false;
} else {
-return msr_ee && (cs->interrupt_request & CPU_INTERRUPT_HARD);
+return FIELD_EX64(env->msr, MSR, EE) &&
+ (cs->interrupt_request & CPU_INTERRUPT_HARD);
}
}
@@ -6120,7 +6121,8 @@ static bool cpu_has_work_POWER8(CPUState *cs)
}
return false;
} else {
-return msr_ee && (cs->interrupt_request & CPU_INTERRUPT_HARD);
+return FIELD_EX64(env->msr, MSR, EE) &&
+ (cs->interrupt_request & CPU_INTERRUPT_HARD);
}
}
@@ -6337,7 +6339,8 @@ static bool cpu_has_work_POWER9(CPUState *cs)
}
return false;
} else {
-return msr_ee && (cs->interrupt_request & CPU_INTERRUPT_HARD);
+return FIELD_EX64(env->msr, MSR, EE) &&
+ (cs->interrupt_request & CPU_INTERRUPT_HARD);
}
}
@@ -6551,7 +6554,8 @@ static bool cpu_has_work_POWER10(CPUState *cs)
}
return false;
} else {
-return msr_ee && (cs->interrupt_request & CPU_INTERRUPT_HARD);
+return FIELD_EX64(env->msr, MSR, EE) &&
+ (cs->interrupt_request & CPU_INTERRUPT_HARD);
}
}
@@ -7119,7 +7123,8 @@ static bool ppc_cpu_has_work(CPUState *cs)
PowerPCCPU *cpu = POWERPC_CPU(cs);
CPUPPCState *env = >env;
-return msr_ee && (cs->interrupt_request & CPU_INTERRUPT_HARD);
+return FIELD_EX64(env->msr, MSR, EE) &&
+ (cs->interrupt_request & CPU_INTERRUPT_HARD);
}
static void ppc_cpu_reset(DeviceState *dev)
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 7e8e34ef06..4c206ba209 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -1709,7 +1709,7 @@ static void ppc_hw_interrupt(CPUPPCState *env)
* clear when coming out of some power management states (in order
* for them to become a 0x100).
*/
-async_deliver = (msr_ee != 0) || env->resume_as_sreset;
+async_deliver = FIELD_EX64(env->msr, MSR, EE) || env->resume_as_sreset;
/* Hypervisor decrementer exception */
if (env->pending_interrupts & (1 << PPC_INTERRUPT_HDECR)) {
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index a3c31b4e48..8276326de9 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -1351,7 +1351,8 @@ static int kvmppc_handle_halt(PowerPCCPU *cpu)
CPUState *cs = CPU(cpu);
CPUPPCState *env = >env;
-if (!(cs->interrupt_request & CPU_INTERRUPT_HARD) && (msr_ee)) {
+if (!(cs->interrupt_request & CPU_INTERRUPT_HARD) &&
+FIELD_EX64(env->msr, MSR, EE)) {
cs->halted = 1;
cs->exception_index = EXCP_HLT;
}
--
2.25.1
[PATCH v2 08/21] target/ppc: Remove msr_ce macro
msr_ce macro hides the usage of env->msr, which is a bad behavior
Substitute it with FIELD_EX64 calls that explicitly use env->msr
as a parameter.
Suggested-by: Richard Henderson
Signed-off-by: Víctor Colombo
---
v2: Remove M_MSR_CE and use FIELD_EX64 instead
Signed-off-by: Víctor Colombo
---
target/ppc/cpu.h | 2 +-
target/ppc/excp_helper.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 51db67ff92..65cb457e6f 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -354,6 +354,7 @@ typedef enum {
#define MSR_RI 1 /* Recoverable interrupt1*/
#define MSR_LE 0 /* Little-endian mode 1 hflags */
+FIELD(MSR, CE, MSR_CE, 1)
FIELD(MSR, ILE, MSR_ILE, 1)
FIELD(MSR, EE, MSR_EE, 1)
FIELD(MSR, PR, MSR_PR, 1)
@@ -478,7 +479,6 @@ FIELD(MSR, LE, MSR_LE, 1)
#define msr_cm ((env->msr >> MSR_CM) & 1)
#define msr_gs ((env->msr >> MSR_GS) & 1)
#define msr_pow ((env->msr >> MSR_POW) & 1)
-#define msr_ce ((env->msr >> MSR_CE) & 1)
#define msr_fp ((env->msr >> MSR_FP) & 1)
#define msr_me ((env->msr >> MSR_ME) & 1)
#define msr_fe0 ((env->msr >> MSR_FE0) & 1)
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 4c206ba209..ca80c1ed63 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -1749,7 +1749,7 @@ static void ppc_hw_interrupt(CPUPPCState *env)
return;
}
}
-if (msr_ce != 0) {
+if (FIELD_EX64(env->msr, MSR, CE)) {
/* External critical interrupt */
if (env->pending_interrupts & (1 << PPC_INTERRUPT_CEXT)) {
powerpc_excp(cpu, POWERPC_EXCP_CRITICAL);
--
2.25.1
[PATCH v2 20/21] target/ppc: Add unused msr bits FIELDs
Add FIELDs macros for msr bits that had an unused msr_* before.
Signed-off-by: Víctor Colombo
---
v2: Remove M_MSR_* and use FIELD macro now.
Signed-off-by: Víctor Colombo
---
target/ppc/cpu.h | 26 ++
1 file changed, 26 insertions(+)
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 9b765af4db..5cd9d88555 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -354,16 +354,31 @@ typedef enum {
#define MSR_RI 1 /* Recoverable interrupt1*/
#define MSR_LE 0 /* Little-endian mode 1 hflags */
+FIELD(MSR, SF, MSR_SF, 1)
+FIELD(MSR, TAG, MSR_TAG, 1)
+FIELD(MSR, ISF, MSR_ISF, 1)
#if defined(TARGET_PPC64)
FIELD(MSR, HV, MSR_HV, 1)
#define FIELD_EX64_HV(storage) FIELD_EX64(storage, MSR, HV)
#else
#define FIELD_EX64_HV(storage) 0
#endif
+FIELD(MSR, TS0, MSR_TS0, 1)
+FIELD(MSR, TS1, MSR_TS1, 1)
FIELD(MSR, TS, MSR_TS0, 2)
+FIELD(MSR, TM, MSR_TM, 1)
FIELD(MSR, CM, MSR_CM, 1)
+FIELD(MSR, ICM, MSR_ICM, 1)
FIELD(MSR, GS, MSR_GS, 1)
+FIELD(MSR, UCLE, MSR_UCLE, 1)
+FIELD(MSR, VR, MSR_VR, 1)
+FIELD(MSR, SPE, MSR_SPE, 1)
+FIELD(MSR, VSX, MSR_VSX, 1)
+FIELD(MSR, S, MSR_S, 1)
+FIELD(MSR, KEY, MSR_KEY, 1)
FIELD(MSR, POW, MSR_POW, 1)
+FIELD(MSR, WE, MSR_WE, 1)
+FIELD(MSR, TGPR, MSR_TGPR, 1)
FIELD(MSR, CE, MSR_CE, 1)
FIELD(MSR, ILE, MSR_ILE, 1)
FIELD(MSR, EE, MSR_EE, 1)
@@ -373,10 +388,21 @@ FIELD(MSR, ME, MSR_ME, 1)
/* MSR_FE0 and MSR_FE1 are not side-by-side so we can't combine them */
FIELD(MSR, FE0, MSR_FE0, 1)
FIELD(MSR, FE1, MSR_FE1, 1)
+FIELD(MSR, SE, MSR_SE, 1)
+FIELD(MSR, DWE, MSR_DWE, 1)
+FIELD(MSR, UBLE, MSR_UBLE, 1)
+FIELD(MSR, BE, MSR_BE, 1)
+FIELD(MSR, DE, MSR_DE, 1)
+FIELD(MSR, AL, MSR_AL, 1)
FIELD(MSR, EP, MSR_EP, 1)
FIELD(MSR, IR, MSR_IR, 1)
FIELD(MSR, DR, MSR_DR, 1)
+FIELD(MSR, IS, MSR_IS, 1)
FIELD(MSR, DS, MSR_DS, 1)
+FIELD(MSR, PE, MSR_PE, 1)
+FIELD(MSR, PX, MSR_PX, 1)
+FIELD(MSR, PMM, MSR_PMM, 1)
+FIELD(MSR, RI, MSR_RI, 1)
FIELD(MSR, LE, MSR_LE, 1)
/* PMU bits */
--
2.25.1
[PATCH v2 03/21] target/ppc: Remove msr_pr macro
msr_pr macro hides the usage of env->msr, which is a bad behavior
Substitute it with FIELD_EX64 calls that explicitly use env->msr
as a parameter.
Suggested-by: Richard Henderson
Signed-off-by: Víctor Colombo
---
v2: Remove M_MSR_PR and use FIELD_EX64 instead
Signed-off-by: Víctor Colombo
---
hw/ppc/pegasos2.c| 2 +-
hw/ppc/spapr.c | 2 +-
target/ppc/cpu.h | 4 +++-
target/ppc/cpu_init.c| 4 ++--
target/ppc/excp_helper.c | 8 +---
target/ppc/mem_helper.c | 5 +++--
target/ppc/mmu-radix64.c | 5 +++--
target/ppc/mmu_common.c | 23 ---
8 files changed, 30 insertions(+), 23 deletions(-)
diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
index 56bf203dfd..9411ca6b16 100644
--- a/hw/ppc/pegasos2.c
+++ b/hw/ppc/pegasos2.c
@@ -461,7 +461,7 @@ static void pegasos2_hypercall(PPCVirtualHypervisor *vhyp,
PowerPCCPU *cpu)
/* The TCG path should also be holding the BQL at this point */
g_assert(qemu_mutex_iothread_locked());
-if (msr_pr) {
+if (FIELD_EX64(env->msr, MSR, PR)) {
qemu_log_mask(LOG_GUEST_ERROR, "Hypercall made with MSR[PR]=1\n");
env->gpr[3] = H_PRIVILEGE;
} else if (env->gpr[3] == KVMPPC_H_RTAS) {
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 22569305d2..fe9937e811 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1269,7 +1269,7 @@ static void emulate_spapr_hypercall(PPCVirtualHypervisor
*vhyp,
g_assert(!vhyp_cpu_in_nested(cpu));
-if (msr_pr) {
+if (FIELD_EX64(env->msr, MSR, PR)) {
hcall_dprintf("Hypercall made with MSR[PR]=1\n");
env->gpr[3] = H_PRIVILEGE;
} else {
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 106b555b86..21d1f14381 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -25,6 +25,7 @@
#include "exec/cpu-defs.h"
#include "cpu-qom.h"
#include "qom/object.h"
+#include "hw/registerfields.h"
#define TCG_GUEST_DEFAULT_MO 0
@@ -353,6 +354,8 @@ typedef enum {
#define MSR_RI 1 /* Recoverable interrupt1*/
#define MSR_LE 0 /* Little-endian mode 1 hflags */
+FIELD(MSR, PR, MSR_PR, 1)
+
/* PMU bits */
#define MMCR0_FC PPC_BIT(32) /* Freeze Counters */
#define MMCR0_PMAO PPC_BIT(56) /* Perf Monitor Alert Ocurred */
@@ -474,7 +477,6 @@ typedef enum {
#define msr_ce ((env->msr >> MSR_CE) & 1)
#define msr_ile ((env->msr >> MSR_ILE) & 1)
#define msr_ee ((env->msr >> MSR_EE) & 1)
-#define msr_pr ((env->msr >> MSR_PR) & 1)
#define msr_fp ((env->msr >> MSR_FP) & 1)
#define msr_me ((env->msr >> MSR_ME) & 1)
#define msr_fe0 ((env->msr >> MSR_FE0) & 1)
diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index d42e2ba8e0..ac16a64846 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -6303,7 +6303,7 @@ static bool cpu_has_work_POWER9(CPUState *cs)
if ((env->pending_interrupts & (1u << PPC_INTERRUPT_EXT)) &&
(env->spr[SPR_LPCR] & LPCR_EEE)) {
bool heic = !!(env->spr[SPR_LPCR] & LPCR_HEIC);
-if (heic == 0 || !msr_hv || msr_pr) {
+if (!heic || !msr_hv || FIELD_EX64(env->msr, MSR, PR)) {
return true;
}
}
@@ -6517,7 +6517,7 @@ static bool cpu_has_work_POWER10(CPUState *cs)
if ((env->pending_interrupts & (1u << PPC_INTERRUPT_EXT)) &&
(env->spr[SPR_LPCR] & LPCR_EEE)) {
bool heic = !!(env->spr[SPR_LPCR] & LPCR_HEIC);
-if (heic == 0 || !msr_hv || msr_pr) {
+if (!heic || !msr_hv || FIELD_EX64(env->msr, MSR, PR)) {
return true;
}
}
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index d3e2cfcd71..7e8e34ef06 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -1738,7 +1738,8 @@ static void ppc_hw_interrupt(CPUPPCState *env)
bool lpes0 = !!(env->spr[SPR_LPCR] & LPCR_LPES0);
bool heic = !!(env->spr[SPR_LPCR] & LPCR_HEIC);
/* HEIC blocks delivery to the hypervisor */
-if ((async_deliver && !(heic && msr_hv && !msr_pr)) ||
+if ((async_deliver && !(heic && msr_hv &&
+!FIELD_EX64(env->msr, MSR, PR))) ||
(env->has_hv_mode && msr_hv == 0 && !lpes0)) {
if (books_vhyp_promotes_external_to_hvirt(cpu)) {
powerpc_excp(cpu, POWERPC_EXCP_HVIRT);
@@ -1818,7 +1819,8 @@ static void ppc_hw_interrupt(CPUPPCState *env)
* EBB exception must be taken in problem state and
* with BESCR_GE set.
*/
-if (msr_pr == 1 && env->spr[SPR_BESCR] & BESCR_GE) {
+if (FIELD_EX64(env->msr, MSR, PR) &&
+(env->spr[SPR_BESCR] & BESCR_GE)) {
env->pending_interrupts &= ~(1 << PPC_INTERRUPT_EBB);
if (env->spr[SPR_BESCR] & BESCR_PMEO) {
@@ -2094,7 +2096,7 @@ static void do_ebb(CPUPPCState *env, int ebb_excp)
[PATCH v2 05/21] target/ppc: Remove msr_ds macro
msr_ds macro hides the usage of env->msr, which is a bad behavior
Substitute it with FIELD_EX64 calls that explicitly use env->msr
as a parameter.
Suggested-by: Richard Henderson
Signed-off-by: Víctor Colombo
---
v2: Remove M_MSR_DS and use FIELD_EX64 instead
Signed-off-by: Víctor Colombo
---
target/ppc/cpu.h| 2 +-
target/ppc/mmu_common.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 932c5f4bdd..8aff6f1faf 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -355,6 +355,7 @@ typedef enum {
#define MSR_LE 0 /* Little-endian mode 1 hflags */
FIELD(MSR, PR, MSR_PR, 1)
+FIELD(MSR, DS, MSR_DS, 1)
FIELD(MSR, LE, MSR_LE, 1)
/* PMU bits */
@@ -485,7 +486,6 @@ FIELD(MSR, LE, MSR_LE, 1)
#define msr_ep ((env->msr >> MSR_EP) & 1)
#define msr_ir ((env->msr >> MSR_IR) & 1)
#define msr_dr ((env->msr >> MSR_DR) & 1)
-#define msr_ds ((env->msr >> MSR_DS) & 1)
#define msr_ts ((env->msr >> MSR_TS1) & 3)
#define DBCR0_ICMP (1 << 27)
diff --git a/target/ppc/mmu_common.c b/target/ppc/mmu_common.c
index 6ef8b1c00d..7e77b9b84a 100644
--- a/target/ppc/mmu_common.c
+++ b/target/ppc/mmu_common.c
@@ -768,7 +768,7 @@ static bool mmubooke206_get_as(CPUPPCState *env,
*pr_out = !!(epidr & EPID_EPR);
return true;
} else {
-*as_out = msr_ds;
+*as_out = FIELD_EX64(env->msr, MSR, DS);
*pr_out = FIELD_EX64(env->msr, MSR, PR);
return false;
}
--
2.25.1
[PATCH v2 02/21] target/ppc: Remove unused msr_* macros
Some msr_* macros are not used anywhere. Remove them as part of
the work to remove all hidden usage of *env.
Suggested-by: Richard Henderson
Reviewed-by: Richard Henderson
Signed-off-by: Víctor Colombo
---
target/ppc/cpu.h | 21 -
1 file changed, 21 deletions(-)
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index ad31e51d69..106b555b86 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -463,23 +463,14 @@ typedef enum {
#define HFSCR_MSGP PPC_BIT(53) /* Privileged Message Send Facilities */
#define HFSCR_IC_MSGP 0xA
-#define msr_sf ((env->msr >> MSR_SF) & 1)
-#define msr_isf ((env->msr >> MSR_ISF) & 1)
#if defined(TARGET_PPC64)
#define msr_hv ((env->msr >> MSR_HV) & 1)
#else
#define msr_hv (0)
#endif
#define msr_cm ((env->msr >> MSR_CM) & 1)
-#define msr_icm ((env->msr >> MSR_ICM) & 1)
#define msr_gs ((env->msr >> MSR_GS) & 1)
-#define msr_ucle ((env->msr >> MSR_UCLE) & 1)
-#define msr_vr ((env->msr >> MSR_VR) & 1)
-#define msr_spe ((env->msr >> MSR_SPE) & 1)
-#define msr_vsx ((env->msr >> MSR_VSX) & 1)
-#define msr_key ((env->msr >> MSR_KEY) & 1)
#define msr_pow ((env->msr >> MSR_POW) & 1)
-#define msr_tgpr ((env->msr >> MSR_TGPR) & 1)
#define msr_ce ((env->msr >> MSR_CE) & 1)
#define msr_ile ((env->msr >> MSR_ILE) & 1)
#define msr_ee ((env->msr >> MSR_EE) & 1)
@@ -487,25 +478,13 @@ typedef enum {
#define msr_fp ((env->msr >> MSR_FP) & 1)
#define msr_me ((env->msr >> MSR_ME) & 1)
#define msr_fe0 ((env->msr >> MSR_FE0) & 1)
-#define msr_se ((env->msr >> MSR_SE) & 1)
-#define msr_dwe ((env->msr >> MSR_DWE) & 1)
-#define msr_uble ((env->msr >> MSR_UBLE) & 1)
-#define msr_be ((env->msr >> MSR_BE) & 1)
-#define msr_de ((env->msr >> MSR_DE) & 1)
#define msr_fe1 ((env->msr >> MSR_FE1) & 1)
-#define msr_al ((env->msr >> MSR_AL) & 1)
#define msr_ep ((env->msr >> MSR_EP) & 1)
#define msr_ir ((env->msr >> MSR_IR) & 1)
#define msr_dr ((env->msr >> MSR_DR) & 1)
-#define msr_is ((env->msr >> MSR_IS) & 1)
#define msr_ds ((env->msr >> MSR_DS) & 1)
-#define msr_pe ((env->msr >> MSR_PE) & 1)
-#define msr_px ((env->msr >> MSR_PX) & 1)
-#define msr_pmm ((env->msr >> MSR_PMM) & 1)
-#define msr_ri ((env->msr >> MSR_RI) & 1)
#define msr_le ((env->msr >> MSR_LE) & 1)
#define msr_ts ((env->msr >> MSR_TS1) & 3)
-#define msr_tm ((env->msr >> MSR_TM) & 1)
#define DBCR0_ICMP (1 << 27)
#define DBCR0_BRT (1 << 26)
--
2.25.1
Re: [PATCH 0/7] qapi: Primarily whitespace tweaks
On Mon, May 02, 2022 at 02:43:52PM +0200, Markus Armbruster wrote: > PATCH 1-5 queued, because no-brainers :) Thanks! How do you want me to handle respinning 6/7 and 7/7? Send out the entire series again with those two patches tweaked, wait for your pull request to make it into the tree, something else... ? What about the changes you suggested to the commit message of 3/7? -- Andrea Bolognani / Red Hat / Virtualization
[PATCH v2 01/21] target/ppc: Remove fpscr_* macros from cpu.h
fpscr_* defined macros are hiding the usage of *env behind them.
Substitute the usage of these macros with `env->fpscr & FP_*` to make
the code cleaner.
Suggested-by: Richard Henderson
Reviewed-by: Richard Henderson
Signed-off-by: Víctor Colombo
---
target/ppc/cpu.c| 2 +-
target/ppc/cpu.h| 29 -
target/ppc/fpu_helper.c | 28 ++--
3 files changed, 15 insertions(+), 44 deletions(-)
diff --git a/target/ppc/cpu.c b/target/ppc/cpu.c
index d7b42bae52..401b6f9e63 100644
--- a/target/ppc/cpu.c
+++ b/target/ppc/cpu.c
@@ -88,7 +88,7 @@ static inline void fpscr_set_rounding_mode(CPUPPCState *env)
int rnd_type;
/* Set rounding mode */
-switch (fpscr_rn) {
+switch (env->fpscr & FP_RN) {
case 0:
/* Best approximation (round to nearest) */
rnd_type = float_round_nearest_even;
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index c2b6c987c0..ad31e51d69 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -713,41 +713,12 @@ enum {
#define FPSCR_NI 2 /* Floating-point non-IEEE mode */
#define FPSCR_RN11
#define FPSCR_RN00 /* Floating-point rounding control */
-#define fpscr_drn(((env->fpscr) & FP_DRN) >> FPSCR_DRN0)
-#define fpscr_fex(((env->fpscr) >> FPSCR_FEX)& 0x1)
-#define fpscr_vx (((env->fpscr) >> FPSCR_VX) & 0x1)
-#define fpscr_ox (((env->fpscr) >> FPSCR_OX) & 0x1)
-#define fpscr_ux (((env->fpscr) >> FPSCR_UX) & 0x1)
-#define fpscr_zx (((env->fpscr) >> FPSCR_ZX) & 0x1)
-#define fpscr_xx (((env->fpscr) >> FPSCR_XX) & 0x1)
-#define fpscr_vxsnan (((env->fpscr) >> FPSCR_VXSNAN) & 0x1)
-#define fpscr_vxisi (((env->fpscr) >> FPSCR_VXISI) & 0x1)
-#define fpscr_vxidi (((env->fpscr) >> FPSCR_VXIDI) & 0x1)
-#define fpscr_vxzdz (((env->fpscr) >> FPSCR_VXZDZ) & 0x1)
-#define fpscr_vximz (((env->fpscr) >> FPSCR_VXIMZ) & 0x1)
-#define fpscr_vxvc (((env->fpscr) >> FPSCR_VXVC) & 0x1)
-#define fpscr_fpcc (((env->fpscr) >> FPSCR_FPCC) & 0xF)
-#define fpscr_vxsoft (((env->fpscr) >> FPSCR_VXSOFT) & 0x1)
-#define fpscr_vxsqrt (((env->fpscr) >> FPSCR_VXSQRT) & 0x1)
-#define fpscr_vxcvi (((env->fpscr) >> FPSCR_VXCVI) & 0x1)
-#define fpscr_ve (((env->fpscr) >> FPSCR_VE) & 0x1)
-#define fpscr_oe (((env->fpscr) >> FPSCR_OE) & 0x1)
-#define fpscr_ue (((env->fpscr) >> FPSCR_UE) & 0x1)
-#define fpscr_ze (((env->fpscr) >> FPSCR_ZE) & 0x1)
-#define fpscr_xe (((env->fpscr) >> FPSCR_XE) & 0x1)
-#define fpscr_ni (((env->fpscr) >> FPSCR_NI) & 0x1)
-#define fpscr_rn (((env->fpscr) >> FPSCR_RN0)& 0x3)
/* Invalid operation exception summary */
#define FPSCR_IX ((1 << FPSCR_VXSNAN) | (1 << FPSCR_VXISI) | \
(1 << FPSCR_VXIDI) | (1 << FPSCR_VXZDZ) | \
(1 << FPSCR_VXIMZ) | (1 << FPSCR_VXVC) | \
(1 << FPSCR_VXSOFT) | (1 << FPSCR_VXSQRT) | \
(1 << FPSCR_VXCVI))
-/* exception summary */
-#define fpscr_ex (((env->fpscr) >> FPSCR_XX) & 0x1F)
-/* enabled exception summary */
-#define fpscr_eex (((env->fpscr) >> FPSCR_XX) & ((env->fpscr) >> FPSCR_XE) & \
- 0x1F)
#define FP_DRN2 (1ull << FPSCR_DRN2)
#define FP_DRN1 (1ull << FPSCR_DRN1)
diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c
index 99281cc37a..f6c8318a71 100644
--- a/target/ppc/fpu_helper.c
+++ b/target/ppc/fpu_helper.c
@@ -202,7 +202,7 @@ static void finish_invalid_op_excp(CPUPPCState *env, int
op, uintptr_t retaddr)
env->fpscr |= FP_VX;
/* Update the floating-point exception summary */
env->fpscr |= FP_FX;
-if (fpscr_ve != 0) {
+if (env->fpscr & FP_VE) {
/* Update the floating-point enabled exception summary */
env->fpscr |= FP_FEX;
if (fp_exceptions_enabled(env)) {
@@ -216,7 +216,7 @@ static void finish_invalid_op_arith(CPUPPCState *env, int
op,
bool set_fpcc, uintptr_t retaddr)
{
env->fpscr &= ~(FP_FR | FP_FI);
-if (fpscr_ve == 0) {
+if (!(env->fpscr & FP_VE)) {
if (set_fpcc) {
env->fpscr &= ~FP_FPCC;
env->fpscr |= (FP_C | FP_FU);
@@ -286,7 +286,7 @@ static void float_invalid_op_vxvc(CPUPPCState *env, bool
set_fpcc,
/* Update the floating-point exception summary */
env->fpscr |= FP_FX;
/* We must update the target FPR before raising the exception */
-if (fpscr_ve != 0) {
+if (env->fpscr & FP_VE) {
CPUState *cs = env_cpu(env);
cs->exception_index = POWERPC_EXCP_PROGRAM;
@@ -303,7 +303,7 @@ static void float_invalid_op_vxcvi(CPUPPCState *env, bool
set_fpcc,
{
env->fpscr |= FP_VXCVI;
env->fpscr &= ~(FP_FR | FP_FI);
-if (fpscr_ve == 0) {
+if (!(env->fpscr & FP_VE)) {
if (set_fpcc) {
env->fpscr &= ~FP_FPCC;
env->fpscr |=
[PATCH v2 00/21] target/ppc: Remove hidden usages of *env
By running the grep command `git grep -nr 'define \(fpscr\|msr\)_[a-z0-9]\+\>'` we can find multiple macros that use `env->fpscr` and `env->msr` but doesn't take *env as a parameter. Richard Henderson said [1] that these macros hiding the usage of *env "are evil". This patch series remove them and substitute with an explicit usage of *env by using registerfields API. Patch 20 (target/ppc: Add unused msr bits FIELDs) declares unused FIELDs, the same that were removed in patch 02 (target/ppc: Remove unused msr_* macros). I did that to keep the changes consistent with what was already present before. Patch 21 (target/ppc: Change MSR_* to follow POWER ISA numbering convention) changes the MSR_* bit number to match POWER ISA by adding a new macro to 'invert' the ordering. (added in v2) [1]: https://lists.gnu.org/archive/html/qemu-ppc/2021-11/msg00280.html Patches requiring review: Patch 3 and after Hello everyone, thanks for your kind reviews in v1, What do you think of this new approach I did for v2? v2: - Abandon the ideia to add an M_MSR_* macro - Instead, use registerfields API as suggested by Richard - Add patch 21 to invert MSR_* values to match ISA ordering Víctor Colombo (21): target/ppc: Remove fpscr_* macros from cpu.h target/ppc: Remove unused msr_* macros target/ppc: Remove msr_pr macro target/ppc: Remove msr_le macro target/ppc: Remove msr_ds macro target/ppc: Remove msr_ile macro target/ppc: Remove msr_ee macro target/ppc: Remove msr_ce macro target/ppc: Remove msr_pow macro target/ppc: Remove msr_me macro target/ppc: Remove msr_gs macro target/ppc: Remove msr_fp macro target/ppc: Remove msr_me macro target/ppc: Remove msr_ir macro target/ppc: Remove msr_dr macro target/ppc: Remove msr_ep macro target/ppc: Remove msr_fe0 and msr_fe1 macros target/ppc: Remove msr_ts macro target/ppc: Remove msr_hv macro target/ppc: Add unused msr bits FIELDs target/ppc: Change MSR_* to follow POWER ISA numbering convention hw/ppc/pegasos2.c| 2 +- hw/ppc/spapr.c | 2 +- target/ppc/cpu.c | 2 +- target/ppc/cpu.h | 214 ++- target/ppc/cpu_init.c| 23 +++-- target/ppc/excp_helper.c | 66 +++- target/ppc/fpu_helper.c | 28 ++--- target/ppc/gdbstub.c | 2 +- target/ppc/helper_regs.c | 12 +-- target/ppc/kvm.c | 7 +- target/ppc/machine.c | 2 +- target/ppc/mem_helper.c | 23 +++-- target/ppc/misc_helper.c | 2 +- target/ppc/mmu-radix64.c | 11 +- target/ppc/mmu_common.c | 40 target/ppc/mmu_helper.c | 6 +- 16 files changed, 225 insertions(+), 217 deletions(-) -- 2.25.1
Re: [PATCH RFC 09/10] migration/dirtyrate: Expand dirty_bitmap to be tracked separately for devices
On 5/2/22 13:54, Markus Armbruster wrote:
> Joao Martins writes:
>
>> Expand dirtyrate measurer that is accessible via HMP calc_dirty_rate
>> or QMP 'calc-dirty-rate' to receive a @scope argument. The scope
>> then restricts the dirty tracking to be done at devices only,
>> while neither enabling or using the KVM (CPU) dirty tracker.
>> The default stays as is i.e. dirty-ring / dirty-bitmap from KVM.
>>
>> This is useful to test, exercise the IOMMU dirty tracker and observe
>> how much a given device is dirtying memory.
>>
>> Signed-off-by: Joao Martins
>
> [...]
>
>> diff --git a/qapi/migration.json b/qapi/migration.json
>> index 27d7b281581d..082830c6e771 100644
>> --- a/qapi/migration.json
>> +++ b/qapi/migration.json
>> @@ -1793,6 +1793,19 @@
>> { 'enum': 'DirtyRateMeasureMode',
>>'data': ['page-sampling', 'dirty-ring', 'dirty-bitmap'] }
>>
>> +##
>> +# @DirtyRateScope:
>> +#
>> +# An enumeration of scope of measuring dirtyrate.
>
> "dirtyrate" is not a word.
>
Indeed. I will be more verbose rather than using 'dirty rate'.
>> +#
>> +# @dirty-devices: calculate dirtyrate by devices only.
>
> Please document @all, too.
>
OK. I probably should have used 'vcpu' and 'devices',
rather than 'all' and 'dirty-devices'
>> +#
>> +# Since: 6.2
>> +#
This should be 7.1.
>> +##
>> +{ 'enum': 'DirtyRateScope',
>> + 'data': ['all', 'dirty-devices'] }
>> +
>> ##
>> # @DirtyRateInfo:
>> #
>> @@ -1827,6 +1840,7 @@
>> 'calc-time': 'int64',
>> 'sample-pages': 'uint64',
>> 'mode': 'DirtyRateMeasureMode',
>> + 'scope': 'DirtyRateScope',
>
> Please document new member @scope.
>
OK.
Re: [PATCH v3 0/3] Misc AC97 clean ups
On Sat, 23 Apr 2022, BALATON Zoltan wrote: During trying to implement via-ac97 I did some small clean ups to ac97 which is in this series. The via-ac97 is not working yet so that's not included but these unrelated clean ups could be merged now. v3: Fixed misalignments and drop spaces before commas (Víctor Colombo) v2: Added R-b and changes from Peter Maydell Ping? https://patchew.org/QEMU/cover.1650706617.git.bala...@eik.bme.hu/ Regards. BALATON Zoltan (3): hw/audio/ac97: Coding style fixes to avoid checkpatch errors hw/audio/ac97: Remove unimplemented reset functions hw/audio/ac97: Remove unneeded local variables hw/audio/ac97.c | 758 +++- 1 file changed, 363 insertions(+), 395 deletions(-)
Re: [Patch 1/3] hw/vfio/pci: fix vfio_pci_hot_reset_result trace point
On Mon, 2 May 2022 02:42:21 -0700 Yi Liu wrote: > From: Eric Auger > > Properly output the errno string. More explanation please, why is it broken and how does this fix it? Thanks, Alex > Signed-off-by: Eric Auger > Signed-off-by: Yi Liu > --- > hw/vfio/pci.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c > index 9fd9faee1d..4a66376be6 100644 > --- a/hw/vfio/pci.c > +++ b/hw/vfio/pci.c > @@ -2337,7 +2337,7 @@ static int vfio_pci_hot_reset(VFIOPCIDevice *vdev, bool > single) > g_free(reset); > > trace_vfio_pci_hot_reset_result(vdev->vbasedev.name, > -ret ? "%m" : "Success"); > +ret ? strerror(errno) : "Success"); > > out: > /* Re-enable INTx on affected devices */
Re: [PATCH 6/7] qapi: Drop unnecessary horizontal spacing in comments
On Mon, May 02, 2022 at 10:50:07AM +0200, Markus Armbruster wrote: > Andrea Bolognani writes: > > -# @writeback: true if writeback mode is enabled > > -# @direct: true if the host page cache is bypassed (O_DIRECT) > > -# @no-flush:true if flush requests are ignored for the device > > +# @writeback: true if writeback mode is enabled > > +# @direct:true if the host page cache is bypassed (O_DIRECT) > > +# @no-flush: true if flush requests are ignored for the device > > I'm no fan of horizontally aligning descriptions, because when you add a > longer name, you either realign (I hate the churn) or live with the > inconsistency (I hate that, too). We seem to be in violent agreement on the topic, but it's apparent that other people feel diffently :) > I doubt changing to a different alignment now is useful. The next > patch, however, drops the alignment entirely. Possibly useful. > > Thoughts? My rationale for splitting things the way I did is that, if dropping the horizontal alignment entirely was not considered desirable, we could at least get rid of the extra whitespace. But if you think that the benefit from the half measure doesn't offset the cost of the churn it causes, I'm happy to drop these hunks and go straight from the current status to no horizontal alignment at all in one fell swoop with the next patch. > > -# Since: 0.14 > > +# Since: 0.14 > > This one is TAG: TEXT, whereas the one above is a multiple @NAME: > DESCRIPTION. Extra space in the latter can provide alignment. Extra > space in the former is always redundant. I'd take a patch dropping > these obviously redundant spaces without debate :) Okay, I'll respin this so that the first patch drops all extra whitespace in contexts where horizontal alignment is either not attempted or not possible, and the second one implements the more controversial changes. -- Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH] Meson: Make mremap() detecting works correctly
Queued, thanks. Paolo
Re: [RFC PATCH v1 0/8] qapi: add generator for Golang interface
On Mon, May 02, 2022 at 01:46:23PM +0200, Markus Armbruster wrote: > Andrea Bolognani writes: > >> > The wire protocol would still retain the unappealing name, but at > >> > least client libraries could hide the uglyness from users. > >> > >> At the price of mild inconsistency between the library interface and > >> QMP. > > > > That's fine, and in fact it already happens all the time when QAPI > > names (log-append) are translated to C identifiers (log_append). > > There's a difference between trivial translations like "replace '-' by > '_'" and arbitrary replacement like the one for enumeration constants > involving 'prefix'. Fair enough. I still feel that 1) users of a language SDK will ideally not need to look at the QAPI schema or wire chatter too often and 2) even when that ends up being necessary, figuring out that LogAppend and logappend are the same thing is not going to be an unreasonable hurdle, especially when the status quo would be to work with Logappend instead. > > The point is that, if we want to provide a language interface that > > feels natural, we need a way to mark parts of a QAPI symbol's name in > > a way that makes it possible for the generator to know they're > > acronyms and treat them in an appropriate, language-specific manner. > > It's not just acronyms. Consider IAmALittleTeapot. If you can assume > that only beginning of words are capitalized, even for acronyms, you can > split this into words without trouble. You can't recover correct case, > though: "i am a little teapot" is wrong. Is there any scenario in which we would care though? We're in the business of translating identifiers from one machine representation to another, so once it has been split up correctly into the words that compose it (which in your example above it has) then we don't really care about anything else unless acronyms are involved. In other words, we can obtain the list of words "i am a little teapot" programmatically both from IAmALittleTeapot and i-am-a-little-teapot, and in both cases we can then generate IAmALittleTeapot or I_AM_A_LITTLE_TEAPOT or i_am_a_little_teapot or whatever is appropriate for the context and target language, but the fact that in a proper English sentence "I" would have to be capitalized doesn't really enter the picture. > "Split before capital letter" falls apart when you have characters that > cannot be capitalized: Point3d. > > Camel case is hopeless. I would argue that it works quite well for most scenarios, but there are some corner cases where it's clearly not good enough. If we can define a way to clue in the generator about "Point3d" having to be interpreted as "point 3d" and "VNCProps" as "vnc props", then we are golden. That wouldn't be necessary for simple cases that are already handled correctly. A more radical idea would be to start using dash-notation for types too. That'd remove the word splitting issue altogether, at the cost of the schema being (possibly) harder to read and more distanced from the generated code. You'd still only be able to generate VncProps from vnc-props though. > > The obvious way to implement this would be with an annotation along > > the lines of the one I proposed. Other ideas? > > I'm afraid having the schema spell out names in multiple naming > conventions could be onerous. How many names will need it? I don't have hard data on this. I could try extracting it, but that might end up being a bigger job than I had anticipated. My guess is that the number of cases where the naive algorithm can't split words correctly is relatively small compared to the size of the entire QAPI schema. Fair warning: I have made incorrect guesses in the past ;) > Times how many naming conventions? Yeah, I don't think requiring all possible permutations to be spelled out in the schema is the way to go. That's exactly why my proposal was to offer a way to inject the semantic information that the parser can't figure out itself. Once you have a way to inform the generator that "VNCProps" is made of the two words "vnc" and "props", and that "vnc" is an acronym, then it can generate an identifier appropriate for the target language without having to spell out anywhere that such an identifier would be VNCProps for Go and VncProps for Rust. By the way, while looking around I realized that we also have to take into account things like D-Bus: the QAPI type ChardevDBus, for example, would probably translate verbatim to Go but have to be changed to ChardevDbus for Rust. Fun :) Revised proposal for the annotation: ns:word-WORD-WoRD-123Word Words are always separated by dashes; "regular" words are entirely lowercase, while the presence of even a single uppercase letter in a word denotes the fact that its case should be preserved when the naming conventions of the target language allow that. > Another issue: the fancier the translation from schema name to > language-specific name gets, the harder it becomes to find one from the > other. That's
Re: [RFC PATCH] target/s390x: Check storage keys in the TPROT instruction
On 5/2/22 12:17, Janis Schoetterl-Glausch wrote:
> On 5/2/22 10:25, Thomas Huth wrote:
>> TPROT allows to specify an access key that should be used for checking
>> with the storage key of the destination page, to see whether an access
>> is allowed or not. Honor this storage key checking now in the emulated
>> TPROT instruction, too.
>>
>> Since we need the absolute address of the page (for getting the storage
>> key), we are now also calling mmu_translate() directly instead of
>> going via s390_cpu_virt_mem_check_write/read() - since we're only
>> interested in one page, and since mmu_translate() does not try to inject
>> excetions directly (it reports them via the return code instead), this
>> is likely the better function to use in TPROT anyway.
>>
>> Signed-off-by: Thomas Huth
>> ---
>> This fixes the new TPROT-related storage key checks in this new
>> kvm-unit-tests patch:
>> https://lore.kernel.org/kvm/20220425161731.1575742-1-s...@linux.ibm.com/
>
> Thanks for having a go at this.
> The key checking logic looks good to me; the expressions get a bit unwieldy,
> but that is a style thing.
> However, I'm wondering whether it would be better to mirror what the kernel
> is doing and address the
>
> * TODO: key-controlled protection. Only CPU accesses make use of the
> * PSW key. CSS accesses are different - we have to pass in the key.
>
> in mmu_handle_skey, then tprot emulation would just relay the result of trying
> the translation with store or fetch, passing in the key.
>>
>> target/s390x/cpu.h| 1 +
>> target/s390x/tcg/mem_helper.c | 61 ---
>> 2 files changed, 50 insertions(+), 12 deletions(-)
>>
>> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
>> index 7d6d01325b..348950239f 100644
>> --- a/target/s390x/cpu.h
>> +++ b/target/s390x/cpu.h
>> @@ -328,6 +328,7 @@ extern const VMStateDescription vmstate_s390_cpu;
>> /* Control register 0 bits */
>> #define CR0_LOWPROT 0x1000ULL
>> #define CR0_SECONDARY 0x0400ULL
>> +#define CR0_STOR_PROT_OVERRIDE 0x0100ULL
>> #define CR0_EDAT0x0080ULL
>> #define CR0_AFP 0x0004ULL
>> #define CR0_VECTOR 0x0002ULL
>> diff --git a/target/s390x/tcg/mem_helper.c b/target/s390x/tcg/mem_helper.c
>> index fc52aa128b..1e0309bbe8 100644
>> --- a/target/s390x/tcg/mem_helper.c
>> +++ b/target/s390x/tcg/mem_helper.c
>> @@ -2141,43 +2141,80 @@ uint32_t HELPER(testblock)(CPUS390XState *env,
>> uint64_t real_addr)
>> return 0;
>> }
>>
>
> [...]
>
>> +
>> uint32_t HELPER(tprot)(CPUS390XState *env, uint64_t a1, uint64_t a2)
>> {
>> S390CPU *cpu = env_archcpu(env);
>> -CPUState *cs = env_cpu(env);
>> +const int tp_acc = (a2 & SK_ACC_MASK) >> 4;
>> +uint8_t skey;
>> +int acc, pgm_code, pflags;
>> +target_ulong abs_addr;
>> +uint64_t asc = cpu->env.psw.mask & PSW_MASK_ASC;
>> +uint64_t tec;
>>
>> /*
>> * TODO: we currently don't handle all access protection types
>> - * (including access-list and key-controlled) as well as AR mode.
>> + * (including access-list) as well as AR mode.
>> */
>> -if (!s390_cpu_virt_mem_check_write(cpu, a1, 0, 1)) {
>> -/* Fetching permitted; storing permitted */
>> +pgm_code = mmu_translate(env, a1, true, asc, _addr, , );
mmu_translate/mmu_handle_skey sets the change bit for stores, whereas TPROT
specifies
that it doesn't.
Not sure what the best way to handle this is.
Additional pretend fetch/store access modes?
>
> I don't like the use of true to indicate a store here, when values other than
> 0 and 1 are possible.
> Any reason not to use MMU_DATA_STORE?
>
> A comment about fetch protection override might be nice here:
>/*
> * Since fetch protection override may apply to half of page 0 only,
> * it need not be considered in the following.
> */
Disregard that, it's not true, TPROT does honor fetch-protection override, I
just
made a mistake while adding a test for it to the kvm-unit-test.
[...]
Re: [RFC PATCH] target/s390x: Check storage keys in the TPROT instruction
On 5/2/22 10:25, Thomas Huth wrote:
> TPROT allows to specify an access key that should be used for checking
> with the storage key of the destination page, to see whether an access
> is allowed or not. Honor this storage key checking now in the emulated
> TPROT instruction, too.
>
> Since we need the absolute address of the page (for getting the storage
> key), we are now also calling mmu_translate() directly instead of
> going via s390_cpu_virt_mem_check_write/read() - since we're only
> interested in one page, and since mmu_translate() does not try to inject
> excetions directly (it reports them via the return code instead), this
> is likely the better function to use in TPROT anyway.
>
> Signed-off-by: Thomas Huth
> ---
> This fixes the new TPROT-related storage key checks in this new
> kvm-unit-tests patch:
> https://lore.kernel.org/kvm/20220425161731.1575742-1-s...@linux.ibm.com/
Thanks for having a go at this.
The key checking logic looks good to me; the expressions get a bit unwieldy,
but that is a style thing.
However, I'm wondering whether it would be better to mirror what the kernel
is doing and address the
* TODO: key-controlled protection. Only CPU accesses make use of the
* PSW key. CSS accesses are different - we have to pass in the key.
in mmu_handle_skey, then tprot emulation would just relay the result of trying
the translation with store or fetch, passing in the key.
>
> target/s390x/cpu.h| 1 +
> target/s390x/tcg/mem_helper.c | 61 ---
> 2 files changed, 50 insertions(+), 12 deletions(-)
>
> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
> index 7d6d01325b..348950239f 100644
> --- a/target/s390x/cpu.h
> +++ b/target/s390x/cpu.h
> @@ -328,6 +328,7 @@ extern const VMStateDescription vmstate_s390_cpu;
> /* Control register 0 bits */
> #define CR0_LOWPROT 0x1000ULL
> #define CR0_SECONDARY 0x0400ULL
> +#define CR0_STOR_PROT_OVERRIDE 0x0100ULL
> #define CR0_EDAT0x0080ULL
> #define CR0_AFP 0x0004ULL
> #define CR0_VECTOR 0x0002ULL
> diff --git a/target/s390x/tcg/mem_helper.c b/target/s390x/tcg/mem_helper.c
> index fc52aa128b..1e0309bbe8 100644
> --- a/target/s390x/tcg/mem_helper.c
> +++ b/target/s390x/tcg/mem_helper.c
> @@ -2141,43 +2141,80 @@ uint32_t HELPER(testblock)(CPUS390XState *env,
> uint64_t real_addr)
> return 0;
> }
>
[...]
> +
> uint32_t HELPER(tprot)(CPUS390XState *env, uint64_t a1, uint64_t a2)
> {
> S390CPU *cpu = env_archcpu(env);
> -CPUState *cs = env_cpu(env);
> +const int tp_acc = (a2 & SK_ACC_MASK) >> 4;
> +uint8_t skey;
> +int acc, pgm_code, pflags;
> +target_ulong abs_addr;
> +uint64_t asc = cpu->env.psw.mask & PSW_MASK_ASC;
> +uint64_t tec;
>
> /*
> * TODO: we currently don't handle all access protection types
> - * (including access-list and key-controlled) as well as AR mode.
> + * (including access-list) as well as AR mode.
> */
> -if (!s390_cpu_virt_mem_check_write(cpu, a1, 0, 1)) {
> -/* Fetching permitted; storing permitted */
> +pgm_code = mmu_translate(env, a1, true, asc, _addr, , );
I don't like the use of true to indicate a store here, when values other than 0
and 1 are possible.
Any reason not to use MMU_DATA_STORE?
A comment about fetch protection override might be nice here:
/*
* Since fetch protection override may apply to half of page 0 only,
* it need not be considered in the following.
*/
> +if (!pgm_code) {
> +/* Fetching permitted; storing permitted - but still check skeys */
> +skey = get_skey(abs_addr);
> +acc = (skey & SK_ACC_MASK) >> 4;
> +if (tp_acc != 0 && tp_acc != acc &&
> +!((env->cregs[0] & CR0_STOR_PROT_OVERRIDE) && acc == 9)) {
> +if (skey & SK_F) {
> +return 2;
> +} else {
> +return 1;
> +}
> +}
> return 0;
> }
>
> -if (env->int_pgm_code == PGM_PROTECTION) {
> +if (pgm_code == PGM_PROTECTION) {
> /* retry if reading is possible */
> -cs->exception_index = -1;
> -if (!s390_cpu_virt_mem_check_read(cpu, a1, 0, 1)) {
> +pgm_code = mmu_translate(env, a1, false, asc, _addr, ,
> );
> +if (!pgm_code) {
> /* Fetching permitted; storing not permitted */
> +skey = get_skey(abs_addr);
> +acc = (skey & SK_ACC_MASK) >> 4;
> +if ((skey & SK_F) && tp_acc != 0 && tp_acc != acc &&
> +!((env->cregs[0] & CR0_STOR_PROT_OVERRIDE) && acc == 9)) {
> +return 2;
> +}
> return 1;
> }
> }
>
> -switch (env->int_pgm_code) {
> +switch (pgm_code) {
> case PGM_PROTECTION:
> /* Fetching not permitted;
Re: [PATCH] net/slirp: Use newer slirp_*_hostxfwd API
On 9/25/21 22:48, Nicholas Ngai wrote: libslirp provides a newer slirp_*_hostxfwd API meant for address-agnostic forwarding instead of the is_udp parameter which is limited to just TCP/UDP. Signed-off-by: Nicholas Ngai Reviewed-by: Samuel Thibault Tested-by: Breno Leitao
Re: [PULL 4/8] tests/tcg/s390x: Test BRASL and BRCL with large negative offsets
On 15/03/2022 12.20, Thomas Huth wrote:
From: Ilya Leoshkevich
Add a small test in order to prevent regressions.
Signed-off-by: Ilya Leoshkevich
Message-Id: <20220314104232.675863-4-...@linux.ibm.com>
Reviewed-by: Richard Henderson
Reviewed-by: David Hildenbrand
Signed-off-by: Thomas Huth
---
tests/tcg/s390x/Makefile.target| 1 +
tests/tcg/s390x/branch-relative-long.c | 68 ++
2 files changed, 69 insertions(+)
create mode 100644 tests/tcg/s390x/branch-relative-long.c
diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
index 7aa502a557..f0d474a245 100644
--- a/tests/tcg/s390x/Makefile.target
+++ b/tests/tcg/s390x/Makefile.target
@@ -15,6 +15,7 @@ TESTS+=mvc
TESTS+=shift
TESTS+=trap
TESTS+=signals-s390x
+TESTS+=branch-relative-long
ifneq ($(HAVE_GDB_BIN),)
GDB_SCRIPT=$(SRC_PATH)/tests/guest-debug/run-test.py
diff --git a/tests/tcg/s390x/branch-relative-long.c
b/tests/tcg/s390x/branch-relative-long.c
new file mode 100644
index 00..94219afcad
--- /dev/null
+++ b/tests/tcg/s390x/branch-relative-long.c
@@ -0,0 +1,68 @@
+#include
+#include
+#include
+#include
+
+#define DEFINE_ASM(_name, _code) \
+extern const char _name[]; \
+extern const char _name ## _end[]; \
+asm(".globl " #_name "\n" \
+#_name ":\n" \
+"" _code "\n" \
+".globl " #_name "_end\n" \
+#_name "_end:\n");
+
+DEFINE_ASM(br_r14, "br %r14");
+DEFINE_ASM(brasl_r0, "brasl %r0,.-0x1");
+DEFINE_ASM(brcl_0xf, "brcl 0xf,.-0x1");
Hi Ilya,
I just noticed that this does not compile with Clang
(version 13.0.1):
:8:15: error: offset out of range
brasl %r0,.-0x1
^
:13:14: error: offset out of range
brcl 0xf,.-0x1
^
2 errors generated.
Any ideas how to fix this? Could we decrease the offset a little bit? Or
maybe detect Clang and run with -no-integrated-as in that case?
Thomas
Re: [PATCH] mos6522: fix linking error when CONFIG_MOS6522 is not set
Hi, Mark.
Thanks for reviewing. Comments below.
On 5/2/22 06:43, Mark Cave-Ayland wrote:
On 30/04/2022 00:31, Murilo Opsfelder Araujo wrote:
When CONFIG_MOS6522 is not set, building ppc64-softmmu target fails:
/usr/bin/ld: libqemu-ppc64-softmmu.fa.p/monitor_misc.c.o:(.data+0x1158):
undefined reference to `hmp_info_via'
clang-13: error: linker command failed with exit code 1 (use -v to see
invocation)
Add CONFIG_MOS6522 check for hmp_info_via in hmp-commands-info.hx to fix
such linking error.
Fixes: 409e9f7131e5 (mos6522: add "info via" HMP command for debugging)
Signed-off-by: Murilo Opsfelder Araujo
Cc: Mark Cave-Ayland
Cc: Fabiano Rosas
---
hmp-commands-info.hx | 2 ++
1 file changed, 2 insertions(+)
diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index adfa085a9b..9ad784dd9f 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -881,6 +881,7 @@ SRST
ERST
#if defined(TARGET_M68K) || defined(TARGET_PPC)
+#if defined(CONFIG_MOS6522)
{
.name = "via",
.args_type = "",
@@ -889,6 +890,7 @@ ERST
.cmd = hmp_info_via,
},
#endif
+#endif
SRST
``info via``
Hmmm. The patch in its proposed form isn't correct, since device CONFIG_*
defines aren't declared when processing hmp-commands-info.hx. This was
something that was discovered and discussed in the original thread for which
the current workaround is to use the per-target TARGET_* defines instead.
So my proposed fix worked just by coincidence. Thanks for providing the
background.
Given that the g3beige and mac99 machines are included by default in
qemu-system-ppc64 which both contain the MOS6522 device, I can't quite
understand how CONFIG_MOS6522 isn't being selected.
Can you give more information about how you are building QEMU including your
configure command line?
Here is a reproducer adapted from CentOS 9 Stream qemu-kvm[0] package
(build failed on c9s ppc64le with QEMU at commit
f5643914a9e8f79c606a76e6a9d7ea82a3fc3e65):
$ cat > configs/devices/rh-virtio.mak <<"EOF"
CONFIG_VIRTIO=y
CONFIG_VIRTIO_BALLOON=y
CONFIG_VIRTIO_BLK=y
CONFIG_VIRTIO_GPU=y
CONFIG_VIRTIO_INPUT=y
CONFIG_VIRTIO_INPUT_HOST=y
CONFIG_VIRTIO_NET=y
CONFIG_VIRTIO_RNG=y
CONFIG_VIRTIO_SCSI=y
CONFIG_VIRTIO_SERIAL=y
EOF
$ cat > configs/devices/ppc64-softmmu/ppc64-rh-devices.mak <<"EOF"
include ../rh-virtio.mak
CONFIG_DIMM=y
CONFIG_MEM_DEVICE=y
CONFIG_NVDIMM=y
CONFIG_PCI=y
CONFIG_PCI_DEVICES=y
CONFIG_PCI_TESTDEV=y
CONFIG_PCI_EXPRESS=y
CONFIG_PSERIES=y
CONFIG_SCSI=y
CONFIG_SPAPR_VSCSI=y
CONFIG_TEST_DEVICES=y
CONFIG_USB=y
CONFIG_USB_OHCI=y
CONFIG_USB_OHCI_PCI=y
CONFIG_USB_SMARTCARD=y
CONFIG_USB_STORAGE_CORE=y
CONFIG_USB_STORAGE_CLASSIC=y
CONFIG_USB_XHCI=y
CONFIG_USB_XHCI_NEC=y
CONFIG_USB_XHCI_PCI=y
CONFIG_VFIO=y
CONFIG_VFIO_PCI=y
CONFIG_VGA=y
CONFIG_VGA_PCI=y
CONFIG_VHOST_USER=y
CONFIG_VIRTIO_PCI=y
CONFIG_VIRTIO_VGA=y
CONFIG_WDT_IB6300ESB=y
CONFIG_XICS=y
CONFIG_XIVE=y
CONFIG_TPM=y
CONFIG_TPM_SPAPR=y
CONFIG_TPM_EMULATOR=y
EOF
$ mkdir build
$ cd build
$ ../configure --cc=clang --cxx=/bin/false --prefix=/usr --libdir=/usr/lib64
--datadir=/usr/share --sysconfdir=/etc --interp-prefix=/usr/qemu-%M
--localstatedir=/var --docdir=/usr/share/doc --libexecdir=/usr/libexec
'--extra-ldflags=-Wl,-z,relro -Wl,--as-needed -Wl,-z,now '
'--extra-cflags=-O2 -fexceptions -g -grecord-gcc-switches -pipe -Wall
-Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS
--config /usr/lib/rpm/redhat/redhat-hardened-clang.cfg -fstack-protector-strong
-m64 -mcpu=power9 -mtune=power9 -fasynchronous-unwind-tables
-fstack-clash-protection -Wno-string-plus-int'
--with-pkgversion=qemu-kvm-7.0.0-1.el9 --with-suffix=qemu-kvm
--firmwarepath=/usr/share/qemu-firmware:/usr/share/ipxe/qemu:/usr/share/seavgabios:/usr/share/seabios
--meson=internal --enable-trace-backend=dtrace --with-coroutine=ucontext
--with-git=git --tls-priority=@QEMU,SYSTEM --audio-drv-list= --disable-alsa
--disable-attr --disable-auth-pam --disable-avx2 --disable-avx512f
--disable-block-drv-whitelist-in-tools --disable-bochs --disable-bpf
--disable-brlapi --disable-bsd-user --disable-bzip2 --disable-cap-ng
--disable-capstone --disable-cfi --disable-cfi-debug --disable-cloop
--disable-cocoa --disable-coreaudio --disable-coroutine-pool
--disable-crypto-afalg --disable-curl --disable-curses --disable-dbus-display
--disable-debug-info --disable-debug-mutex --disable-debug-tcg --disable-dmg
--disable-docs --disable-dsound --disable-fdt --disable-fuse
--disable-fuse-lseek --disable-gcrypt --disable-gettext --disable-gio
--disable-glusterfs --disable-gnutls --disable-gtk --disable-guest-agent
--disable-guest-agent-msi --disable-hax --disable-hvf --disable-iconv
--disable-jack --disable-kvm --disable-l2tpv3 --disable-libdaxctl
--disable-libiscsi --disable-libnfs --disable-libpmem --disable-libssh
--disable-libudev --disable-libusb --disable-linux-aio --disable-linux-io-uring
Re: [RFC PATCH v2 0/8] Removal of AioContext lock, bs->parents and ->children: new rwlock
On 5/2/22 10:02, Emanuele Giuseppe Esposito wrote:
Are you saying rdlock isn't necessary in the main loop because nothing
can take the wrlock while our code is executing in the main loop?
Yes, that's the idea.
If I am not mistaken (and I hope I am not), only the main loop currently
modifies/is allowed to modify the graph.
The only case where currently we need to take the rdlock in main loop is
when we have the case
simplified_flush_callback(bs) {
for (child in bs)
bdrv_flush(child->bs);
}
some_function() {
GLOBAL_STATE_CODE();
/* assume bdrv_get_aio_context(bs) != qemu_in_main_thread() */
bdrv_flush(bs);
co = coroutine_create(bdrv_get_aio_context(bs))
qemu_coroutine_enter(co, simplified_flush_callback)
}
This is correct, but it is very unsafe as long as bdrv_flush(bs) is
allowed to run both in coroutine context and outside. So we go circling
back to the same issue that was there in the stackless coroutine
experiment, i.e. functions that can run both in coroutine context and
outside.
This issue is fundamentally one of unclear invariants, and reminds me a
lot of the problems with recursive mutexes.
Paolo
[PATCH] Meson: Make mremap() detecting works correctly
Without this (at least in Fedora 35) it don't detect mremap()
correctly.
Signed-off-by: Juan Quintela
---
meson.build | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/meson.build b/meson.build
index 1fe7d257ff..f96da78741 100644
--- a/meson.build
+++ b/meson.build
@@ -2041,7 +2041,8 @@ config_host_data.set('CONFIG_AVX512F_OPT',
get_option('avx512f') \
have_pvrdma = get_option('pvrdma') \
.require(rdma.found(), error_message: 'PVRDMA requires OpenFabrics
libraries') \
- .require(cc.compiles('''
+ .require(cc.compiles(gnu_source_prefix + '''
+#include
int main(void)
{
char buf = 0;
--
2.35.1
Re: [PATCH RFC 09/10] migration/dirtyrate: Expand dirty_bitmap to be tracked separately for devices
Joao Martins writes:
> Expand dirtyrate measurer that is accessible via HMP calc_dirty_rate
> or QMP 'calc-dirty-rate' to receive a @scope argument. The scope
> then restricts the dirty tracking to be done at devices only,
> while neither enabling or using the KVM (CPU) dirty tracker.
> The default stays as is i.e. dirty-ring / dirty-bitmap from KVM.
>
> This is useful to test, exercise the IOMMU dirty tracker and observe
> how much a given device is dirtying memory.
>
> Signed-off-by: Joao Martins
[...]
> diff --git a/qapi/migration.json b/qapi/migration.json
> index 27d7b281581d..082830c6e771 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -1793,6 +1793,19 @@
> { 'enum': 'DirtyRateMeasureMode',
>'data': ['page-sampling', 'dirty-ring', 'dirty-bitmap'] }
>
> +##
> +# @DirtyRateScope:
> +#
> +# An enumeration of scope of measuring dirtyrate.
"dirtyrate" is not a word.
> +#
> +# @dirty-devices: calculate dirtyrate by devices only.
Please document @all, too.
> +#
> +# Since: 6.2
> +#
> +##
> +{ 'enum': 'DirtyRateScope',
> + 'data': ['all', 'dirty-devices'] }
> +
> ##
> # @DirtyRateInfo:
> #
> @@ -1827,6 +1840,7 @@
> 'calc-time': 'int64',
> 'sample-pages': 'uint64',
> 'mode': 'DirtyRateMeasureMode',
> + 'scope': 'DirtyRateScope',
Please document new member @scope.
> '*vcpu-dirty-rate': [ 'DirtyRateVcpu' ] } }
>
> ##
> @@ -1851,6 +1865,7 @@
> ##
> { 'command': 'calc-dirty-rate', 'data': {'calc-time': 'int64',
> '*sample-pages': 'int',
> + '*scope': 'DirtyRateScope',
> '*mode': 'DirtyRateMeasureMode'} }
>
> ##
[...]
Re: [PATCH 0/7] qapi: Primarily whitespace tweaks
PATCH 1-5 queued, because no-brainers :)
Re: [PATCH] qapi: Fix malformed "Since:" section tags
Markus Armbruster writes: > "Since X.Y" is not recognized as a tagged section, and therefore not > formatted as such in generated documentation. Fix by adding the > required colon. > > Signed-off-by: Markus Armbruster Queued.
Re: [RFC PATCH v1 0/8] qapi: add generator for Golang interface
Andrea Bolognani writes: > On Mon, May 02, 2022 at 09:21:36AM +0200, Markus Armbruster wrote: >> Andrea Bolognani writes: >> > The wire protocol would still retain the unappealing name, but at >> > least client libraries could hide the uglyness from users. >> >> At the price of mild inconsistency between the library interface and >> QMP. > > That's fine, and in fact it already happens all the time when QAPI > names (log-append) are translated to C identifiers (log_append). There's a difference between trivial translations like "replace '-' by '_'" and arbitrary replacement like the one for enumeration constants involving 'prefix'. >> We could clean up QMP if we care, keeping around the old names for >> compatibility. See also Kevin's work on QAPI aliases. Which is much >> more ambitious, though. > > I wasn't aware of that effort. Personally I'm always in favor of > cleaning up inconsistencies, so I am automatically a fan :) > > That said, the idea of exposing a sub-par Go API until such massive > undertaking can be completed is not terribly appealing. Point. > And it would > not address every facet of the issue (see below). > >> > Capitalization of these acronyms is inconsistent across the schema, >> >> Common issue with camel-cased compounds containing acronyms, because >> either way is ugly. > > Agreed :) But consistent ugliness is still preferable to inconsistent > ugliness. True. >> > with one of the two forms disagreeing with Go naming expectations. >> >> Pardon my ignorance: What are Go's expectations? > > Acronyms are usually all upper case: > > https://pkg.go.dev/net/http#ParseHTTPVersion > https://pkg.go.dev/net/http#ProxyURL > https://pkg.go.dev/crypto/tls#NewLRUClientSessionCache > > The same seems to be true of Python: > > https://docs.python.org/3/library/http.html#http.HTTPStatus > https://docs.python.org/3/library/urllib.error.html#urllib.error.URLError > > https://docs.python.org/3/library/xmlrpc.server.html#xmlrpc.server.SimpleXMLRPCServer > > Rust, on the other hand, seems to prefer only capitalizing the first > letter of a word, no matter if it's an acronym: > > https://doc.rust-lang.org/std/net/struct.TcpStream.html > https://doc.rust-lang.org/std/net/struct.Ipv4Addr.html > https://doc.rust-lang.org/std/ffi/struct.OsString.html Another strange game where the only winning move is not to play. > Whether different naming conventions are used for types, functions > and struct members is also language-dependent. Yes. >> > In this case we might be able to just change the schema without >> > introducing backwards compatibility issues, though? Type names are >> > not actually transmitted on the wire IIUC. >> >> Correct! > > That's great, but even if we decided to change all type names so that > the schema is internally consistent and follows a naming convention > that's reasonable for C, Go and Python, we'd still leave the Rust > interface looking weird... There's no one-size-fits-all name, > unfortunately. C will remain the primary customer for quite a while. It doesn't come with universally accepted naming conventions, so we made up our own. I think we have some wiggle room there. We could, for instance, decide to clean up the current inconsistent capitalization of acronyms in the QAPI schema to either style, TCPStream or TcpStream. Your point remains: some names will still look "weird" in some possible target languages. >> > Of course such annotations would only be necessary to deal with >> > identifiers that are not already following the expected naming >> > conventions and when MLAs are involved. >> >> Pardon my ignorance some more: what are MLAs? > > Multi Letter Acronyms. Which are actually just called "acronyms" I > guess? O:-) Well played! > The point is that, if we want to provide a language interface that > feels natural, we need a way to mark parts of a QAPI symbol's name in > a way that makes it possible for the generator to know they're > acronyms and treat them in an appropriate, language-specific manner. It's not just acronyms. Consider IAmALittleTeapot. If you can assume that only beginning of words are capitalized, even for acronyms, you can split this into words without trouble. You can't recover correct case, though: "i am a little teapot" is wrong. "Split before capital letter" falls apart when you have characters that cannot be capitalized: Point3d. Camel case is hopeless. > The obvious way to implement this would be with an annotation along > the lines of the one I proposed. Other ideas? I'm afraid having the schema spell out names in multiple naming conventions could be onerous. How many names will need it? Times how many naming conventions? Another issue: the fancier the translation from schema name to language-specific name gets, the harder it becomes to find one from the other.
Re: [PATCH v5 7/9] s390x/pci: enable adapter event notification for interpreted devices
On 5/2/22 11:19, Niklas Schnelle wrote:
On Mon, 2022-05-02 at 09:48 +0200, Pierre Morel wrote:
On 4/22/22 14:10, Matthew Rosato wrote:
On 4/22/22 5:39 AM, Pierre Morel wrote:
On 4/4/22 20:17, Matthew Rosato wrote:
Use the associated kvm ioctl operation to enable adapter event
notification
and forwarding for devices when requested. This feature will be set up
with or without firmware assist based upon the 'forwarding_assist'
setting.
Signed-off-by: Matthew Rosato
---
hw/s390x/s390-pci-bus.c | 20 ++---
hw/s390x/s390-pci-inst.c| 40 +++--
hw/s390x/s390-pci-kvm.c | 30 +
include/hw/s390x/s390-pci-bus.h | 1 +
include/hw/s390x/s390-pci-kvm.h | 14
5 files changed, 100 insertions(+), 5 deletions(-)
diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index 9c02d31250..47918d2ce9 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -190,7 +190,10 @@ void s390_pci_sclp_deconfigure(SCCB *sccb)
rc = SCLP_RC_NO_ACTION_REQUIRED;
break;
default:
-if (pbdev->summary_ind) {
+if (pbdev->interp && (pbdev->fh & FH_MASK_ENABLE)) {
+/* Interpreted devices were using interrupt forwarding */
+s390_pci_kvm_aif_disable(pbdev);
Same remark as for the kernel part.
The VFIO device is already initialized and the action is on this
device, Shouldn't we use the VFIO device interface instead of the KVM
interface?
I don't necessarily disagree, but in v3 of the kernel series I was told
not to use VFIO ioctls to accomplish tasks that are unique to KVM (e.g.
AEN interpretation) and to instead use a KVM ioctl.
VFIO_DEVICE_SET_IRQS won't work as-is for reasons described in the
kernel series (e.g. we don't see any of the config space notifiers
because of instruction interpretation) -- as far as I can figure we
could add our own s390 code to QEMU to issue VFIO_DEVICE_SET_IRQS
directly for an interpreted device, but I think would also need
s390-specific changes to VFIO_DEVICE_SET_IRQS accommodate this (e.g.
maybe something like a VFIO_IRQ_SET_DATA_S390AEN where we can then
specify the aen information in vfio_irq_set.data -- or something else I
Hi,
yes this in VFIO_DEVICE_SET_IRQS is what I think should be done.
haven't though of yet) -- I can try to look at this some more and see if
I get a good idea.
I understood that the demand was concerning the IOMMU but I may be wrong.
For my opinion, the handling of AEN is not specific to KVM but specific
to the device, for example the code should be the same if Z ever decide
to use XEN or another hypervizor, except for the GISA part but this part
is already implemented in KVM in a way it can be used from a device like
in VFIO AP.
@Alex, what do you think?
Regards,
Pierre
As I understand it the question isn't if it is specific to KVM but
rather if it is specific to virtualization. As vfio-pci is also used
for non virtualization purposes such as with DPDK/SPDK or a fully
emulating QEMU, it should only be in VFIO if it is relevant for these
kinds of user-space PCI accesses too. I'm not an AEN expert but as I
understand it, this does forwarding interrupts into a SIE context which
only makes sense for virtualization not for general user-space PCI.
Being in VFIO kernel part does not mean that this part should be called
from any user of VFIO in userland.
That is a reason why I did propose an extension and not using the
current implementation of VFIO_DEVICE_SET_IRQS as is.
The reason behind is that the AEN hardware handling is device specific:
we need the Function Handle to program AEN.
If the API is through KVM which is device agnostic the implementation in
KVM has to search through the system to find the device being handled to
apply AEN on it.
This not the logical way for me and it is a potential source of problems
for future extensions.
Regards,
Pierre
--
Pierre Morel
IBM Lab Boeblingen
[PATCH qemu v13 12/15] target/riscv: rvv: Add tail agnostic for vector reduction instructions
From: eopXD
Signed-off-by: eop Chen
Reviewed-by: Frank Chang
Reviewed-by: Weiwei Li
---
target/riscv/vector_helper.c | 20
1 file changed, 20 insertions(+)
diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
index f67ec1f249..a319cda969 100644
--- a/target/riscv/vector_helper.c
+++ b/target/riscv/vector_helper.c
@@ -4537,6 +4537,9 @@ void HELPER(NAME)(void *vd, void *v0, void *vs1,
\
{ \
uint32_t vm = vext_vm(desc); \
uint32_t vl = env->vl;\
+uint32_t esz = sizeof(TD);\
+uint32_t vlenb = simd_maxsz(desc);\
+uint32_t vta = vext_vta(desc);\
uint32_t i; \
TD s1 = *((TD *)vs1 + HD(0));\
\
@@ -4549,6 +4552,8 @@ void HELPER(NAME)(void *vd, void *v0, void *vs1,
\
} \
*((TD *)vd + HD(0)) = s1; \
env->vstart = 0; \
+/* set tail elements to 1s */ \
+vext_set_elems_1s(vd, vta, esz, vlenb); \
}
/* vd[0] = sum(vs1[0], vs2[*]) */
@@ -4618,6 +4623,9 @@ void HELPER(NAME)(void *vd, void *v0, void *vs1,
\
{ \
uint32_t vm = vext_vm(desc); \
uint32_t vl = env->vl; \
+uint32_t esz = sizeof(TD); \
+uint32_t vlenb = simd_maxsz(desc); \
+uint32_t vta = vext_vta(desc); \
uint32_t i;\
TD s1 = *((TD *)vs1 + HD(0)); \
\
@@ -4630,6 +4638,8 @@ void HELPER(NAME)(void *vd, void *v0, void *vs1,
\
} \
*((TD *)vd + HD(0)) = s1; \
env->vstart = 0; \
+/* set tail elements to 1s */ \
+vext_set_elems_1s(vd, vta, esz, vlenb);\
}
/* Unordered sum */
@@ -4654,6 +4664,9 @@ void HELPER(vfwredsum_vs_h)(void *vd, void *v0, void *vs1,
{
uint32_t vm = vext_vm(desc);
uint32_t vl = env->vl;
+uint32_t esz = sizeof(uint32_t);
+uint32_t vlenb = simd_maxsz(desc);
+uint32_t vta = vext_vta(desc);
uint32_t i;
uint32_t s1 = *((uint32_t *)vs1 + H4(0));
@@ -4667,6 +4680,8 @@ void HELPER(vfwredsum_vs_h)(void *vd, void *v0, void *vs1,
}
*((uint32_t *)vd + H4(0)) = s1;
env->vstart = 0;
+/* set tail elements to 1s */
+vext_set_elems_1s(vd, vta, esz, vlenb);
}
void HELPER(vfwredsum_vs_w)(void *vd, void *v0, void *vs1,
@@ -4674,6 +4689,9 @@ void HELPER(vfwredsum_vs_w)(void *vd, void *v0, void *vs1,
{
uint32_t vm = vext_vm(desc);
uint32_t vl = env->vl;
+uint32_t esz = sizeof(uint64_t);
+uint32_t vlenb = simd_maxsz(desc);
+uint32_t vta = vext_vta(desc);
uint32_t i;
uint64_t s1 = *((uint64_t *)vs1);
@@ -4687,6 +4705,8 @@ void HELPER(vfwredsum_vs_w)(void *vd, void *v0, void *vs1,
}
*((uint64_t *)vd) = s1;
env->vstart = 0;
+/* set tail elements to 1s */
+vext_set_elems_1s(vd, vta, esz, vlenb);
}
/*
--
2.34.2
[PATCH qemu v13 14/15] target/riscv: rvv: Add tail agnostic for vector permutation instructions
From: eopXD
Signed-off-by: eop Chen
Reviewed-by: Frank Chang
Reviewed-by: Weiwei Li
---
target/riscv/insn_trans/trans_rvv.c.inc | 22 ++
target/riscv/vector_helper.c| 40 +
2 files changed, 62 insertions(+)
diff --git a/target/riscv/insn_trans/trans_rvv.c.inc
b/target/riscv/insn_trans/trans_rvv.c.inc
index ab9f876c28..2c9993844a 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -3744,6 +3744,16 @@ static bool trans_vrgather_vx(DisasContext *s, arg_rmrr
*a)
}
if (a->vm && s->vl_eq_vlmax) {
+if (s->vta && s->lmul < 0) {
+/*
+ * tail elements may pass vlmax when lmul < 0
+ * set tail elements to 1s
+ */
+uint32_t vlenb = s->cfg_ptr->vlen >> 3;
+tcg_gen_gvec_ori(s->sew, vreg_ofs(s, a->rd),
+ vreg_ofs(s, a->rd), -1,
+ vlenb, vlenb);
+}
int scale = s->lmul - (s->sew + 3);
int vlmax = scale < 0 ?
s->cfg_ptr->vlen >> -scale : s->cfg_ptr->vlen << scale;
@@ -3777,6 +3787,16 @@ static bool trans_vrgather_vi(DisasContext *s, arg_rmrr
*a)
}
if (a->vm && s->vl_eq_vlmax) {
+if (s->vta && s->lmul < 0) {
+/*
+ * tail elements may pass vlmax when lmul < 0
+ * set tail elements to 1s
+ */
+uint32_t vlenb = s->cfg_ptr->vlen >> 3;
+tcg_gen_gvec_ori(s->sew, vreg_ofs(s, a->rd),
+ vreg_ofs(s, a->rd), -1,
+ vlenb, vlenb);
+}
int scale = s->lmul - (s->sew + 3);
int vlmax = scale < 0 ?
s->cfg_ptr->vlen >> -scale : s->cfg_ptr->vlen << scale;
@@ -3829,6 +3849,7 @@ static bool trans_vcompress_vm(DisasContext *s, arg_r *a)
tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over);
data = FIELD_DP32(data, VDATA, LMUL, s->lmul);
+data = FIELD_DP32(data, VDATA, VTA, s->vta);
tcg_gen_gvec_4_ptr(vreg_ofs(s, a->rd), vreg_ofs(s, 0),
vreg_ofs(s, a->rs1), vreg_ofs(s, a->rs2),
cpu_env, s->cfg_ptr->vlen / 8,
@@ -3934,6 +3955,7 @@ static bool int_ext_op(DisasContext *s, arg_rmr *a,
uint8_t seq)
}
data = FIELD_DP32(data, VDATA, VM, a->vm);
+data = FIELD_DP32(data, VDATA, VTA, s->vta);
tcg_gen_gvec_3_ptr(vreg_ofs(s, a->rd), vreg_ofs(s, 0),
vreg_ofs(s, a->rs2), cpu_env,
diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
index dcb6d3538c..8b3833b299 100644
--- a/target/riscv/vector_helper.c
+++ b/target/riscv/vector_helper.c
@@ -4933,6 +4933,9 @@ void HELPER(NAME)(void *vd, void *v0, target_ulong s1,
void *vs2, \
{ \
uint32_t vm = vext_vm(desc); \
uint32_t vl = env->vl;\
+uint32_t esz = sizeof(ETYPE); \
+uint32_t total_elems = vext_get_total_elems(env, desc, esz); \
+uint32_t vta = vext_vta(desc);\
target_ulong offset = s1, i_min, i; \
\
i_min = MAX(env->vstart, offset); \
@@ -4942,6 +4945,8 @@ void HELPER(NAME)(void *vd, void *v0, target_ulong s1,
void *vs2, \
} \
*((ETYPE *)vd + H(i)) = *((ETYPE *)vs2 + H(i - offset)); \
} \
+/* set tail elements to 1s */ \
+vext_set_elems_1s(vd, vta, vl * esz, total_elems * esz); \
}
/* vslideup.vx vd, vs2, rs1, vm # vd[i+rs1] = vs2[i] */
@@ -4957,6 +4962,9 @@ void HELPER(NAME)(void *vd, void *v0, target_ulong s1,
void *vs2, \
uint32_t vlmax = vext_max_elems(desc, ctzl(sizeof(ETYPE))); \
uint32_t vm = vext_vm(desc); \
uint32_t vl = env->vl;\
+uint32_t esz = sizeof(ETYPE); \
+uint32_t total_elems = vext_get_total_elems(env, desc, esz); \
+uint32_t vta = vext_vta(desc);\
target_ulong i_max, i;\
\
i_max = MAX(MIN(s1 < vlmax ? vlmax - s1 : 0, vl), env->vstart); \
@@ -4973,6 +4981,8 @@ void HELPER(NAME)(void *vd, void *v0, target_ulong s1,
void *vs2,
[PATCH qemu v13 11/15] target/riscv: rvv: Add tail agnostic for vector floating-point instructions
From: eopXD Compares write mask registers, and so always operate under a tail- agnostic policy. Signed-off-by: eop Chen Reviewed-by: Frank Chang Reviewed-by: Weiwei Li --- target/riscv/insn_trans/trans_rvv.c.inc | 15 + target/riscv/vector_helper.c| 440 +--- 2 files changed, 259 insertions(+), 196 deletions(-) diff --git a/target/riscv/insn_trans/trans_rvv.c.inc b/target/riscv/insn_trans/trans_rvv.c.inc index 46ee673040..a267945267 100644 --- a/target/riscv/insn_trans/trans_rvv.c.inc +++ b/target/riscv/insn_trans/trans_rvv.c.inc @@ -2404,6 +2404,9 @@ static bool trans_##NAME(DisasContext *s, arg_rmrr *a) \ \ data = FIELD_DP32(data, VDATA, VM, a->vm); \ data = FIELD_DP32(data, VDATA, LMUL, s->lmul); \ +data = FIELD_DP32(data, VDATA, VTA, s->vta); \ +data = \ +FIELD_DP32(data, VDATA, VTA_ALL_1S, s->cfg_vta_all_1s);\ tcg_gen_gvec_4_ptr(vreg_ofs(s, a->rd), vreg_ofs(s, 0), \ vreg_ofs(s, a->rs1),\ vreg_ofs(s, a->rs2), cpu_env, \ @@ -2486,6 +2489,9 @@ static bool trans_##NAME(DisasContext *s, arg_rmrr *a) \ gen_set_rm(s, RISCV_FRM_DYN); \ data = FIELD_DP32(data, VDATA, VM, a->vm);\ data = FIELD_DP32(data, VDATA, LMUL, s->lmul);\ +data = FIELD_DP32(data, VDATA, VTA, s->vta); \ +data = FIELD_DP32(data, VDATA, VTA_ALL_1S,\ + s->cfg_vta_all_1s); \ return opfvf_trans(a->rd, a->rs1, a->rs2, data, \ fns[s->sew - 1], s); \ } \ @@ -2524,6 +2530,7 @@ static bool trans_##NAME(DisasContext *s, arg_rmrr *a) \ \ data = FIELD_DP32(data, VDATA, VM, a->vm); \ data = FIELD_DP32(data, VDATA, LMUL, s->lmul); \ +data = FIELD_DP32(data, VDATA, VTA, s->vta); \ tcg_gen_gvec_4_ptr(vreg_ofs(s, a->rd), vreg_ofs(s, 0), \ vreg_ofs(s, a->rs1), \ vreg_ofs(s, a->rs2), cpu_env, \ @@ -2563,6 +2570,7 @@ static bool trans_##NAME(DisasContext *s, arg_rmrr *a) \ gen_set_rm(s, RISCV_FRM_DYN);\ data = FIELD_DP32(data, VDATA, VM, a->vm); \ data = FIELD_DP32(data, VDATA, LMUL, s->lmul); \ +data = FIELD_DP32(data, VDATA, VTA, s->vta); \ return opfvf_trans(a->rd, a->rs1, a->rs2, data, \ fns[s->sew - 1], s); \ }\ @@ -2599,6 +2607,7 @@ static bool trans_##NAME(DisasContext *s, arg_rmrr *a) \ \ data = FIELD_DP32(data, VDATA, VM, a->vm); \ data = FIELD_DP32(data, VDATA, LMUL, s->lmul); \ +data = FIELD_DP32(data, VDATA, VTA, s->vta); \ tcg_gen_gvec_4_ptr(vreg_ofs(s, a->rd), vreg_ofs(s, 0), \ vreg_ofs(s, a->rs1),\ vreg_ofs(s, a->rs2), cpu_env, \ @@ -2638,6 +2647,7 @@ static bool trans_##NAME(DisasContext *s, arg_rmrr *a) \ gen_set_rm(s, RISCV_FRM_DYN);\ data = FIELD_DP32(data, VDATA, VM, a->vm); \ data = FIELD_DP32(data, VDATA, LMUL, s->lmul); \ +data = FIELD_DP32(data, VDATA, VTA, s->vta); \ return opfvf_trans(a->rd, a->rs1, a->rs2, data, \ fns[s->sew - 1], s); \ }\ @@ -2721,6 +2731,7 @@ static bool do_opfv(DisasContext *s, arg_rmr *a, data = FIELD_DP32(data, VDATA, VM, a->vm); data = FIELD_DP32(data, VDATA, LMUL, s->lmul); +data = FIELD_DP32(data, VDATA, VTA, s->vta); tcg_gen_gvec_3_ptr(vreg_ofs(s, a->rd), vreg_ofs(s, 0), vreg_ofs(s, a->rs2), cpu_env, s->cfg_ptr->vlen / 8, @@ -2935,6 +2946,7 @@ static bool trans_##NAME(DisasContext *s, arg_rmr *a) \ \ data = FIELD_DP32(data, VDATA, VM, a->vm); \ data = FIELD_DP32(data, VDATA, LMUL, s->lmul);
[PATCH qemu v13 06/15] target/riscv: rvv: Add tail agnostic for vx, vvm, vxm instructions
From: eopXD
`vmadc` and `vmsbc` produces a mask value, they always operate with
a tail agnostic policy.
Signed-off-by: eop Chen
Reviewed-by: Frank Chang
Reviewed-by: Weiwei Li
---
target/riscv/insn_trans/trans_rvv.c.inc | 29 +++
target/riscv/internals.h| 5 +-
target/riscv/vector_helper.c| 314 +---
3 files changed, 208 insertions(+), 140 deletions(-)
diff --git a/target/riscv/insn_trans/trans_rvv.c.inc
b/target/riscv/insn_trans/trans_rvv.c.inc
index 99691f1b9f..d15858fc6f 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -1299,6 +1299,8 @@ static bool opivx_trans(uint32_t vd, uint32_t rs1,
uint32_t vs2, uint32_t vm,
data = FIELD_DP32(data, VDATA, VM, vm);
data = FIELD_DP32(data, VDATA, LMUL, s->lmul);
+data = FIELD_DP32(data, VDATA, VTA, s->vta);
+data = FIELD_DP32(data, VDATA, VTA_ALL_1S, s->cfg_vta_all_1s);
desc = tcg_constant_i32(simd_desc(s->cfg_ptr->vlen / 8,
s->cfg_ptr->vlen / 8, data));
@@ -1335,6 +1337,16 @@ do_opivx_gvec(DisasContext *s, arg_rmrr *a, GVecGen2sFn
*gvec_fn,
}
if (a->vm && s->vl_eq_vlmax) {
+if (s->vta && s->lmul < 0) {
+/*
+ * tail elements may pass vlmax when lmul < 0
+ * set tail elements to 1s
+ */
+uint32_t vlenb = s->cfg_ptr->vlen >> 3;
+tcg_gen_gvec_ori(s->sew, vreg_ofs(s, a->rd),
+ vreg_ofs(s, a->rd), -1,
+ vlenb, vlenb);
+}
TCGv_i64 src1 = tcg_temp_new_i64();
tcg_gen_ext_tl_i64(src1, get_gpr(s, a->rs1, EXT_SIGN));
@@ -1464,6 +1476,8 @@ static bool opivi_trans(uint32_t vd, uint32_t imm,
uint32_t vs2, uint32_t vm,
data = FIELD_DP32(data, VDATA, VM, vm);
data = FIELD_DP32(data, VDATA, LMUL, s->lmul);
+data = FIELD_DP32(data, VDATA, VTA, s->vta);
+data = FIELD_DP32(data, VDATA, VTA_ALL_1S, s->cfg_vta_all_1s);
desc = tcg_constant_i32(simd_desc(s->cfg_ptr->vlen / 8,
s->cfg_ptr->vlen / 8, data));
@@ -1493,6 +1507,16 @@ do_opivi_gvec(DisasContext *s, arg_rmrr *a, GVecGen2iFn
*gvec_fn,
}
if (a->vm && s->vl_eq_vlmax) {
+if (s->vta && s->lmul < 0) {
+/*
+ * tail elements may pass vlmax when lmul < 0
+ * set tail elements to 1s
+ */
+uint32_t vlenb = s->cfg_ptr->vlen >> 3;
+tcg_gen_gvec_ori(s->sew, vreg_ofs(s, a->rd),
+ vreg_ofs(s, a->rd), -1,
+ vlenb, vlenb);
+}
gvec_fn(s->sew, vreg_ofs(s, a->rd), vreg_ofs(s, a->rs2),
extract_imm(s, a->rs1, imm_mode), MAXSZ(s), MAXSZ(s));
mark_vs_dirty(s);
@@ -1546,6 +1570,7 @@ static bool do_opivv_widen(DisasContext *s, arg_rmrr *a,
data = FIELD_DP32(data, VDATA, VM, a->vm);
data = FIELD_DP32(data, VDATA, LMUL, s->lmul);
+data = FIELD_DP32(data, VDATA, VTA, s->vta);
tcg_gen_gvec_4_ptr(vreg_ofs(s, a->rd), vreg_ofs(s, 0),
vreg_ofs(s, a->rs1),
vreg_ofs(s, a->rs2),
@@ -1627,6 +1652,7 @@ static bool do_opiwv_widen(DisasContext *s, arg_rmrr *a,
data = FIELD_DP32(data, VDATA, VM, a->vm);
data = FIELD_DP32(data, VDATA, LMUL, s->lmul);
+data = FIELD_DP32(data, VDATA, VTA, s->vta);
tcg_gen_gvec_4_ptr(vreg_ofs(s, a->rd), vreg_ofs(s, 0),
vreg_ofs(s, a->rs1),
vreg_ofs(s, a->rs2),
@@ -1705,6 +1731,9 @@ static bool trans_##NAME(DisasContext *s, arg_rmrr *a)
\
\
data = FIELD_DP32(data, VDATA, VM, a->vm); \
data = FIELD_DP32(data, VDATA, LMUL, s->lmul); \
+data = FIELD_DP32(data, VDATA, VTA, s->vta); \
+data = \
+FIELD_DP32(data, VDATA, VTA_ALL_1S, s->cfg_vta_all_1s);\
tcg_gen_gvec_4_ptr(vreg_ofs(s, a->rd), vreg_ofs(s, 0), \
vreg_ofs(s, a->rs1),\
vreg_ofs(s, a->rs2), cpu_env, \
diff --git a/target/riscv/internals.h b/target/riscv/internals.h
index 512c6c30cf..193ce57a6d 100644
--- a/target/riscv/internals.h
+++ b/target/riscv/internals.h
@@ -25,8 +25,9 @@
FIELD(VDATA, VM, 0, 1)
FIELD(VDATA, LMUL, 1, 3)
FIELD(VDATA, VTA, 4, 1)
-FIELD(VDATA, NF, 5, 4)
-FIELD(VDATA, WD, 5, 1)
+FIELD(VDATA, VTA_ALL_1S, 5, 1)
+FIELD(VDATA, NF, 6, 4)
+FIELD(VDATA, WD, 6, 1)
/* float point classify helpers */
target_ulong fclass_h(uint64_t frs1);
diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
index f1a0b4ced7..e00f9353b9 100644
---
[PATCH qemu v13 08/15] target/riscv: rvv: Add tail agnostic for vector integer comparison instructions
From: eopXD
Compares write mask registers, and so always operate under a tail-
agnostic policy.
Signed-off-by: eop Chen
Reviewed-by: Frank Chang
Reviewed-by: Weiwei Li
---
target/riscv/vector_helper.c | 18 ++
1 file changed, 18 insertions(+)
diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
index c1ae2ea2f1..ddaf364573 100644
--- a/target/riscv/vector_helper.c
+++ b/target/riscv/vector_helper.c
@@ -1373,6 +1373,8 @@ void HELPER(NAME)(void *vd, void *v0, void *vs1, void
*vs2, \
{ \
uint32_t vm = vext_vm(desc); \
uint32_t vl = env->vl;\
+uint32_t total_elems = env_archcpu(env)->cfg.vlen;\
+uint32_t vta_all_1s = vext_vta_all_1s(desc); \
uint32_t i; \
\
for (i = env->vstart; i < vl; i++) { \
@@ -1384,6 +1386,13 @@ void HELPER(NAME)(void *vd, void *v0, void *vs1, void
*vs2, \
vext_set_elem_mask(vd, i, DO_OP(s2, s1)); \
} \
env->vstart = 0; \
+/* mask destination register are always tail-agnostic */ \
+/* set tail elements to 1s */ \
+if (vta_all_1s) { \
+for (; i < total_elems; i++) {\
+vext_set_elem_mask(vd, i, 1); \
+} \
+} \
}
GEN_VEXT_CMP_VV(vmseq_vv_b, uint8_t, H1, DO_MSEQ)
@@ -1422,6 +1431,8 @@ void HELPER(NAME)(void *vd, void *v0, target_ulong s1,
void *vs2, \
{ \
uint32_t vm = vext_vm(desc);\
uint32_t vl = env->vl; \
+uint32_t total_elems = env_archcpu(env)->cfg.vlen; \
+uint32_t vta_all_1s = vext_vta_all_1s(desc);\
uint32_t i; \
\
for (i = env->vstart; i < vl; i++) {\
@@ -1433,6 +1444,13 @@ void HELPER(NAME)(void *vd, void *v0, target_ulong s1,
void *vs2, \
DO_OP(s2, (ETYPE)(target_long)s1)); \
} \
env->vstart = 0;\
+/* mask destination register are always tail-agnostic */\
+/* set tail elements to 1s */ \
+if (vta_all_1s) { \
+for (; i < total_elems; i++) { \
+vext_set_elem_mask(vd, i, 1); \
+} \
+} \
}
GEN_VEXT_CMP_VX(vmseq_vx_b, uint8_t, H1, DO_MSEQ)
--
2.34.2
[PATCH qemu v13 09/15] target/riscv: rvv: Add tail agnostic for vector integer merge and move instructions
From: eopXD
Signed-off-by: eop Chen
Reviewed-by: Frank Chang
Reviewed-by: Weiwei Li
---
target/riscv/insn_trans/trans_rvv.c.inc | 44 +
target/riscv/vector_helper.c| 20 +++
2 files changed, 64 insertions(+)
diff --git a/target/riscv/insn_trans/trans_rvv.c.inc
b/target/riscv/insn_trans/trans_rvv.c.inc
index 430847b0f9..46ee673040 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -2123,11 +2123,22 @@ static bool trans_vmv_v_v(DisasContext *s, arg_vmv_v_v
*a)
/* vmv.v.v has rs2 = 0 and vm = 1 */
vext_check_sss(s, a->rd, a->rs1, 0, 1)) {
if (s->vl_eq_vlmax) {
+if (s->vta && s->lmul < 0) {
+/*
+ * tail elements may pass vlmax when lmul < 0
+ * set tail elements to 1s
+ */
+uint32_t vlenb = s->cfg_ptr->vlen >> 3;
+tcg_gen_gvec_ori(s->sew, vreg_ofs(s, a->rd),
+ vreg_ofs(s, a->rd), -1,
+ vlenb, vlenb);
+}
tcg_gen_gvec_mov(s->sew, vreg_ofs(s, a->rd),
vreg_ofs(s, a->rs1),
MAXSZ(s), MAXSZ(s));
} else {
uint32_t data = FIELD_DP32(0, VDATA, LMUL, s->lmul);
+data = FIELD_DP32(data, VDATA, VTA, s->vta);
static gen_helper_gvec_2_ptr * const fns[4] = {
gen_helper_vmv_v_v_b, gen_helper_vmv_v_v_h,
gen_helper_vmv_v_v_w, gen_helper_vmv_v_v_d,
@@ -2163,6 +2174,16 @@ static bool trans_vmv_v_x(DisasContext *s, arg_vmv_v_x
*a)
s1 = get_gpr(s, a->rs1, EXT_SIGN);
if (s->vl_eq_vlmax) {
+if (s->vta && s->lmul < 0) {
+/*
+ * tail elements may pass vlmax when lmul < 0
+ * set tail elements to 1s
+ */
+uint32_t vlenb = s->cfg_ptr->vlen >> 3;
+tcg_gen_gvec_ori(s->sew, vreg_ofs(s, a->rd),
+ vreg_ofs(s, a->rd), -1,
+ vlenb, vlenb);
+}
tcg_gen_gvec_dup_tl(s->sew, vreg_ofs(s, a->rd),
MAXSZ(s), MAXSZ(s), s1);
} else {
@@ -2170,6 +2191,7 @@ static bool trans_vmv_v_x(DisasContext *s, arg_vmv_v_x *a)
TCGv_i64 s1_i64 = tcg_temp_new_i64();
TCGv_ptr dest = tcg_temp_new_ptr();
uint32_t data = FIELD_DP32(0, VDATA, LMUL, s->lmul);
+data = FIELD_DP32(data, VDATA, VTA, s->vta);
static gen_helper_vmv_vx * const fns[4] = {
gen_helper_vmv_v_x_b, gen_helper_vmv_v_x_h,
gen_helper_vmv_v_x_w, gen_helper_vmv_v_x_d,
@@ -2200,6 +,16 @@ static bool trans_vmv_v_i(DisasContext *s, arg_vmv_v_i
*a)
vext_check_ss(s, a->rd, 0, 1)) {
int64_t simm = sextract64(a->rs1, 0, 5);
if (s->vl_eq_vlmax) {
+if (s->vta && s->lmul < 0) {
+/*
+ * tail elements may pass vlmax when lmul < 0
+ * set tail elements to 1s
+ */
+uint32_t vlenb = s->cfg_ptr->vlen >> 3;
+tcg_gen_gvec_ori(s->sew, vreg_ofs(s, a->rd),
+ vreg_ofs(s, a->rd), -1,
+ vlenb, vlenb);
+}
tcg_gen_gvec_dup_imm(s->sew, vreg_ofs(s, a->rd),
MAXSZ(s), MAXSZ(s), simm);
mark_vs_dirty(s);
@@ -2208,6 +2240,7 @@ static bool trans_vmv_v_i(DisasContext *s, arg_vmv_v_i *a)
TCGv_i64 s1;
TCGv_ptr dest;
uint32_t data = FIELD_DP32(0, VDATA, LMUL, s->lmul);
+data = FIELD_DP32(data, VDATA, VTA, s->vta);
static gen_helper_vmv_vx * const fns[4] = {
gen_helper_vmv_v_x_b, gen_helper_vmv_v_x_h,
gen_helper_vmv_v_x_w, gen_helper_vmv_v_x_d,
@@ -2780,6 +2813,16 @@ static bool trans_vfmv_v_f(DisasContext *s, arg_vfmv_v_f
*a)
TCGv_i64 t1;
if (s->vl_eq_vlmax) {
+if (s->vta && s->lmul < 0) {
+/*
+ * tail elements may pass vlmax when lmul < 0
+ * set tail elements to 1s
+ */
+uint32_t vlenb = s->cfg_ptr->vlen >> 3;
+tcg_gen_gvec_ori(s->sew, vreg_ofs(s, a->rd),
+ vreg_ofs(s, a->rd), -1,
+ vlenb, vlenb);
+}
t1 = tcg_temp_new_i64();
/* NaN-box f[rs1] */
do_nanbox(s, t1, cpu_fpr[a->rs1]);
@@ -2791,6 +2834,7 @@ static bool trans_vfmv_v_f(DisasContext *s, arg_vfmv_v_f
*a)
TCGv_ptr dest;
TCGv_i32 desc;
uint32_t data = FIELD_DP32(0, VDATA, LMUL, s->lmul);
+data = FIELD_DP32(data, VDATA, VTA,
[PATCH qemu v13 15/15] target/riscv: rvv: Add option 'rvv_ta_all_1s' to enable optional tail agnostic behavior
From: eopXD
According to v-spec, tail agnostic behavior can be either kept as
undisturbed or set elements' bits to all 1s. To distinguish the
difference of tail policies, QEMU should be able to simulate the tail
agnostic behavior as "set tail elements' bits to all 1s".
There are multiple possibility for agnostic elements according to
v-spec. The main intent of this patch-set tries to add option that
can distinguish between tail policies. Setting agnostic elements to
all 1s allows QEMU to express this.
This commit adds option 'rvv_ta_all_1s' is added to enable the
behavior, it is default as disabled.
Signed-off-by: eop Chen
Reviewed-by: Frank Chang
Reviewed-by: Weiwei Li
---
target/riscv/cpu.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index ddda4906ff..cd4cf4b41e 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -810,6 +810,7 @@ static Property riscv_cpu_properties[] = {
DEFINE_PROP_BOOL("x-aia", RISCVCPU, cfg.aia, false),
DEFINE_PROP_UINT64("resetvec", RISCVCPU, cfg.resetvec, DEFAULT_RSTVEC),
+DEFINE_PROP_BOOL("rvv_ta_all_1s", RISCVCPU, cfg.rvv_ta_all_1s, false),
DEFINE_PROP_END_OF_LIST(),
};
--
2.34.2
[PATCH qemu v13 04/15] target/riscv: rvv: Add tail agnostic for vv instructions
From: eopXD
According to v-spec, tail agnostic behavior can be either kept as
undisturbed or set elements' bits to all 1s. To distinguish the
difference of tail policies, QEMU should be able to simulate the tail
agnostic behavior as "set tail elements' bits to all 1s".
There are multiple possibility for agnostic elements according to
v-spec. The main intent of this patch-set tries to add option that
can distinguish between tail policies. Setting agnostic elements to
all 1s allows QEMU to express this.
This is the first commit regarding the optional tail agnostic
behavior. Follow-up commits will add this optional behavior
for all rvv instructions.
Signed-off-by: eop Chen
Reviewed-by: Frank Chang
Reviewed-by: Weiwei Li
---
target/riscv/cpu.h | 2 +
target/riscv/cpu_helper.c | 2 +
target/riscv/insn_trans/trans_rvv.c.inc | 11 +
target/riscv/internals.h| 5 +-
target/riscv/translate.c| 2 +
target/riscv/vector_helper.c| 295 +---
6 files changed, 186 insertions(+), 131 deletions(-)
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index c069fe85fa..8c4a79b5a0 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -369,6 +369,7 @@ struct RISCVCPUConfig {
bool ext_zhinxmin;
bool ext_zve32f;
bool ext_zve64f;
+bool rvv_ta_all_1s;
/* Vendor-specific custom extensions */
bool ext_XVentanaCondOps;
@@ -516,6 +517,7 @@ FIELD(TB_FLAGS, XL, 20, 2)
/* If PointerMasking should be applied */
FIELD(TB_FLAGS, PM_MASK_ENABLED, 22, 1)
FIELD(TB_FLAGS, PM_BASE_ENABLED, 23, 1)
+FIELD(TB_FLAGS, VTA, 24, 1)
#ifdef TARGET_RISCV32
#define riscv_cpu_mxl(env) ((void)(env), MXL_RV32)
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 1c60fb2e80..2941c88c31 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -65,6 +65,8 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong
*pc,
flags = FIELD_DP32(flags, TB_FLAGS, LMUL,
FIELD_EX64(env->vtype, VTYPE, VLMUL));
flags = FIELD_DP32(flags, TB_FLAGS, VL_EQ_VLMAX, vl_eq_vlmax);
+flags = FIELD_DP32(flags, TB_FLAGS, VTA,
+FIELD_EX64(env->vtype, VTYPE, VTA));
} else {
flags = FIELD_DP32(flags, TB_FLAGS, VILL, 1);
}
diff --git a/target/riscv/insn_trans/trans_rvv.c.inc
b/target/riscv/insn_trans/trans_rvv.c.inc
index 57953923d5..cc80bf00ff 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -1223,6 +1223,16 @@ do_opivv_gvec(DisasContext *s, arg_rmrr *a, GVecGen3Fn
*gvec_fn,
tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over);
if (a->vm && s->vl_eq_vlmax) {
+if (s->vta && s->lmul < 0) {
+/*
+ * tail elements may pass vlmax when lmul < 0
+ * set tail elements to 1s
+ */
+uint32_t vlenb = s->cfg_ptr->vlen >> 3;
+tcg_gen_gvec_ori(s->sew, vreg_ofs(s, a->rd),
+ vreg_ofs(s, a->rd), -1,
+ vlenb, vlenb);
+}
gvec_fn(s->sew, vreg_ofs(s, a->rd),
vreg_ofs(s, a->rs2), vreg_ofs(s, a->rs1),
MAXSZ(s), MAXSZ(s));
@@ -1231,6 +1241,7 @@ do_opivv_gvec(DisasContext *s, arg_rmrr *a, GVecGen3Fn
*gvec_fn,
data = FIELD_DP32(data, VDATA, VM, a->vm);
data = FIELD_DP32(data, VDATA, LMUL, s->lmul);
+data = FIELD_DP32(data, VDATA, VTA, s->vta);
tcg_gen_gvec_4_ptr(vreg_ofs(s, a->rd), vreg_ofs(s, 0),
vreg_ofs(s, a->rs1), vreg_ofs(s, a->rs2),
cpu_env, s->cfg_ptr->vlen / 8,
diff --git a/target/riscv/internals.h b/target/riscv/internals.h
index dbb322bfa7..512c6c30cf 100644
--- a/target/riscv/internals.h
+++ b/target/riscv/internals.h
@@ -24,8 +24,9 @@
/* share data between vector helpers and decode code */
FIELD(VDATA, VM, 0, 1)
FIELD(VDATA, LMUL, 1, 3)
-FIELD(VDATA, NF, 4, 4)
-FIELD(VDATA, WD, 4, 1)
+FIELD(VDATA, VTA, 4, 1)
+FIELD(VDATA, NF, 5, 4)
+FIELD(VDATA, WD, 5, 1)
/* float point classify helpers */
target_ulong fclass_h(uint64_t frs1);
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index fac998a6b5..7775dade26 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -94,6 +94,7 @@ typedef struct DisasContext {
*/
int8_t lmul;
uint8_t sew;
+uint8_t vta;
target_ulong vstart;
bool vl_eq_vlmax;
uint8_t ntemp;
@@ -1083,6 +1084,7 @@ static void riscv_tr_init_disas_context(DisasContextBase
*dcbase, CPUState *cs)
ctx->vill = FIELD_EX32(tb_flags, TB_FLAGS, VILL);
ctx->sew = FIELD_EX32(tb_flags, TB_FLAGS, SEW);
ctx->lmul = sextract32(FIELD_EX32(tb_flags, TB_FLAGS, LMUL), 0, 3);
+ctx->vta = FIELD_EX32(tb_flags, TB_FLAGS, VTA) && cpu->cfg.rvv_ta_all_1s;
ctx->vstart = env->vstart;
[PATCH qemu v13 01/15] target/riscv: rvv: Prune redundant ESZ, DSZ parameter passed
From: eopXD
No functional change intended in this commit.
Signed-off-by: eop Chen
Reviewed-by: Frank Chang
Reviewed-by: Weiwei Li
Reviewed-by: Alistair Francis
---
target/riscv/vector_helper.c | 1132 +-
1 file changed, 565 insertions(+), 567 deletions(-)
diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
index 3bd4aac9c9..e94caf1a3c 100644
--- a/target/riscv/vector_helper.c
+++ b/target/riscv/vector_helper.c
@@ -710,7 +710,6 @@ RVVCALL(OPIVV2, vsub_vv_d, OP_SSS_D, H8, H8, H8, DO_SUB)
static void do_vext_vv(void *vd, void *v0, void *vs1, void *vs2,
CPURISCVState *env, uint32_t desc,
- uint32_t esz, uint32_t dsz,
opivv2_fn *fn)
{
uint32_t vm = vext_vm(desc);
@@ -727,23 +726,23 @@ static void do_vext_vv(void *vd, void *v0, void *vs1,
void *vs2,
}
/* generate the helpers for OPIVV */
-#define GEN_VEXT_VV(NAME, ESZ, DSZ) \
+#define GEN_VEXT_VV(NAME) \
void HELPER(NAME)(void *vd, void *v0, void *vs1, \
void *vs2, CPURISCVState *env, \
uint32_t desc) \
{ \
-do_vext_vv(vd, v0, vs1, vs2, env, desc, ESZ, DSZ, \
+do_vext_vv(vd, v0, vs1, vs2, env, desc, \
do_##NAME);\
}
-GEN_VEXT_VV(vadd_vv_b, 1, 1)
-GEN_VEXT_VV(vadd_vv_h, 2, 2)
-GEN_VEXT_VV(vadd_vv_w, 4, 4)
-GEN_VEXT_VV(vadd_vv_d, 8, 8)
-GEN_VEXT_VV(vsub_vv_b, 1, 1)
-GEN_VEXT_VV(vsub_vv_h, 2, 2)
-GEN_VEXT_VV(vsub_vv_w, 4, 4)
-GEN_VEXT_VV(vsub_vv_d, 8, 8)
+GEN_VEXT_VV(vadd_vv_b)
+GEN_VEXT_VV(vadd_vv_h)
+GEN_VEXT_VV(vadd_vv_w)
+GEN_VEXT_VV(vadd_vv_d)
+GEN_VEXT_VV(vsub_vv_b)
+GEN_VEXT_VV(vsub_vv_h)
+GEN_VEXT_VV(vsub_vv_w)
+GEN_VEXT_VV(vsub_vv_d)
typedef void opivx2_fn(void *vd, target_long s1, void *vs2, int i);
@@ -773,7 +772,6 @@ RVVCALL(OPIVX2, vrsub_vx_d, OP_SSS_D, H8, H8, DO_RSUB)
static void do_vext_vx(void *vd, void *v0, target_long s1, void *vs2,
CPURISCVState *env, uint32_t desc,
- uint32_t esz, uint32_t dsz,
opivx2_fn fn)
{
uint32_t vm = vext_vm(desc);
@@ -790,27 +788,27 @@ static void do_vext_vx(void *vd, void *v0, target_long
s1, void *vs2,
}
/* generate the helpers for OPIVX */
-#define GEN_VEXT_VX(NAME, ESZ, DSZ) \
+#define GEN_VEXT_VX(NAME) \
void HELPER(NAME)(void *vd, void *v0, target_ulong s1,\
void *vs2, CPURISCVState *env, \
uint32_t desc) \
{ \
-do_vext_vx(vd, v0, s1, vs2, env, desc, ESZ, DSZ, \
+do_vext_vx(vd, v0, s1, vs2, env, desc,\
do_##NAME);\
}
-GEN_VEXT_VX(vadd_vx_b, 1, 1)
-GEN_VEXT_VX(vadd_vx_h, 2, 2)
-GEN_VEXT_VX(vadd_vx_w, 4, 4)
-GEN_VEXT_VX(vadd_vx_d, 8, 8)
-GEN_VEXT_VX(vsub_vx_b, 1, 1)
-GEN_VEXT_VX(vsub_vx_h, 2, 2)
-GEN_VEXT_VX(vsub_vx_w, 4, 4)
-GEN_VEXT_VX(vsub_vx_d, 8, 8)
-GEN_VEXT_VX(vrsub_vx_b, 1, 1)
-GEN_VEXT_VX(vrsub_vx_h, 2, 2)
-GEN_VEXT_VX(vrsub_vx_w, 4, 4)
-GEN_VEXT_VX(vrsub_vx_d, 8, 8)
+GEN_VEXT_VX(vadd_vx_b)
+GEN_VEXT_VX(vadd_vx_h)
+GEN_VEXT_VX(vadd_vx_w)
+GEN_VEXT_VX(vadd_vx_d)
+GEN_VEXT_VX(vsub_vx_b)
+GEN_VEXT_VX(vsub_vx_h)
+GEN_VEXT_VX(vsub_vx_w)
+GEN_VEXT_VX(vsub_vx_d)
+GEN_VEXT_VX(vrsub_vx_b)
+GEN_VEXT_VX(vrsub_vx_h)
+GEN_VEXT_VX(vrsub_vx_w)
+GEN_VEXT_VX(vrsub_vx_d)
void HELPER(vec_rsubs8)(void *d, void *a, uint64_t b, uint32_t desc)
{
@@ -889,30 +887,30 @@ RVVCALL(OPIVV2, vwadd_wv_w, WOP_WSSS_W, H8, H4, H4,
DO_ADD)
RVVCALL(OPIVV2, vwsub_wv_b, WOP_WSSS_B, H2, H1, H1, DO_SUB)
RVVCALL(OPIVV2, vwsub_wv_h, WOP_WSSS_H, H4, H2, H2, DO_SUB)
RVVCALL(OPIVV2, vwsub_wv_w, WOP_WSSS_W, H8, H4, H4, DO_SUB)
-GEN_VEXT_VV(vwaddu_vv_b, 1, 2)
-GEN_VEXT_VV(vwaddu_vv_h, 2, 4)
-GEN_VEXT_VV(vwaddu_vv_w, 4, 8)
-GEN_VEXT_VV(vwsubu_vv_b, 1, 2)
-GEN_VEXT_VV(vwsubu_vv_h, 2, 4)
-GEN_VEXT_VV(vwsubu_vv_w, 4, 8)
-GEN_VEXT_VV(vwadd_vv_b, 1, 2)
-GEN_VEXT_VV(vwadd_vv_h, 2, 4)
-GEN_VEXT_VV(vwadd_vv_w, 4, 8)
-GEN_VEXT_VV(vwsub_vv_b, 1, 2)
-GEN_VEXT_VV(vwsub_vv_h, 2, 4)
-GEN_VEXT_VV(vwsub_vv_w, 4, 8)
-GEN_VEXT_VV(vwaddu_wv_b, 1, 2)
-GEN_VEXT_VV(vwaddu_wv_h, 2, 4)
-GEN_VEXT_VV(vwaddu_wv_w, 4, 8)
-GEN_VEXT_VV(vwsubu_wv_b, 1, 2)
-GEN_VEXT_VV(vwsubu_wv_h, 2, 4)
-GEN_VEXT_VV(vwsubu_wv_w, 4, 8)
-GEN_VEXT_VV(vwadd_wv_b, 1, 2)
-GEN_VEXT_VV(vwadd_wv_h, 2, 4)
-GEN_VEXT_VV(vwadd_wv_w, 4, 8)
-GEN_VEXT_VV(vwsub_wv_b, 1, 2)
-GEN_VEXT_VV(vwsub_wv_h, 2, 4)
-GEN_VEXT_VV(vwsub_wv_w, 4, 8)
+GEN_VEXT_VV(vwaddu_vv_b)
+GEN_VEXT_VV(vwaddu_vv_h)
+GEN_VEXT_VV(vwaddu_vv_w)
+GEN_VEXT_VV(vwsubu_vv_b)
+GEN_VEXT_VV(vwsubu_vv_h)
+GEN_VEXT_VV(vwsubu_vv_w)
+GEN_VEXT_VV(vwadd_vv_b)
+GEN_VEXT_VV(vwadd_vv_h)
[PATCH qemu v13 03/15] target/riscv: rvv: Early exit when vstart >= vl
From: eopXD
According to v-spec (section 5.4):
When vstart ≥ vl, there are no body elements, and no elements are
updated in any destination vector register group, including that
no tail elements are updated with agnostic values.
vmsbf.m, vmsif.m, vmsof.m, viota.m, vcompress instructions themselves
require vstart to be zero. So they don't need the early exit.
Signed-off-by: eop Chen
Reviewed-by: Frank Chang
Reviewed-by: Weiwei Li
Acked-by: Alistair Francis
---
target/riscv/insn_trans/trans_rvv.c.inc | 27 +
1 file changed, 27 insertions(+)
diff --git a/target/riscv/insn_trans/trans_rvv.c.inc
b/target/riscv/insn_trans/trans_rvv.c.inc
index 275fded6e4..57953923d5 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -652,6 +652,7 @@ static bool ldst_us_trans(uint32_t vd, uint32_t rs1,
uint32_t data,
TCGLabel *over = gen_new_label();
tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over);
+tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over);
dest = tcg_temp_new_ptr();
mask = tcg_temp_new_ptr();
@@ -818,6 +819,7 @@ static bool ldst_stride_trans(uint32_t vd, uint32_t rs1,
uint32_t rs2,
TCGLabel *over = gen_new_label();
tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over);
+tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over);
dest = tcg_temp_new_ptr();
mask = tcg_temp_new_ptr();
@@ -925,6 +927,7 @@ static bool ldst_index_trans(uint32_t vd, uint32_t rs1,
uint32_t vs2,
TCGLabel *over = gen_new_label();
tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over);
+tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over);
dest = tcg_temp_new_ptr();
mask = tcg_temp_new_ptr();
@@ -1067,6 +1070,7 @@ static bool ldff_trans(uint32_t vd, uint32_t rs1,
uint32_t data,
TCGLabel *over = gen_new_label();
tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over);
+tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over);
dest = tcg_temp_new_ptr();
mask = tcg_temp_new_ptr();
@@ -1216,6 +1220,7 @@ do_opivv_gvec(DisasContext *s, arg_rmrr *a, GVecGen3Fn
*gvec_fn,
}
tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over);
+tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over);
if (a->vm && s->vl_eq_vlmax) {
gvec_fn(s->sew, vreg_ofs(s, a->rd),
@@ -1263,6 +1268,7 @@ static bool opivx_trans(uint32_t vd, uint32_t rs1,
uint32_t vs2, uint32_t vm,
TCGLabel *over = gen_new_label();
tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over);
+tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over);
dest = tcg_temp_new_ptr();
mask = tcg_temp_new_ptr();
@@ -1427,6 +1433,7 @@ static bool opivi_trans(uint32_t vd, uint32_t imm,
uint32_t vs2, uint32_t vm,
TCGLabel *over = gen_new_label();
tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over);
+tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over);
dest = tcg_temp_new_ptr();
mask = tcg_temp_new_ptr();
@@ -1513,6 +1520,7 @@ static bool do_opivv_widen(DisasContext *s, arg_rmrr *a,
uint32_t data = 0;
TCGLabel *over = gen_new_label();
tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over);
+tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over);
data = FIELD_DP32(data, VDATA, VM, a->vm);
data = FIELD_DP32(data, VDATA, LMUL, s->lmul);
@@ -1593,6 +1601,7 @@ static bool do_opiwv_widen(DisasContext *s, arg_rmrr *a,
uint32_t data = 0;
TCGLabel *over = gen_new_label();
tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over);
+tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over);
data = FIELD_DP32(data, VDATA, VM, a->vm);
data = FIELD_DP32(data, VDATA, LMUL, s->lmul);
@@ -1670,6 +1679,7 @@ static bool trans_##NAME(DisasContext *s, arg_rmrr *a)
\
}; \
TCGLabel *over = gen_new_label(); \
tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over); \
+tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over); \
\
data = FIELD_DP32(data, VDATA, VM, a->vm); \
data = FIELD_DP32(data, VDATA, LMUL, s->lmul); \
@@ -1851,6 +1861,7 @@ static bool trans_##NAME(DisasContext *s, arg_rmrr *a)
\
}; \
TCGLabel *over = gen_new_label(); \
tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over); \
+tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over); \
\
data = FIELD_DP32(data, VDATA, VM, a->vm); \
data = FIELD_DP32(data, VDATA, LMUL, s->lmul); \
@@ -2061,6 +2072,7 @@ static bool
[PATCH qemu v13 13/15] target/riscv: rvv: Add tail agnostic for vector mask instructions
From: eopXD
The tail elements in the destination mask register are updated under
a tail-agnostic policy.
Signed-off-by: eop Chen
Reviewed-by: Frank Chang
Reviewed-by: Weiwei Li
---
target/riscv/insn_trans/trans_rvv.c.inc | 6 +
target/riscv/vector_helper.c| 30 +
2 files changed, 36 insertions(+)
diff --git a/target/riscv/insn_trans/trans_rvv.c.inc
b/target/riscv/insn_trans/trans_rvv.c.inc
index a267945267..ab9f876c28 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -3209,6 +3209,8 @@ static bool trans_##NAME(DisasContext *s, arg_r *a)
\
tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over); \
\
data = FIELD_DP32(data, VDATA, LMUL, s->lmul); \
+data = \
+FIELD_DP32(data, VDATA, VTA_ALL_1S, s->cfg_vta_all_1s);\
tcg_gen_gvec_4_ptr(vreg_ofs(s, a->rd), vreg_ofs(s, 0), \
vreg_ofs(s, a->rs1),\
vreg_ofs(s, a->rs2), cpu_env, \
@@ -3313,6 +3315,8 @@ static bool trans_##NAME(DisasContext *s, arg_rmr *a)
\
\
data = FIELD_DP32(data, VDATA, VM, a->vm); \
data = FIELD_DP32(data, VDATA, LMUL, s->lmul); \
+data = \
+FIELD_DP32(data, VDATA, VTA_ALL_1S, s->cfg_vta_all_1s);\
tcg_gen_gvec_3_ptr(vreg_ofs(s, a->rd), \
vreg_ofs(s, 0), vreg_ofs(s, a->rs2),\
cpu_env, s->cfg_ptr->vlen / 8, \
@@ -3350,6 +3354,7 @@ static bool trans_viota_m(DisasContext *s, arg_viota_m *a)
data = FIELD_DP32(data, VDATA, VM, a->vm);
data = FIELD_DP32(data, VDATA, LMUL, s->lmul);
+data = FIELD_DP32(data, VDATA, VTA, s->vta);
static gen_helper_gvec_3_ptr * const fns[4] = {
gen_helper_viota_m_b, gen_helper_viota_m_h,
gen_helper_viota_m_w, gen_helper_viota_m_d,
@@ -3379,6 +3384,7 @@ static bool trans_vid_v(DisasContext *s, arg_vid_v *a)
data = FIELD_DP32(data, VDATA, VM, a->vm);
data = FIELD_DP32(data, VDATA, LMUL, s->lmul);
+data = FIELD_DP32(data, VDATA, VTA, s->vta);
static gen_helper_gvec_2_ptr * const fns[4] = {
gen_helper_vid_v_b, gen_helper_vid_v_h,
gen_helper_vid_v_w, gen_helper_vid_v_d,
diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
index a319cda969..dcb6d3538c 100644
--- a/target/riscv/vector_helper.c
+++ b/target/riscv/vector_helper.c
@@ -4719,6 +4719,8 @@ void HELPER(NAME)(void *vd, void *v0, void *vs1,
\
uint32_t desc) \
{ \
uint32_t vl = env->vl;\
+uint32_t total_elems = env_archcpu(env)->cfg.vlen;\
+uint32_t vta_all_1s = vext_vta_all_1s(desc); \
uint32_t i; \
int a, b; \
\
@@ -4728,6 +4730,15 @@ void HELPER(NAME)(void *vd, void *v0, void *vs1,
\
vext_set_elem_mask(vd, i, OP(b, a)); \
} \
env->vstart = 0; \
+/* mask destination register are always tail- \
+ * agnostic \
+ */ \
+/* set tail elements to 1s */ \
+if (vta_all_1s) { \
+for (; i < total_elems; i++) {\
+vext_set_elem_mask(vd, i, 1); \
+} \
+} \
}
#define DO_NAND(N, M) (!(N & M))
@@ -4795,6 +4806,8 @@ static void vmsetm(void *vd, void *v0, void *vs2,
CPURISCVState *env,
{
uint32_t vm = vext_vm(desc);
uint32_t vl = env->vl;
+uint32_t total_elems = env_archcpu(env)->cfg.vlen;
+uint32_t vta_all_1s = vext_vta_all_1s(desc);
int i;
bool first_mask_bit = false;
@@ -4823,6 +4836,13 @@ static void vmsetm(void *vd, void *v0, void *vs2,
CPURISCVState *env,
}
}
env->vstart = 0;
+/* mask destination register are always tail-agnostic */
+/* set tail elements to 1s */
+if (vta_all_1s) {
+for (; i < total_elems; i++) {
+vext_set_elem_mask(vd, i, 1);
+}
+}
}
void
[PATCH qemu v13 10/15] target/riscv: rvv: Add tail agnostic for vector fix-point arithmetic instructions
From: eopXD
Signed-off-by: eop Chen
Reviewed-by: Frank Chang
Reviewed-by: Weiwei Li
---
target/riscv/vector_helper.c | 220 ++-
1 file changed, 114 insertions(+), 106 deletions(-)
diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
index 87faf1770b..90af4a3bd2 100644
--- a/target/riscv/vector_helper.c
+++ b/target/riscv/vector_helper.c
@@ -2105,10 +2105,12 @@ static inline void
vext_vv_rm_2(void *vd, void *v0, void *vs1, void *vs2,
CPURISCVState *env,
uint32_t desc,
- opivv2_rm_fn *fn)
+ opivv2_rm_fn *fn, uint32_t esz)
{
uint32_t vm = vext_vm(desc);
uint32_t vl = env->vl;
+uint32_t total_elems = vext_get_total_elems(env, desc, esz);
+uint32_t vta = vext_vta(desc);
switch (env->vxrm) {
case 0: /* rnu */
@@ -2128,15 +2130,17 @@ vext_vv_rm_2(void *vd, void *v0, void *vs1, void *vs2,
env, vl, vm, 3, fn);
break;
}
+/* set tail elements to 1s */
+vext_set_elems_1s(vd, vta, vl * esz, total_elems * esz);
}
/* generate helpers for fixed point instructions with OPIVV format */
-#define GEN_VEXT_VV_RM(NAME)\
+#define GEN_VEXT_VV_RM(NAME, ESZ) \
void HELPER(NAME)(void *vd, void *v0, void *vs1, void *vs2, \
CPURISCVState *env, uint32_t desc)\
{ \
vext_vv_rm_2(vd, v0, vs1, vs2, env, desc, \
- do_##NAME);\
+ do_##NAME, ESZ); \
}
static inline uint8_t saddu8(CPURISCVState *env, int vxrm, uint8_t a, uint8_t
b)
@@ -2186,10 +2190,10 @@ RVVCALL(OPIVV2_RM, vsaddu_vv_b, OP_UUU_B, H1, H1, H1,
saddu8)
RVVCALL(OPIVV2_RM, vsaddu_vv_h, OP_UUU_H, H2, H2, H2, saddu16)
RVVCALL(OPIVV2_RM, vsaddu_vv_w, OP_UUU_W, H4, H4, H4, saddu32)
RVVCALL(OPIVV2_RM, vsaddu_vv_d, OP_UUU_D, H8, H8, H8, saddu64)
-GEN_VEXT_VV_RM(vsaddu_vv_b)
-GEN_VEXT_VV_RM(vsaddu_vv_h)
-GEN_VEXT_VV_RM(vsaddu_vv_w)
-GEN_VEXT_VV_RM(vsaddu_vv_d)
+GEN_VEXT_VV_RM(vsaddu_vv_b, 1)
+GEN_VEXT_VV_RM(vsaddu_vv_h, 2)
+GEN_VEXT_VV_RM(vsaddu_vv_w, 4)
+GEN_VEXT_VV_RM(vsaddu_vv_d, 8)
typedef void opivx2_rm_fn(void *vd, target_long s1, void *vs2, int i,
CPURISCVState *env, int vxrm);
@@ -,10 +2226,12 @@ static inline void
vext_vx_rm_2(void *vd, void *v0, target_long s1, void *vs2,
CPURISCVState *env,
uint32_t desc,
- opivx2_rm_fn *fn)
+ opivx2_rm_fn *fn, uint32_t esz)
{
uint32_t vm = vext_vm(desc);
uint32_t vl = env->vl;
+uint32_t total_elems = vext_get_total_elems(env, desc, esz);
+uint32_t vta = vext_vta(desc);
switch (env->vxrm) {
case 0: /* rnu */
@@ -2245,25 +2251,27 @@ vext_vx_rm_2(void *vd, void *v0, target_long s1, void
*vs2,
env, vl, vm, 3, fn);
break;
}
+/* set tail elements to 1s */
+vext_set_elems_1s(vd, vta, vl * esz, total_elems * esz);
}
/* generate helpers for fixed point instructions with OPIVX format */
-#define GEN_VEXT_VX_RM(NAME) \
+#define GEN_VEXT_VX_RM(NAME, ESZ) \
void HELPER(NAME)(void *vd, void *v0, target_ulong s1,\
void *vs2, CPURISCVState *env, uint32_t desc) \
{ \
vext_vx_rm_2(vd, v0, s1, vs2, env, desc, \
- do_##NAME); \
+ do_##NAME, ESZ); \
}
RVVCALL(OPIVX2_RM, vsaddu_vx_b, OP_UUU_B, H1, H1, saddu8)
RVVCALL(OPIVX2_RM, vsaddu_vx_h, OP_UUU_H, H2, H2, saddu16)
RVVCALL(OPIVX2_RM, vsaddu_vx_w, OP_UUU_W, H4, H4, saddu32)
RVVCALL(OPIVX2_RM, vsaddu_vx_d, OP_UUU_D, H8, H8, saddu64)
-GEN_VEXT_VX_RM(vsaddu_vx_b)
-GEN_VEXT_VX_RM(vsaddu_vx_h)
-GEN_VEXT_VX_RM(vsaddu_vx_w)
-GEN_VEXT_VX_RM(vsaddu_vx_d)
+GEN_VEXT_VX_RM(vsaddu_vx_b, 1)
+GEN_VEXT_VX_RM(vsaddu_vx_h, 2)
+GEN_VEXT_VX_RM(vsaddu_vx_w, 4)
+GEN_VEXT_VX_RM(vsaddu_vx_d, 8)
static inline int8_t sadd8(CPURISCVState *env, int vxrm, int8_t a, int8_t b)
{
@@ -2309,19 +2317,19 @@ RVVCALL(OPIVV2_RM, vsadd_vv_b, OP_SSS_B, H1, H1, H1,
sadd8)
RVVCALL(OPIVV2_RM, vsadd_vv_h, OP_SSS_H, H2, H2, H2, sadd16)
RVVCALL(OPIVV2_RM, vsadd_vv_w, OP_SSS_W, H4, H4, H4, sadd32)
RVVCALL(OPIVV2_RM, vsadd_vv_d, OP_SSS_D, H8, H8, H8, sadd64)
-GEN_VEXT_VV_RM(vsadd_vv_b)
-GEN_VEXT_VV_RM(vsadd_vv_h)
-GEN_VEXT_VV_RM(vsadd_vv_w)
-GEN_VEXT_VV_RM(vsadd_vv_d)
+GEN_VEXT_VV_RM(vsadd_vv_b, 1)
+GEN_VEXT_VV_RM(vsadd_vv_h, 2)
+GEN_VEXT_VV_RM(vsadd_vv_w, 4)
+GEN_VEXT_VV_RM(vsadd_vv_d, 8)
RVVCALL(OPIVX2_RM, vsadd_vx_b, OP_SSS_B, H1, H1, sadd8)
RVVCALL(OPIVX2_RM, vsadd_vx_h, OP_SSS_H, H2, H2, sadd16)
RVVCALL(OPIVX2_RM, vsadd_vx_w, OP_SSS_W, H4, H4, sadd32)
[PATCH qemu v13 02/15] target/riscv: rvv: Rename ambiguous esz
From: eopXD
No functional change intended in this commit.
Signed-off-by: eop Chen
Reviewed-by: Frank Chang
Reviewed-by: Weiwei Li
Reviewed-by: Alistair Francis
---
target/riscv/vector_helper.c | 76 ++--
1 file changed, 38 insertions(+), 38 deletions(-)
diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
index e94caf1a3c..d0452a7756 100644
--- a/target/riscv/vector_helper.c
+++ b/target/riscv/vector_helper.c
@@ -125,9 +125,9 @@ static inline int32_t vext_lmul(uint32_t desc)
/*
* Get the maximum number of elements can be operated.
*
- * esz: log2 of element size in bytes.
+ * log2_esz: log2 of element size in bytes.
*/
-static inline uint32_t vext_max_elems(uint32_t desc, uint32_t esz)
+static inline uint32_t vext_max_elems(uint32_t desc, uint32_t log2_esz)
{
/*
* As simd_desc support at most 2048 bytes, the max vlen is 1024 bits.
@@ -136,7 +136,7 @@ static inline uint32_t vext_max_elems(uint32_t desc,
uint32_t esz)
uint32_t vlenb = simd_maxsz(desc);
/* Return VLMAX */
-int scale = vext_lmul(desc) - esz;
+int scale = vext_lmul(desc) - log2_esz;
return scale < 0 ? vlenb >> -scale : vlenb << scale;
}
@@ -231,11 +231,11 @@ vext_ldst_stride(void *vd, void *v0, target_ulong base,
target_ulong stride, CPURISCVState *env,
uint32_t desc, uint32_t vm,
vext_ldst_elem_fn *ldst_elem,
- uint32_t esz, uintptr_t ra, MMUAccessType access_type)
+ uint32_t log2_esz, uintptr_t ra, MMUAccessType access_type)
{
uint32_t i, k;
uint32_t nf = vext_nf(desc);
-uint32_t max_elems = vext_max_elems(desc, esz);
+uint32_t max_elems = vext_max_elems(desc, log2_esz);
for (i = env->vstart; i < env->vl; i++, env->vstart++) {
if (!vm && !vext_elem_mask(v0, i)) {
@@ -244,7 +244,7 @@ vext_ldst_stride(void *vd, void *v0, target_ulong base,
k = 0;
while (k < nf) {
-target_ulong addr = base + stride * i + (k << esz);
+target_ulong addr = base + stride * i + (k << log2_esz);
ldst_elem(env, adjust_addr(env, addr), i + k * max_elems, vd, ra);
k++;
}
@@ -289,18 +289,18 @@ GEN_VEXT_ST_STRIDE(vsse64_v, int64_t, ste_d)
/* unmasked unit-stride load and store operation*/
static void
vext_ldst_us(void *vd, target_ulong base, CPURISCVState *env, uint32_t desc,
- vext_ldst_elem_fn *ldst_elem, uint32_t esz, uint32_t evl,
+ vext_ldst_elem_fn *ldst_elem, uint32_t log2_esz, uint32_t evl,
uintptr_t ra, MMUAccessType access_type)
{
uint32_t i, k;
uint32_t nf = vext_nf(desc);
-uint32_t max_elems = vext_max_elems(desc, esz);
+uint32_t max_elems = vext_max_elems(desc, log2_esz);
/* load bytes from guest memory */
for (i = env->vstart; i < evl; i++, env->vstart++) {
k = 0;
while (k < nf) {
-target_ulong addr = base + ((i * nf + k) << esz);
+target_ulong addr = base + ((i * nf + k) << log2_esz);
ldst_elem(env, adjust_addr(env, addr), i + k * max_elems, vd, ra);
k++;
}
@@ -399,12 +399,12 @@ vext_ldst_index(void *vd, void *v0, target_ulong base,
void *vs2, CPURISCVState *env, uint32_t desc,
vext_get_index_addr get_index_addr,
vext_ldst_elem_fn *ldst_elem,
-uint32_t esz, uintptr_t ra, MMUAccessType access_type)
+uint32_t log2_esz, uintptr_t ra, MMUAccessType access_type)
{
uint32_t i, k;
uint32_t nf = vext_nf(desc);
uint32_t vm = vext_vm(desc);
-uint32_t max_elems = vext_max_elems(desc, esz);
+uint32_t max_elems = vext_max_elems(desc, log2_esz);
/* load bytes from guest memory */
for (i = env->vstart; i < env->vl; i++, env->vstart++) {
@@ -414,7 +414,7 @@ vext_ldst_index(void *vd, void *v0, target_ulong base,
k = 0;
while (k < nf) {
-abi_ptr addr = get_index_addr(base, i, vs2) + (k << esz);
+abi_ptr addr = get_index_addr(base, i, vs2) + (k << log2_esz);
ldst_elem(env, adjust_addr(env, addr), i + k * max_elems, vd, ra);
k++;
}
@@ -480,13 +480,13 @@ static inline void
vext_ldff(void *vd, void *v0, target_ulong base,
CPURISCVState *env, uint32_t desc,
vext_ldst_elem_fn *ldst_elem,
- uint32_t esz, uintptr_t ra)
+ uint32_t log2_esz, uintptr_t ra)
{
void *host;
uint32_t i, k, vl = 0;
uint32_t nf = vext_nf(desc);
uint32_t vm = vext_vm(desc);
-uint32_t max_elems = vext_max_elems(desc, esz);
+uint32_t max_elems = vext_max_elems(desc, log2_esz);
target_ulong addr, offset, remain;
/* probe every access*/
@@ -494,12 +494,12 @@ vext_ldff(void *vd, void *v0, target_ulong base,
if (!vm && !vext_elem_mask(v0, i)) {
continue;
}
-
[PATCH qemu v13 05/15] target/riscv: rvv: Add tail agnostic for vector load / store instructions
From: eopXD
Destination register of unit-stride mask load and store instructions are
always written with a tail-agnostic policy.
A vector segment load / store instruction may contain fractional lmul
with nf * lmul > 1. The rest of the elements in the last register should
be treated as tail elements.
Signed-off-by: eop Chen
Reviewed-by: Frank Chang
Reviewed-by: Weiwei Li
---
target/riscv/insn_trans/trans_rvv.c.inc | 11 +
target/riscv/translate.c| 2 +
target/riscv/vector_helper.c| 60 +
3 files changed, 73 insertions(+)
diff --git a/target/riscv/insn_trans/trans_rvv.c.inc
b/target/riscv/insn_trans/trans_rvv.c.inc
index cc80bf00ff..99691f1b9f 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -711,6 +711,7 @@ static bool ld_us_op(DisasContext *s, arg_r2nfvm *a,
uint8_t eew)
data = FIELD_DP32(data, VDATA, VM, a->vm);
data = FIELD_DP32(data, VDATA, LMUL, emul);
data = FIELD_DP32(data, VDATA, NF, a->nf);
+data = FIELD_DP32(data, VDATA, VTA, s->vta);
return ldst_us_trans(a->rd, a->rs1, data, fn, s, false);
}
@@ -748,6 +749,7 @@ static bool st_us_op(DisasContext *s, arg_r2nfvm *a,
uint8_t eew)
data = FIELD_DP32(data, VDATA, VM, a->vm);
data = FIELD_DP32(data, VDATA, LMUL, emul);
data = FIELD_DP32(data, VDATA, NF, a->nf);
+data = FIELD_DP32(data, VDATA, VTA, s->vta);
return ldst_us_trans(a->rd, a->rs1, data, fn, s, true);
}
@@ -774,6 +776,8 @@ static bool ld_us_mask_op(DisasContext *s, arg_vlm_v *a,
uint8_t eew)
/* EMUL = 1, NFIELDS = 1 */
data = FIELD_DP32(data, VDATA, LMUL, 0);
data = FIELD_DP32(data, VDATA, NF, 1);
+/* Mask destination register are always tail-agnostic */
+data = FIELD_DP32(data, VDATA, VTA, s->cfg_vta_all_1s);
return ldst_us_trans(a->rd, a->rs1, data, fn, s, false);
}
@@ -791,6 +795,8 @@ static bool st_us_mask_op(DisasContext *s, arg_vsm_v *a,
uint8_t eew)
/* EMUL = 1, NFIELDS = 1 */
data = FIELD_DP32(data, VDATA, LMUL, 0);
data = FIELD_DP32(data, VDATA, NF, 1);
+/* Mask destination register are always tail-agnostic */
+data = FIELD_DP32(data, VDATA, VTA, s->cfg_vta_all_1s);
return ldst_us_trans(a->rd, a->rs1, data, fn, s, true);
}
@@ -862,6 +868,7 @@ static bool ld_stride_op(DisasContext *s, arg_rnfvm *a,
uint8_t eew)
data = FIELD_DP32(data, VDATA, VM, a->vm);
data = FIELD_DP32(data, VDATA, LMUL, emul);
data = FIELD_DP32(data, VDATA, NF, a->nf);
+data = FIELD_DP32(data, VDATA, VTA, s->vta);
return ldst_stride_trans(a->rd, a->rs1, a->rs2, data, fn, s, false);
}
@@ -891,6 +898,7 @@ static bool st_stride_op(DisasContext *s, arg_rnfvm *a,
uint8_t eew)
data = FIELD_DP32(data, VDATA, VM, a->vm);
data = FIELD_DP32(data, VDATA, LMUL, emul);
data = FIELD_DP32(data, VDATA, NF, a->nf);
+data = FIELD_DP32(data, VDATA, VTA, s->vta);
fn = fns[eew];
if (fn == NULL) {
return false;
@@ -991,6 +999,7 @@ static bool ld_index_op(DisasContext *s, arg_rnfvm *a,
uint8_t eew)
data = FIELD_DP32(data, VDATA, VM, a->vm);
data = FIELD_DP32(data, VDATA, LMUL, emul);
data = FIELD_DP32(data, VDATA, NF, a->nf);
+data = FIELD_DP32(data, VDATA, VTA, s->vta);
return ldst_index_trans(a->rd, a->rs1, a->rs2, data, fn, s, false);
}
@@ -1043,6 +1052,7 @@ static bool st_index_op(DisasContext *s, arg_rnfvm *a,
uint8_t eew)
data = FIELD_DP32(data, VDATA, VM, a->vm);
data = FIELD_DP32(data, VDATA, LMUL, emul);
data = FIELD_DP32(data, VDATA, NF, a->nf);
+data = FIELD_DP32(data, VDATA, VTA, s->vta);
return ldst_index_trans(a->rd, a->rs1, a->rs2, data, fn, s, true);
}
@@ -1108,6 +1118,7 @@ static bool ldff_op(DisasContext *s, arg_r2nfvm *a,
uint8_t eew)
data = FIELD_DP32(data, VDATA, VM, a->vm);
data = FIELD_DP32(data, VDATA, LMUL, emul);
data = FIELD_DP32(data, VDATA, NF, a->nf);
+data = FIELD_DP32(data, VDATA, VTA, s->vta);
return ldff_trans(a->rd, a->rs1, data, fn, s);
}
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 7775dade26..58cbb6ded3 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -95,6 +95,7 @@ typedef struct DisasContext {
int8_t lmul;
uint8_t sew;
uint8_t vta;
+bool cfg_vta_all_1s;
target_ulong vstart;
bool vl_eq_vlmax;
uint8_t ntemp;
@@ -1085,6 +1086,7 @@ static void riscv_tr_init_disas_context(DisasContextBase
*dcbase, CPUState *cs)
ctx->sew = FIELD_EX32(tb_flags, TB_FLAGS, SEW);
ctx->lmul = sextract32(FIELD_EX32(tb_flags, TB_FLAGS, LMUL), 0, 3);
ctx->vta = FIELD_EX32(tb_flags, TB_FLAGS, VTA) && cpu->cfg.rvv_ta_all_1s;
+ctx->cfg_vta_all_1s = cpu->cfg.rvv_ta_all_1s;
ctx->vstart = env->vstart;
ctx->vl_eq_vlmax = FIELD_EX32(tb_flags, TB_FLAGS, VL_EQ_VLMAX);
ctx->misa_mxl_max = env->misa_mxl_max;
diff --git
[PATCH qemu v13 07/15] target/riscv: rvv: Add tail agnostic for vector integer shift instructions
From: eopXD
Signed-off-by: eop Chen
Reviewed-by: Frank Chang
Reviewed-by: Weiwei Li
---
target/riscv/insn_trans/trans_rvv.c.inc | 11 +++
target/riscv/vector_helper.c| 11 +++
2 files changed, 22 insertions(+)
diff --git a/target/riscv/insn_trans/trans_rvv.c.inc
b/target/riscv/insn_trans/trans_rvv.c.inc
index d15858fc6f..430847b0f9 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -1858,6 +1858,16 @@ do_opivx_gvec_shift(DisasContext *s, arg_rmrr *a,
GVecGen2sFn32 *gvec_fn,
}
if (a->vm && s->vl_eq_vlmax) {
+if (s->vta && s->lmul < 0) {
+/*
+ * tail elements may pass vlmax when lmul < 0
+ * set tail elements to 1s
+ */
+uint32_t vlenb = s->cfg_ptr->vlen >> 3;
+tcg_gen_gvec_ori(s->sew, vreg_ofs(s, a->rd),
+ vreg_ofs(s, a->rd), -1,
+ vlenb, vlenb);
+}
TCGv_i32 src1 = tcg_temp_new_i32();
tcg_gen_trunc_tl_i32(src1, get_gpr(s, a->rs1, EXT_NONE));
@@ -1916,6 +1926,7 @@ static bool trans_##NAME(DisasContext *s, arg_rmrr *a)
\
\
data = FIELD_DP32(data, VDATA, VM, a->vm); \
data = FIELD_DP32(data, VDATA, LMUL, s->lmul); \
+data = FIELD_DP32(data, VDATA, VTA, s->vta); \
tcg_gen_gvec_4_ptr(vreg_ofs(s, a->rd), vreg_ofs(s, 0), \
vreg_ofs(s, a->rs1),\
vreg_ofs(s, a->rs2), cpu_env, \
diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
index e00f9353b9..c1ae2ea2f1 100644
--- a/target/riscv/vector_helper.c
+++ b/target/riscv/vector_helper.c
@@ -1273,6 +1273,9 @@ void HELPER(NAME)(void *vd, void *v0, void *vs1,
\
{ \
uint32_t vm = vext_vm(desc); \
uint32_t vl = env->vl;\
+uint32_t esz = sizeof(TS1); \
+uint32_t total_elems = vext_get_total_elems(env, desc, esz); \
+uint32_t vta = vext_vta(desc);\
uint32_t i; \
\
for (i = env->vstart; i < vl; i++) { \
@@ -1284,6 +1287,8 @@ void HELPER(NAME)(void *vd, void *v0, void *vs1,
\
*((TS1 *)vd + HS1(i)) = OP(s2, s1 & MASK);\
} \
env->vstart = 0; \
+/* set tail elements to 1s */ \
+vext_set_elems_1s(vd, vta, vl * esz, total_elems * esz); \
}
GEN_VEXT_SHIFT_VV(vsll_vv_b, uint8_t, uint8_t, H1, H1, DO_SLL, 0x7)
@@ -1308,6 +1313,10 @@ void HELPER(NAME)(void *vd, void *v0, target_ulong s1,
\
{ \
uint32_t vm = vext_vm(desc);\
uint32_t vl = env->vl; \
+uint32_t esz = sizeof(TD); \
+uint32_t total_elems = \
+vext_get_total_elems(env, desc, esz); \
+uint32_t vta = vext_vta(desc); \
uint32_t i; \
\
for (i = env->vstart; i < vl; i++) {\
@@ -1318,6 +1327,8 @@ void HELPER(NAME)(void *vd, void *v0, target_ulong s1,
\
*((TD *)vd + HD(i)) = OP(s2, s1 & MASK);\
} \
env->vstart = 0;\
+/* set tail elements to 1s */ \
+vext_set_elems_1s(vd, vta, vl * esz, total_elems * esz);\
}
GEN_VEXT_SHIFT_VX(vsll_vx_b, uint8_t, int8_t, H1, H1, DO_SLL, 0x7)
--
2.34.2
