Re: [PATCH] hw/loongarch: Add cfi01 pflash device

2022-11-17 Thread yangxiaojuan


在 2022/11/15 下午8:10, Philippe Mathieu-Daudé 写道:

On 15/11/22 12:56, Xiaojuan Yang wrote:

Add cfi01 pflash device for LoongArch virt machine


So the subject prefix should be "hw/loongarch/virt:".


Signed-off-by: Xiaojuan Yang 
---
  hw/loongarch/Kconfig    |   1 +
  hw/loongarch/acpi-build.c   |  39 +++
  hw/loongarch/virt.c | 130 +---
  include/hw/loongarch/virt.h |   7 ++
  4 files changed, 168 insertions(+), 9 deletions(-)



  static bool memhp_type_supported(DeviceState *dev)
diff --git a/include/hw/loongarch/virt.h b/include/hw/loongarch/virt.h
index 45c383f5a7..4ec4a7b4fe 100644
--- a/include/hw/loongarch/virt.h
+++ b/include/hw/loongarch/virt.h
@@ -12,6 +12,7 @@
  #include "hw/boards.h"
  #include "qemu/queue.h"
  #include "hw/intc/loongarch_ipi.h"
+#include "hw/block/flash.h"
    #define LOONGARCH_MAX_VCPUS 4
  @@ -20,6 +21,11 @@
  #define VIRT_FWCFG_BASE 0x1e02UL
  #define VIRT_BIOS_BASE  0x1c00UL
  #define VIRT_BIOS_SIZE  (4 * MiB)
+#define VIRT_FLASH_SECTOR_SIZE  (128 * KiB)
+#define VIRT_FLASH0_BASE    VIRT_BIOS_BASE
+#define VIRT_FLASH0_SIZE    (4 * MiB)
+#define VIRT_FLASH1_BASE    (VIRT_FLASH0_BASE + VIRT_FLASH0_SIZE)
+#define VIRT_FLASH1_SIZE    (4 * MiB)
    #define VIRT_LOWMEM_BASE    0
  #define VIRT_LOWMEM_SIZE    0x1000
@@ -48,6 +54,7 @@ struct LoongArchMachineState {
  int  fdt_size;
  DeviceState *platform_bus_dev;
  PCIBus   *pci_bus;
+    PFlashCFI01  *flash[2];
  };


Since you are starting a virtual machine from scratch, you should take
the opportunity to learn from other early mistakes. X86 ended that way
due to 1/ old firmwares back-compability and 2/ QEMU pflash block
protections not being implemented. IIUC if we were starting with a
UEFI firmware today, the layout design (still using QEMU) would be
to map the CODE area in a dumb ROM device, and the VARSTORE area
in a PFlash device. Since Virt machines don't need to use Capsule
update, having the CODE area in ROM drastically simplifies the design
and maintainance.

Thanks, we will  use only one pflash to save the VARS.bin, and use -bios 
to load the CODE.bin.


Thanks.
XiaoJuan


Re: [PATCH v8 2/2] hw/intc: Fix LoongArch extioi coreisr accessing

2022-10-21 Thread yangxiaojuan



在 2022/10/21 下午5:11, Philippe Mathieu-Daudé 写道:

On 21/10/22 03:53, Xiaojuan Yang wrote:

1. When cpu read or write extioi COREISR reg, it should access
the reg belonged to itself, so the cpu index of 's->coreisr'
is current cpu number. Using MemTxAttrs' requester_id to get
the cpu index.
2. it need not to mask 0x1f when calculate the coreisr array index.

Signed-off-by: Xiaojuan Yang 
---
  hw/intc/loongarch_extioi.c  | 10 ++
  target/loongarch/iocsr_helper.c | 19 +++
  2 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/hw/intc/loongarch_extioi.c b/hw/intc/loongarch_extioi.c
index 72f4b0cde5..4b8ec3f28a 100644
--- a/hw/intc/loongarch_extioi.c
+++ b/hw/intc/loongarch_extioi.c
@@ -93,8 +93,9 @@ static MemTxResult extioi_readw(void *opaque, 
hwaddr addr, uint64_t *data,

  *data = s->bounce[index];
  break;
  case EXTIOI_COREISR_START ... EXTIOI_COREISR_END - 1:
-    index = ((offset - EXTIOI_COREISR_START) & 0x1f) >> 2;
-    cpu = ((offset - EXTIOI_COREISR_START) >> 8) & 0x3;
+    index = (offset - EXTIOI_COREISR_START) >> 2;
+    /* using attrs to get current cpu index */
+    cpu = attrs.requester_id;
  *data = s->coreisr[cpu][index];
  break;
  case EXTIOI_COREMAP_START ... EXTIOI_COREMAP_END - 1:
@@ -185,8 +186,9 @@ static MemTxResult extioi_writew(void *opaque, 
hwaddr addr,

  s->bounce[index] = val;
  break;
  case EXTIOI_COREISR_START ... EXTIOI_COREISR_END - 1:
-    index = ((offset - EXTIOI_COREISR_START) & 0x1f) >> 2;
-    cpu = ((offset - EXTIOI_COREISR_START) >> 8) & 0x3;
+    index = (offset - EXTIOI_COREISR_START) >> 2;
+    /* using attrs to get current cpu index */
+    cpu = attrs.requester_id;
  old_data = s->coreisr[cpu][index];
  s->coreisr[cpu][index] = old_data & ~val;
  /* write 1 to clear interrrupt */
diff --git a/target/loongarch/iocsr_helper.c 
b/target/loongarch/iocsr_helper.c

index 0e9c537dc7..505853e17b 100644
--- a/target/loongarch/iocsr_helper.c
+++ b/target/loongarch/iocsr_helper.c
@@ -14,54 +14,57 @@
  #include "exec/cpu_ldst.h"
  #include "tcg/tcg-ldst.h"
  +#define GET_MEMTXATTRS(cas) \
+    ((MemTxAttrs){.requester_id = env_cpu(cas)->cpu_index})


The suggestion from v7 is incomplete, I apologize for missing it.

#define GET_MEMTXATTRS(cas) ((MemTxAttrs) {\
   .requester_type = MTRT_CPU,\
   .requester_id = env_cpu(cas)->cpu_index,\
    })

Also see from v6, add in the read/write handlers:

    assert(attrs.requester_type == MTRT_CPU);

https://lore.kernel.org/qemu-devel/f7c4f7ca-cbf9-87d6-4d8c-5957c36ae...@linaro.org/ 




hi,
we do not based on the 'MemTxAttrs requester_type patch' so far, and 
when that

patch merged we will apply it quickly.

Thanks.
Xiaojuan.




Re: [PATCH v2 3/3] hw/intc: Fix LoongArch ipi device emulation

2022-09-28 Thread yangxiaojuan


在 2022/9/29 上午10:42, Richard Henderson 写道:

On 9/26/22 23:12, Xiaojuan Yang wrote:

In ipi_send function, it should not to set irq before
writing data to dest cpu iocsr space, as the irq will
trigger after data writing.

Signed-off-by: Xiaojuan Yang 
---
  hw/intc/loongarch_ipi.c | 1 -
  1 file changed, 1 deletion(-)

diff --git a/hw/intc/loongarch_ipi.c b/hw/intc/loongarch_ipi.c
index 4f3c58f872..aa4bf9eb74 100644
--- a/hw/intc/loongarch_ipi.c
+++ b/hw/intc/loongarch_ipi.c
@@ -88,7 +88,6 @@ static void ipi_send(uint64_t val)
  cs = qemu_get_cpu(cpuid);
  cpu = LOONGARCH_CPU(cs);
  env = >env;
-    loongarch_cpu_set_irq(cpu, IRQ_IPI, 1);
  address_space_stl(>address_space_iocsr, 0x1008,
    data, MEMTXATTRS_UNSPECIFIED, NULL);


Did you mean to move the call below the set?
Otherwise, where does the irq get raised?

When call this function 'address_space_stl(>address_space_iocsr, 
0x1008, ... ...)',  it will trigger  loongarch_ipi_writel(), the addr 
arg is 0x1008 ('CORE_SET_OFFSET'), and qemu_irq_raise will be called in 
this case.


Thanks.
Xiaojuan Yang



Re: [PULL 38/43] hw/loongarch: Add LoongArch ls7a rtc device support

2022-06-28 Thread yangxiaojuan

Hi Peter

On 2022/6/28 下午7:05, Peter Maydell wrote:

On Tue, 7 Jun 2022 at 00:34, Richard Henderson
 wrote:

From: Xiaojuan Yang 

This patch add ls7a rtc device support.

Signed-off-by: Xiaojuan Yang 
Signed-off-by: Song Gao 
Reviewed-by: Richard Henderson 
Message-Id: <20220606124333.2060567-39-yangxiaoj...@loongson.cn>
Signed-off-by: Richard Henderson 

Hi; Coverity points out some issues with this code, and I
noticed a few more reading through it.
Thanks for your advice. I am reading your comments carefully now, and I 
will correct it immediately,


Thanks.
Xiaojuan

+static inline void toymatch_val_to_time(uint64_t val, struct tm *tm)
+{
+tm->tm_sec = FIELD_EX32(val, TOY_MATCH, SEC);
+tm->tm_min = FIELD_EX32(val, TOY_MATCH, MIN);
+tm->tm_hour = FIELD_EX32(val, TOY_MATCH, HOUR);
+tm->tm_mday = FIELD_EX32(val, TOY_MATCH, DAY);
+tm->tm_mon = FIELD_EX32(val, TOY_MATCH, MON) - 1;
+tm->tm_year += (FIELD_EX32(val, TOY_MATCH, YEAR) - (tm->tm_year & 0x3f));
+}
+
+static void toymatch_write(LS7ARtcState *s, struct tm *tm, uint64_t val, int 
num)
+{

Why does this function take a pointer to a struct tm? The callsites
all pass in an entirely uninitialized struct tm and don't try to
read from it after the call. It would be clearer to just define
the struct tm as a local in this function.


+int64_t now, expire_time;
+
+/* it do not support write when toy disabled */
+if (toy_enabled(s)) {
+s->toymatch[num] = val;
+/* caculate expire time */
+now = qemu_clock_get_ms(rtc_clock);
+toymatch_val_to_time(val, tm);
+expire_time = now + (qemu_timedate_diff(tm) - s->offset_toy) * 1000;

Coverity complains (CID 1489766) that we end up using uninitialized
fields in the struct tm here. There's two reasons for that:
(1) toymatch_val_to_time() doesn't set all the fields in the struct,
and we never zero-initialized the struct. This accounts for
tm_gmtoff, tm_isdst, tm_wday, tm_yday, tm_zone. You need to look
at whether any of those ought to be initialized, and set them.
Zero-init the struct to make Coverity happy about the rest of them.

(2) toymatch_val_to_time() sets tm_year based on the previous value
of tm_year. This doesn't make sense if the struct isn't initialized.
What was the intention here ?



+timer_mod(s->toy_timer[num], expire_time);
+}
+}
+static void ls7a_toy_start(LS7ARtcState *s)
+{
+int i;
+uint64_t expire_time, now;
+struct tm tm;

Coverity issue CID 1489763: we don't zero initialize the struct tm here,
but we don't individually initialize all its fields. In particular
the tm_wday field is never set up and Coverity complains it might
be used uninitialized.

The easy fix is to zero-init everything:
struct tm tm = {};


+/*
+ * need to recaculate toy offset

typo: "recalculate" (here and in other comments above and below)


+ * and expire time when enable it.
+ */
+toy_val_to_time_mon(s->save_toy_mon, );
+toy_val_to_time_year(s->save_toy_year, );
+
+s->offset_toy = qemu_timedate_diff();
+now = qemu_clock_get_ms(rtc_clock);
+
+/* recaculate expire time and enable timer */
+for (i = 0; i < TIMER_NUMS; i++) {
+toymatch_val_to_time(s->toymatch[i], );
+expire_time = now + (qemu_timedate_diff() - s->offset_toy) * 1000;
+timer_mod(s->toy_timer[i], expire_time);
+}
+}
+static void toy_timer_cb(void *opaque)
+{
+LS7ARtcState *s = opaque;
+
+if (toy_enabled(s)) {
+qemu_irq_pulse(s->irq);
+}
+}
+
+static void rtc_timer_cb(void *opaque)
+{
+LS7ARtcState *s = opaque;
+
+if (rtc_enabled(s)) {
+qemu_irq_pulse(s->irq);
+}

Does the real hardware really pulse the IRQ line?


+}
+
+static void ls7a_rtc_realize(DeviceState *dev, Error **errp)
+{
+int i;
+SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+LS7ARtcState *d = LS7A_RTC(sbd);
+memory_region_init_io(>iomem, NULL, _rtc_ops,
+ (void *)d, "ls7a_rtc", 0x100);
+
+sysbus_init_irq(sbd, >irq);
+
+sysbus_init_mmio(sbd, >iomem);
+for (i = 0; i < TIMER_NUMS; i++) {
+d->toymatch[i] = 0;
+d->rtcmatch[i] = 0;
+d->toy_timer[i] = timer_new_ms(rtc_clock, toy_timer_cb, d);
+d->rtc_timer[i] = timer_new_ms(rtc_clock, rtc_timer_cb, d);
+}
+d->offset_toy = 0;
+d->offset_rtc = 0;
+d->save_toy_mon = 0;
+d->save_toy_year = 0;
+d->save_rtc = 0;

This device is missing an implementation of the reset method,
and a lot of this looks like it is code that ought to be in reset.


+
+create_unimplemented_device("mmio fallback 1", 0x10013ffc, 0x4);

This call to create_unimplemented_device() is wrong -- device realize
code must not map anything into the system memory map. That is up to
board or SoC level code to do. I'm not sure what it's trying to do,
but it should be done some other way.


+}

thanks
-- PMM





Re: [PATCH v5 00/43] Add LoongArch softmmu support

2022-05-24 Thread yangxiaojuan


在 2022/5/25 6:41, Richard Henderson 写道:

On 5/24/22 15:32, Richard Henderson wrote:

When the syntax errors are fixed, it does not pass "make check".


When I configure with --enable-debug --enable-sanitizers I get


I got the same error.

The 'make check '  result:

Summary of Failures:

 95/117 qemu:qtest+qtest-loongarch64 / 
qtest-loongarch64/device-introspect-test ERROR   1.20s killed by 
signal 6 SIGABRT

Ok: 114
Expected Fail:  0
Fail:   1
Unexpected Pass:    0
Skipped:    2
Timeout:    0


We will fix this error as soon as possible.  And  what necessary tests 
do we need to do?
'mak check-tcg' ,  'make check' and 'make docker-test-build',  these are 
we know so far.


I also see the wiki  [1],   should  we need tests all of them? Could you 
give us some advice?

[1] : https://wiki.qemu.org/Testing#Tests_included_in_the_QEMU_source

Thanks.
Xiaojuan


$ QTEST_QEMU_BINARY='./qemu-system-loongarch64' 
./tests/qtest/device-introspect-test -v

...
# Testing device 'loongarch_ipi'

=

==911066==ERROR: AddressSanitizer: heap-buffer-overflow on address 
0x61393550 at pc 0x7f97cb425c23 bp 0x7ffe6583f4f0 sp 0x7ffe6583ec98


WRITE of size 8 at 0x61393550 thread T0

    #0 0x7f97cb425c22 in __interceptor_memset 
../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:799


    #1 0x562b21b23916 in qdev_init_gpio_out_named 
../qemu/hw/core/gpio.c:85


    #2 0x562b21b23b89 in qdev_init_gpio_out ../qemu/hw/core/gpio.c:101

    #3 0x562b22562d77 in loongarch_ipi_init 
../qemu/hw/intc/loongarch_ipi.c:187


    #4 0x562b22992ef0 in object_init_with_type ../qemu/qom/object.c:377

    #5 0x562b2299445f in object_initialize_with_type 
../qemu/qom/object.c:519


    #6 0x562b22995b54 in object_new_with_type ../qemu/qom/object.c:734

    #7 0x562b22995c6d in object_new ../qemu/qom/object.c:749

    #8 0x562b22ddc1d3 in qmp_device_list_properties 
../qemu/qom/qom-qmp-cmds.c:146


    #9 0x562b22f4ad2c in qmp_marshal_device_list_properties 
qapi/qapi-commands-qdev.c:66


    #10 0x562b22fa7ab6 in do_qmp_dispatch_bh 
../qemu/qapi/qmp-dispatch.c:128


    #11 0x562b230354b1 in aio_bh_call ../qemu/util/async.c:142

    #12 0x562b23035c09 in aio_bh_poll ../qemu/util/async.c:170

    #13 0x562b22fd6531 in aio_dispatch ../qemu/util/aio-posix.c:421

    #14 0x562b2303714c in aio_ctx_dispatch ../qemu/util/async.c:312

    #15 0x7f97caafdd1a in g_main_dispatch ../../../glib/gmain.c:3417

    #16 0x7f97caafdd1a in g_main_context_dispatch 
../../../glib/gmain.c:4135


    #17 0x562b23089479 in glib_pollfds_poll ../qemu/util/main-loop.c:297

    #18 0x562b23089663 in os_host_main_loop_wait 
../qemu/util/main-loop.c:320


    #19 0x562b23089968 in main_loop_wait ../qemu/util/main-loop.c:596

    #20 0x562b2223edf5 in qemu_main_loop ../qemu/softmmu/runstate.c:726

    #21 0x562b21965c69 in qemu_main ../qemu/softmmu/main.c:36

    #22 0x562b21965c9e in main ../qemu/softmmu/main.c:45

    #23 0x7f97c9354d8f in __libc_start_call_main 
../sysdeps/nptl/libc_start_call_main.h:58


    #24 0x7f97c9354e3f in __libc_start_main_impl ../csu/libc-start.c:392

    #25 0x562b21965b74 in _start 
(/home/rth/chroot-home/bld-x/qemu-system-loongarch64+0x21b0b74)




0x61393550 is located 48 bytes to the left of 376-byte region 
[0x61393580,0x613936f8)


allocated by thread T0 here:

    #0 0x7f97cb4a0a37 in __interceptor_calloc 
../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154


    #1 0x7f97cab06c40 in g_malloc0 ../../../glib/gmem.c:155

    #2 0x562b2298fef0 in type_register_internal ../qemu/qom/object.c:143

    #3 0x562b2298ffcd in type_register ../qemu/qom/object.c:152

    #4 0x562b2199c281 in qemu_console_early_init 
../qemu/ui/console.c:2719


    #5 0x562b2224d16e in qemu_create_early_backends 
../qemu/softmmu/vl.c:1975


    #6 0x562b222565ef in qemu_init ../qemu/softmmu/vl.c:3674

    #7 0x562b21965c64 in qemu_main ../qemu/softmmu/main.c:35

    #8 0x562b21965c9e in main ../qemu/softmmu/main.c:45

    #9 0x7f97c9354d8f in __libc_start_call_main 
../sysdeps/nptl/libc_start_call_main.h:58




SUMMARY: AddressSanitizer: heap-buffer-overflow 
../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:799 
in __interceptor_memset


Shadow bytes around the buggy address:

  0x0c268000a650: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00

  0x0c268000a660: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 fa

  0x0c268000a670: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00

  0x0c268000a680: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00

  0x0c268000a690: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00

=>0x0c268000a6a0: 00 00 00 00 fa fa fa fa fa fa[fa]fa fa fa fa fa

  0x0c268000a6b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00

  0x0c268000a6c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00

  0x0c268000a6d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 

Re: [PATCH v5 00/43] Add LoongArch softmmu support

2022-05-24 Thread yangxiaojuan

Hi, Richard

在 2022/5/25 6:32, Richard Henderson 写道:

On 5/24/22 01:17, Xiaojuan Yang wrote:

Hi All,

As this series only supports running binary files in ELF format, and
does not depend on BIOS and kernel file. so this series are changed 
from RFC to patch vX.



The manual:
   - 
https://github.com/loongson/LoongArch-Documentation/releases/tag/2022.03.17


Old series:
   - 
https://patchew.org/QEMU/20220328125749.2918087-1-yangxiaoj...@loongson.cn/
   - 
https://patchew.org/QEMU/20220106094200.1801206-1-gaos...@loongson.cn/


Need review patches:
   - 0034-hw-intc-Add-LoongArch-extioi-interrupt-controller-EI.patch
   - 0038-hw-loongarch-Add-LoongArch-ls7a-rtc-device-support.patch

This patch need ACPI maintainers review:
   - 0040-hw-loongarch-Add-LoongArch-ls7a-acpi-device-support.patch

Thanks.
Xiaojuan

-
v5:
   - Fixed loongarch extioi device emulation.
   - Fixed loongarch rtc device emulation.
   - Fixed 'make docker-test-build' error.


I had been tempted to accept the patch set as is, and let subsequent 
development happen on mainline, but this patch set does not compile, 
with obvious syntax errors.


When the syntax errors are fixed, it does not pass "make check".

How can you have tested this?

It `s my mistake.  I just tested   `IMAGES='fedora-i386-cross'  make 
docker-test-build `,  I will correct it in v6.


Thanks.
Xiaojuan




Re: [PATCH v4 38/43] hw/loongarch: Add LoongArch ls7a rtc device support

2022-05-20 Thread yangxiaojuan



On 2022/5/19 下午11:24, Richard Henderson wrote:

On 5/19/22 06:04, yangxiaojuan wrote:


On 2022/5/19 上午3:59, Richard Henderson wrote:

On 5/17/22 04:30, Xiaojuan Yang wrote:


+static void ls7a_stop_toymatch(LS7ARtcState *s)
+{
+    int i;
+    uint64_t now;
+
+    now = qemu_clock_get_ms(rtc_clock);
+    for (i = 0; i < TIMER_NUMS; i++) {
+    if (s->toy_timer[i].flag) {
+    s->toy_timer[i].enable_offset = 
s->toy_timer[i].timer->expire_time

+    - now;
+    timer_del(s->toy_timer[i].timer);


I don't think you need to check flag here, or update enable_offset.
Just an unconditional timer_del to stop the timer callback from firing.

Thanks very much, and i fixed it like this: Is this modification 
appropriate?

static void ls7a_rtc_stop(LS7ARtcState *s)
{
 int i;
 int64_t rtc_val, rtc_diff, now;
 now = ls7a_rtc_ticks();

 for (i = 0; i < TIMER_NUMS; i++) {
 if (s->rtc_timer[i].flag) {
 rtc_val = s->rtcmatch[i];
 rtc_diff = rtc_val - now - s->offset_rtc;
 s->rtc_timer[i].save_offset = rtc_diff;
 }
 timer_del(s->rtc_timer[i].timer);
}


I think you should drop "flag" entirely.  I don't see what it 
accomplishes.  It does not correspond to a bit in the state of the 
La7a rtc device.


Do you know if the documentation is incomplete, and rtcmatch[n] == 0 
is a magic number that indicates that the match value is disabled?  
Because if there is no magic number for disabling the match, I would 
expect *any* number, including 0, to match every 2**32 / 2**15 seconds 
~= 36 hours.


The  documentation is indeed not detailed, and we find a colleague in 
hadware  department  to confirm your problem.


The rtc_write and rtc_match both are absolute value, rtc_write will add 
counters if rtc_en enabled.
when rtc_write == rtc_match, rtc interrupt will happen. it also applies 
to this situation: rtc_write=0, rtc_match=0, and the rtc_en enabled, irq 
happens immediately. And the same for TOY.


It also occurs to me that EO is probably the thing that controls 
whether RTC "ticks", and RTCEN is probably the thing that controls 
whether RTC match interrupts are delivered, and the same for TOY.  Can 
you confirm this?



If RTC_EN disabled, it do not support to write rtc_write and 
rtc_match[n], and the rtc_write will not add counters, and the interrupt 
is also disabled, EO do not control this. So, we should check rtc_en 
when write rtc_write and rtc_match. And the same for TOY.


Thanks.
Xiaojuan




Re: [PATCH v4 38/43] hw/loongarch: Add LoongArch ls7a rtc device support

2022-05-19 Thread yangxiaojuan



On 2022/5/19 上午3:59, Richard Henderson wrote:

On 5/17/22 04:30, Xiaojuan Yang wrote:


+static void ls7a_stop_toymatch(LS7ARtcState *s)
+{
+    int i;
+    uint64_t now;
+
+    now = qemu_clock_get_ms(rtc_clock);
+    for (i = 0; i < TIMER_NUMS; i++) {
+    if (s->toy_timer[i].flag) {
+    s->toy_timer[i].enable_offset = 
s->toy_timer[i].timer->expire_time

+    - now;
+    timer_del(s->toy_timer[i].timer);


I don't think you need to check flag here, or update enable_offset.
Just an unconditional timer_del to stop the timer callback from firing.

Thanks very much, and i fixed it like this: Is this modification 
appropriate?

static void ls7a_rtc_stop(LS7ARtcState *s)
{
    int i;
    int64_t rtc_val, rtc_diff, now;
    now = ls7a_rtc_ticks();

    for (i = 0; i < TIMER_NUMS; i++) {
    if (s->rtc_timer[i].flag) {
    rtc_val = s->rtcmatch[i];
    rtc_diff = rtc_val - now - s->offset_rtc;
    s->rtc_timer[i].save_offset = rtc_diff;
    }
    timer_del(s->rtc_timer[i].timer);
}



+    case SYS_RTCWRTIE0:
+    s->offset_rtc = val - ls7a_rtc_ticks();


This needs to behave differently when !rtc_enabled, and when 
rtc_enabled reinit the timers for RTCMATCHn, because the time 
differential has changed.



Thanks, I fixed it as this:
    case SYS_RTCWRTIE0:
    if (rtc_enabled(s)) {
    s->offset_rtc = val - ls7a_rtc_ticks();
    } else {
    s->rtc_write_save = val;
 }
    break;

static void ls7a_rtc_start(LS7ARtcState *s)
{
    int i;
    int64_t expire_time, now;

    now = ls7a_rtc_ticks();

    if (rtc_save_val) {
    old_offset = s->offset_rtc;
    s->offset_rtc = rtc_save_val - now;
    offset_diff = s->offset_rtc - old_offset;
    }

    for (i = 0; i < TIMER_NUMS; i++) {
    if (s->rtc_timer[i].flag && rtc_enabled(s)) {
    expire_time = ticks + s->rtc_timer[i].save_offset + 
offset_diff;

    expire_time = (expire_time * 1000 / LS7A_RTC_FREQ);
    timer_mod(s->rtc_timer[i].timer, expire_time);
    }
    }
}

Thanks.
Xiaojuan




Re: [PATCH v4 34/43] hw/intc: Add LoongArch extioi interrupt controller(EIOINTC)

2022-05-18 Thread yangxiaojuan

Hi Richard

On 2022/5/19 上午2:04, Richard Henderson wrote:


+    uint64_t sw_isr[LOONGARCH_MAX_VCPUS][LS3A_INTC_IP][EXTIOI_IRQS 
/ 64];


This has not been declared with DECLARE_BITMAP, therefore you will see 
a compile-time error when building on an ILP32 (i686) or P64 (win64) 
host.


I pointed this out to you as recently as v2 of this series.
I am really disappointed to see this regress in just one month.

You can test this yourself with

  IMAGES='fedora-i386-cross fedora-win32-cross fedora-win64-cross' \
  make docker-test-build

Please do so before your next submission. 

Thank you for your patient guidance, we will carefully correct them.

Thanks.
Xiaojuan



Re: [PATCH v3 34/43] hw/intc: Add LoongArch extioi interrupt controller(EIOINTC)

2022-05-13 Thread yangxiaojuan



On 2022/5/12 上午9:58, maobibo wrote:


在 2022/5/11 22:14, Richard Henderson 写道:

On 5/11/22 02:54, yangxiaojuan wrote:

On 2022/5/10 上午1:56, Richard Henderson wrote:

+    case EXTIOI_IPMAP_START ... EXTIOI_IPMAP_END - 1:
+    index = (offset - EXTIOI_IPMAP_START) >> 2;
+    s->ipmap[index] = val;
+    break;

Do you need to recompute the entire interrupt map when ipmap changes?


Sorry, could you explain it in more detail? i can not understand the meanning 
of 'the entire interrupt map'?

I mean, ipmap[*] and coremap[*] controls how isr[*] maps to the various cpus, 
as coreisr[*].  If either ipmap or coremap changes, do you need to re-compute 
coreisr[*] from the input isr[*]?

I think the interrupt has been handled by the core before set coremap or ipmap, 
and coreisr[*] also has been set and cleard by original core.
So,  the new mapped core need not  to update the coreisr[*].


Why do you believe that the core to which the interrupt is directed has 
interrupts enabled?  Why do you believe the core to which the interrupt is 
directed is the one that is changing the interrupt mapping?
I think there is no interrupt enable registers of percpu in extioi 
device. So, i think we need not to consider the core to which the 
interrupt is directed if has interrupts enabled.
If the core to which the interrupt is directed is diffrent from the core 
that is changing the mapping, Should we copy the status value of 
coreisr[old_core][irq_num] to coreisr[new_core][irq_num]?
Ip mapping could not affect coreisr[cpu][irq_num], Should we still need 
to update the interrupt?


Thanks.
xiaojuan

By my understanding, irq bit of coreisr will be set even if the interrupt is 
disabled on the core, interrupt has been posted to core already, only that it 
is not serviced by the core. After irq affinity is changed, new interrupt may 
arrived to new core, one interrupt will be serviced by old core and new core at 
the same time. However it is the problem of guest kernel, guest kernel system 
should disable the irq and wait until irq finishes to be serviced on the old 
core when irq affinity is changing.


I think your assumption is not correct.


r~





Re: [PATCH v3 34/43] hw/intc: Add LoongArch extioi interrupt controller(EIOINTC)

2022-05-13 Thread yangxiaojuan


On 2022/5/10 上午1:56, Richard Henderson wrote:

On 5/9/22 04:38, yangxiaojuan wrote:
You are not considering CSR[0x420][49], which changes the format of 
this mapping.


Thanks very much, I will consider the mapping format by read 
iocsr[0x420][49] like this:

static uint64_t map_format(void)
{
 LoongArchCPU *cpu;
 CPULoongArchState *env;
 uint64_t val;

 cpu = LOONGARCH_CPU(current_cpu);
 env = &(cpu->env);

 val = address_space_ldq(>address_space_iocsr, 0x420,
  MEMTXATTRS_UNSPECIFIED, NULL);
 val &= 1 << 49;
 return val;
}


I'm not 100% sure how this "Other configuration control register" 
should be handled, but definitely not like this. 
Could we use the bitmapping as the default cpu or ip map format? Becaue 
we emulate iocsr[0x420] as a default value, and it does not support to 
write. We will add 'TOOD' logs and continue to modify them when using 
other routing methods later.

What do you think of this idea?

Thanks.
Xiaojuan


Re: [PATCH v3 34/43] hw/intc: Add LoongArch extioi interrupt controller(EIOINTC)

2022-05-11 Thread yangxiaojuan


On 2022/5/10 上午1:56, Richard Henderson wrote:



+    case EXTIOI_IPMAP_START ... EXTIOI_IPMAP_END - 1:
+    index = (offset - EXTIOI_IPMAP_START) >> 2;
+    s->ipmap[index] = val;
+    break;


Do you need to recompute the entire interrupt map when ipmap changes?

Sorry, could you explain it in more detail? i can not understand the 
meanning of 'the entire interrupt map'?


I mean, ipmap[*] and coremap[*] controls how isr[*] maps to the 
various cpus, as coreisr[*].  If either ipmap or coremap changes, do 
you need to re-compute coreisr[*] from the input isr[*]? 


I think the interrupt has been handled by the core before set coremap or 
ipmap, and coreisr[*] also has been set and cleard by original core.

So,  the new mapped core need not  to update the coreisr[*].

Thanks.
Xiaojuan



Re: [PATCH v3 02/43] target/loongarch: Add core definition

2022-05-10 Thread yangxiaojuan



On 2022/5/10 上午4:29, Richard Henderson wrote:

On 4/29/22 05:06, Xiaojuan Yang wrote:

+ DEFINE_LOONGARCH_CPU_TYPE("Loongson-3A5000", loongarch_3a5000_initfn),


Follow up on the comments against patch 34, and reading the 3A5000 
manual, I see


# On-chip integration of four 64-bit quad-launch superscalar GS464v 
processor cores.


Therefore it would seem that the cpu core here in target/loongarch/ 
should be named "gs464v", and the 3A5000 name should be reserved for 
the cpu package over in hw/loongarch/.


OK,  the lastest manual  [1]  the cpu core named  ' LA464',  I will 
correct it  on v4.
[1] : 
https://loongson.github.io/LoongArch-Documentation/Loongson-3A5000-usermanual-EN.pdf


Thanks.
Xiaojuan




Re: [PATCH v3 38/43] hw/loongarch: Add LoongArch ls7a rtc device support

2022-05-10 Thread yangxiaojuan



On 2022/5/8 上午5:55, Richard Henderson wrote:

On 4/29/22 05:07, Xiaojuan Yang wrote:

+/*
+ * Shift bits and filed mask
+ */
+#define TOY_MON_MASK   0x3f
+#define TOY_DAY_MASK   0x1f
+#define TOY_HOUR_MASK  0x1f
+#define TOY_MIN_MASK   0x3f
+#define TOY_SEC_MASK   0x3f
+#define TOY_MSEC_MASK  0xf
+
+#define TOY_MON_SHIFT  26
+#define TOY_DAY_SHIFT  21
+#define TOY_HOUR_SHIFT 16
+#define TOY_MIN_SHIFT  10
+#define TOY_SEC_SHIFT  4
+#define TOY_MSEC_SHIFT 0
+
+#define TOY_MATCH_YEAR_MASK  0x3f
+#define TOY_MATCH_MON_MASK   0xf
+#define TOY_MATCH_DAY_MASK   0x1f
+#define TOY_MATCH_HOUR_MASK  0x1f
+#define TOY_MATCH_MIN_MASK   0x3f
+#define TOY_MATCH_SEC_MASK   0x3f
+
+#define TOY_MATCH_YEAR_SHIFT 26
+#define TOY_MATCH_MON_SHIFT  22
+#define TOY_MATCH_DAY_SHIFT  17
+#define TOY_MATCH_HOUR_SHIFT 12
+#define TOY_MATCH_MIN_SHIFT  6
+#define TOY_MATCH_SEC_SHIFT  0


While this is ok, it would be better to use 
This becomes

FIELD(TOY, MON, 26, 6)
FIELD(TOY, DAY, 21, 5)
FIELD(TOY, HOUR, 16, 5)

You then extract with FIELD_EX32(val, TOY, MON), etc.

I will also mention that "millisec" is misnamed in the documentation.  
Since it represents units of 0.1 seconds, the prefix should be "deci".



Thanks very much, i changed it to this format:

+FIELD(TOY, MON, 26, 6)
+FIELD(TOY, DAY, 21, 5)
+FIELD(TOY, HOUR, 16, 5)
+FIELD(TOY, MIN, 10, 6)
+FIELD(TOY, SEC, 4, 6)
+FIELD(TOY, MSEC, 0, 4)
+
+FIELD(TOY_MATCH, YEAR, 26, 6)
+FIELD(TOY_MATCH, MON, 22, 4)
+FIELD(TOY_MATCH, DAY, 17, 5)
+FIELD(TOY_MATCH, HOUR, 12, 5)
+FIELD(TOY_MATCH, MIN, 6, 6)
+FIELD(TOY_MATCH, SEC, 0, 6)
+
+FIELD(RTC_CTRL, RTCEN, 13, 1)
+FIELD(RTC_CTRL, TOYEN, 11, 1)
+FIELD(RTC_CTRL, EO, 8, 1)

 get time from the value, like this:
...
case SYS_TOYWRITE0:
 qemu_get_timedate(, s->offset);
+    tm.tm_sec = FIELD_EX32(val, TOY, SEC);
+    tm.tm_min = FIELD_EX32(val, TOY, MIN);
+    tm.tm_hour = FIELD_EX32(val, TOY, HOUR);
+    tm.tm_mday = FIELD_EX32(val, TOY, DAY);
+    tm.tm_mon = FIELD_EX32(val, TOY, MON) - 1;
...


+#define TOY_ENABLE_BIT (1U << 11)

...

+enum {
+    TOYEN = 1UL << 11,
+    RTCEN = 1UL << 13,
+};


You have two copies of the same bit.  It would be best to fill in the 
rest of the bits in RTCCTRL, using FIELD().



+    case SYS_RTCREAD0:
+    val = s->rtccount;
+    break;


Surely this needs to examine qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) and 
scale the offset back to 32kHz.



+    case SYS_TOYMATCH0:
+    s->toymatch[0] = val;
+    qemu_get_timedate(, s->offset);
+    tm.tm_sec = (val >> TOY_MATCH_SEC_SHIFT) & TOY_MATCH_SEC_MASK;
+    tm.tm_min = (val >> TOY_MATCH_MIN_SHIFT) & TOY_MATCH_MIN_MASK;
+    tm.tm_hour = ((val >> TOY_MATCH_HOUR_SHIFT) & 
TOY_MATCH_HOUR_MASK);
+    tm.tm_mday = ((val >> TOY_MATCH_DAY_SHIFT) & 
TOY_MATCH_DAY_MASK);
+    tm.tm_mon = ((val >> TOY_MATCH_MON_SHIFT) & 
TOY_MATCH_MON_MASK) - 1;
+    year_diff = ((val >> TOY_MATCH_YEAR_SHIFT) & 
TOY_MATCH_YEAR_MASK);

+    year_diff = year_diff - (tm.tm_year & TOY_MATCH_YEAR_MASK);
+    tm.tm_year = tm.tm_year + year_diff;
+    alarm_offset = qemu_timedate_diff() - s->offset;
+    if ((alarm_offset < 0) && (alarm_offset > -5)) {
+    alarm_offset = 0;
+    }
+    expire_time = qemu_clock_get_ms(rtc_clock);
+    expire_time += ((alarm_offset * 1000) + 100);
+    timer_mod(s->timer, expire_time);
+    break;
+    case SYS_TOYMATCH1:
+    s->toymatch[1] = val;
+    break;
+    case SYS_TOYMATCH2:
+    s->toymatch[2] = val;
+    break;


Why does only register 0 affect expire time, and not all 3 registers?

Thanks, the toymatch[1]/[2] should also affect expire time. I fixed it 
like this:


+static void rtc_toymatch_write(LS7ARtcState *s, struct tm *tm, uint64_t 
val)

+{
+    int64_t alarm_offset, year_diff, expire_time;
+
+    qemu_get_timedate(tm, s->offset);
+    tm->tm_sec = FIELD_EX32(val, TOY_MATCH, SEC);
+    tm->tm_min = FIELD_EX32(val, TOY_MATCH, MIN);
+    tm->tm_hour = FIELD_EX32(val, TOY_MATCH, HOUR);
+    tm->tm_mday = FIELD_EX32(val, TOY_MATCH, DAY);
+    tm->tm_mon = FIELD_EX32(val, TOY_MATCH, MON) - 1;
+    year_diff = FIELD_EX32(val, TOY_MATCH, MON);
+    year_diff = year_diff - (tm->tm_year & TOY_MATCH_YEAR_MASK);
+    tm->tm_year = tm->tm_year + year_diff;
+    alarm_offset = qemu_timedate_diff(tm) - s->offset;
+    if ((alarm_offset < 0) && (alarm_offset > -5)) {
+    alarm_offset = 0;
+    }
+    expire_time = qemu_clock_get_ms(rtc_clock);
+    expire_time += ((alarm_offset * 1000) + 100);
+    timer_mod(s->timer, expire_time);
+}

...
case SYS_TOYMATCH0:
 s->toymatch[0] = val;
+ rtc_toymatch_write(s, , val);
 break;
case SYS_TOYMATCH1:
    s->toymatch[1] = val;
+    rtc_toymatch_write(s, , val);
    break;
case SYS_TOYMATCH2:
 s->toymatch[2] = val;
+ rtc_toymatch_write(s, , val);
 break;
...

+    case SYS_RTCCTRL:
+    s->cntrctl = val;
+    break;


Need to check REN, TEN, and EO 

Re: [PATCH v3 34/43] hw/intc: Add LoongArch extioi interrupt controller(EIOINTC)

2022-05-09 Thread yangxiaojuan


On 2022/5/7 下午11:31, Richard Henderson wrote:

+    if (level) {
+    /* if not enable return false */
+    if (((s->enable[enable_index]) & (1 << enable_mask)) == 0) {
+    return;
+    }
+    s->coreisr[cpu][coreisr_index] |= (1 << coreisr_mask);
+    qemu_set_irq(s->parent_irq[cpu][ipnum], level);
+    } else {
+    s->coreisr[cpu][coreisr_index] &= ~(1 << coreisr_mask);
+    qemu_set_irq(s->parent_irq[cpu][ipnum], level);
+    }


This final bit, updating the cpu irq is also wrong, in that it should 
be unconditional. This is the only way that it will work for the usage 
in updating the enable mask.


I think you are not considering when the MAP registers overlap 
outputs.  For instance, if all 256 bits of EXT_IOIMap contain 0, then 
all of EXT_IOI[n*32+31 : n*32] overlap.  When that happens, you cannot 
lower the level of the cpu pin until all of the matching ioi 
interrupts are low.



Thanks, i should consider the MAP registers overlap outputs.
And i want to add 'uint32_t sw_isr_group[256 / 32]', when each bit of 
sw_isr_group[n*32+31 : n*32] is 0, then lower the level of the cpu pin.


Thanks.
Xiaojuan


Re: [PATCH v3 34/43] hw/intc: Add LoongArch extioi interrupt controller(EIOINTC)

2022-05-09 Thread yangxiaojuan

Hi Richard,

On 2022/5/7 下午11:31, Richard Henderson wrote:

On 4/29/22 05:07, Xiaojuan Yang wrote:

+    int ipmap_mask = 0xff << ipmap_offset;

...

+    int cpu_mask = 0xff << ipmap_offset;


These two masks are redundant with

+    ipnum = ((s->ipmap[ipmap_index] & ipmap_mask) >> ipmap_offset) & 
0xf;

...

+    cpu = ((s->coremap[cpu_index] & cpu_mask) >> cpu_offset) & 0xf;


the 0xf masking here.


+    cpu = ctz32(cpu);
+    cpu = (cpu >= 4) ? 0 : cpu;


You are not considering CSR[0x420][49], which changes the format of 
this mapping.


Thanks very much, I will consider the mapping format by read 
iocsr[0x420][49] like this:

static uint64_t map_format(void)
{
    LoongArchCPU *cpu;
    CPULoongArchState *env;
    uint64_t val;

    cpu = LOONGARCH_CPU(current_cpu);
    env = &(cpu->env);

    val = address_space_ldq(>address_space_iocsr, 0x420,
 MEMTXATTRS_UNSPECIFIED, NULL);
    val &= 1 << 49;
    return val;
}
...
if (!map_format()) {
    cpu = ctz32(cpu);
    cpu = (cpu >= 4) ? 0 : cpu;
}
...
I think this function is wrong because you maintain an unmapped enable 
bitmap, but you do not maintain an unmapped status bitmap, which 
*should* be readable from EXTIOI_ISR_{START,END}, but is not present 
in extioi_readw.


I think that only extioi_setirq should actually change the unmapped 
status bitmap, and that extioi_update_irq should only evaluate the 
mapping to apply changes to the cpus.


Ok, there should be ISR registers(the status bitmap), i will add it to 
the LoongArchExtIOI, like this:

struct LoongArchExtIOI {
...
+    uint32_t isr[EXTIOI_IRQS / 32]
...
}

when extioi_setirq, update the isr filed.
static void extioi_setirq(void *opaque, int irq, int level)
{
    LoongArchExtIOI *s = LOONGARCH_EXTIOI(opaque);
    trace_loongarch_extioi_setirq(irq, level);
    s->isr[irq / 32] |= 1 << irq % 32;
    extioi_update_irq(s, irq, level);
}

and add ISR_START ... ISR_END to extioi_readw, like this
...
    case EXTIOI_ISR_START ... EXTIOI_ISR_END - 1:
    index = ((offset - EXTIOI_ISR_START) >> 2;
    ret = s->isr[index];
    break;
...



This final bit, updating the cpu irq is also wrong, in that it should 
be unconditional. This is the only way that it will work for the usage 
in updating the enable mask.


I think you are not considering when the MAP registers overlap 
outputs.  For instance, if all 256 bits of EXT_IOIMap contain 0, then 
all of EXT_IOI[n*32+31 : n*32] overlap.  When that happens, you cannot 
lower the level of the cpu pin until all of the matching ioi 
interrupts are low.


Or, perhaps I don't understand how this is supposed to work?
The documentation is very weak.



+static void extioi_writew(void *opaque, hwaddr addr,
+   uint64_t val, unsigned size)
+{
+    LoongArchExtIOI *s = LOONGARCH_EXTIOI(opaque);
+    int cpu, index, old_data, data_offset;
+    uint32_t offset;
+    trace_loongarch_extioi_writew(size, (uint32_t)addr, val);
+
+    offset = addr & 0x;
+
+    switch (offset) {
+    case EXTIOI_NODETYPE_START ... EXTIOI_NODETYPE_END - 1:
+    index = (offset - EXTIOI_NODETYPE_START) >> 2;
+    s->nodetype[index] = val;
+    break;
+    case EXTIOI_IPMAP_START ... EXTIOI_IPMAP_END - 1:
+    index = (offset - EXTIOI_IPMAP_START) >> 2;
+    s->ipmap[index] = val;
+    break;


Do you need to recompute the entire interrupt map when ipmap changes?

Sorry, could you explain it in more detail? i can not understand the 
meanning of 'the entire interrupt map'?
we only have ipmap and coremap registers in the LoongArchExtIOI, should 
we add an entire interrupt map?

+    case EXTIOI_ENABLE_START ... EXTIOI_ENABLE_END - 1:
+    index = (offset - EXTIOI_ENABLE_START) >> 2;
+    old_data = s->enable[index];
+    if (old_data != (int)val) {
+    s->enable[index] = val;
+    old_data = old_data ^ val;
+    data_offset = ctz32(old_data);
+    while (data_offset != 32) {
+    if (!(val & (1 << data_offset))) {
+    extioi_update_irq(s, data_offset + index * 32, 0);


This is not correct -- you're unconditionally setting level=0, 
corrupting the old value of coreisr[cpu][index].  You need to 
recompute *without* changning those levels.


Thanks, i will add a condition to judge coreisr[cpu][index], excute 
extioi_update_irq when the coreisr val is 1, like this:


static int get_coremap(int irq_num)
{
    int cpu;
    int cpu_index = irq_num / 4;
    int cpu_offset = irq_num & 0x3;
    int cpu_mask = 0xf << cpu_offset;

    cpu = (s->coremap[cpu_index] & cpu_mask) >> cpu_offset;
    if (!map_format()) {
    cpu = ctz32(cpu);
    cpu = (cpu >= 4) ? 0 : cpu;
    }
    return cpu;
}

static int coreisr_level(LoongArchExtIOI *s, int irq_num)
{
    int cpu = get_coremap(irq_num);
    return s->coreisr[cpu][irq_num / 32];
}

...
 while (data_offset != 32) {
 if (!(val & (1 << data_offset))) 

Re: [PATCH v3 00/43] Add LoongArch softmmu support

2022-05-05 Thread yangxiaojuan

Hi  Richard and Mark,
Could you help us review the patch [1] ?

[1] :
[PATCH v3 34/43] hw/intc: Add LoongArch extioi interrupt
[PATCH v3 38/43] hw/loongarch: Add LoongArch ls7a rtc device support
[PATCH v3 39/43] hw/loongarch: Add LoongArch load elf function.
[PATCH v3 40/43] hw/loongarch: Add LoongArch ls7a acpi device support

Thanks.
Xiaojuan
On 2022/4/29 下午6:06, Xiaojuan Yang wrote:

Hi All,

As this series only supports running binary files in ELF format, and does not 
depend on
BIOS and kernel file. so this series are changed from RFC to patch v1.


The manual:
   - https://github.com/loongson/LoongArch-Documentation/releases/tag/2022.03.17

Old series:
   - https://patchew.org/QEMU/20220328125749.2918087-1-yangxiaoj...@loongson.cn/
   - https://patchew.org/QEMU/20220106094200.1801206-1-gaos...@loongson.cn/

Thanks.
Xiaojuan

-
v3:

   - Add Check csr_names.
   - Drop CSR_CPUID, use cpu->cpu_index.
   - Fixed loongarch extioi device emulation. ipmap and coremap register change 
to 32bits.
   - Check_iocsr() function moved to loongarch_ipi_writel().
   - Pch_pic/msi use qdev_init_gpio_out() to init irq, and use 
qdev_connect_gpio_out() to connect irq.
   - Load elf function moved to hw/loongarch/loongson.c

v2:
   - Improvents to CSR/IOCSR instructions translation.
   - Fixed extioi device emulation. It is represented by only one memory region.
   - Fixed IPI device emulation. The registers are represented with uint64_t.
   - Use do_cpu_reset() and cpu_set_pc() to specify the load address.

v2: https://patchew.org/QEMU/20220425091027.2877892-1-yangxiaoj...@loongson.cn/
v1: https://patchew.org/QEMU/20220415094058.3584233-1-yangxiaoj...@loongson.cn/


Song Gao (18):
   target/loongarch: Add README
   target/loongarch: Add core definition
   target/loongarch: Add main translation routines
   target/loongarch: Add fixed point arithmetic instruction translation
   target/loongarch: Add fixed point shift instruction translation
   target/loongarch: Add fixed point bit instruction translation
   target/loongarch: Add fixed point load/store instruction translation
   target/loongarch: Add fixed point atomic instruction translation
   target/loongarch: Add fixed point extra instruction translation
   target/loongarch: Add floating point arithmetic instruction
 translation
   target/loongarch: Add floating point comparison instruction
 translation
   target/loongarch: Add floating point conversion instruction
 translation
   target/loongarch: Add floating point move instruction translation
   target/loongarch: Add floating point load/store instruction
 translation
   target/loongarch: Add branch instruction translation
   target/loongarch: Add disassembler
   target/loongarch: Add target build suport
   target/loongarch: 'make check-tcg' support

Xiaojuan Yang (25):
   target/loongarch: Add system emulation introduction
   target/loongarch: Add CSRs definition
   target/loongarch: Add basic vmstate description of CPU.
   target/loongarch: Implement qmp_query_cpu_definitions()
   target/loongarch: Add MMU support for LoongArch CPU.
   target/loongarch: Add LoongArch interrupt and exception handle
   target/loongarch: Add constant timer support
   target/loongarch: Add LoongArch CSR instruction
   target/loongarch: Add LoongArch IOCSR instruction
   target/loongarch: Add TLB instruction support
   target/loongarch: Add other core instructions support
   target/loongarch: Add timer related instructions support.
   hw/loongarch: Add support loongson3 virt machine type.
   hw/loongarch: Add LoongArch ipi interrupt support(IPI)
   hw/intc: Add LoongArch ls7a interrupt controller support(PCH-PIC)
   hw/intc: Add LoongArch ls7a msi interrupt controller support(PCH-MSI)
   hw/intc: Add LoongArch extioi interrupt controller(EIOINTC)
   hw/loongarch: Add irq hierarchy for the system
   Enable common virtio pci support for LoongArch
   hw/loongarch: Add some devices support for 3A5000.
   hw/loongarch: Add LoongArch ls7a rtc device support
   hw/loongarch: Add LoongArch load elf function.
   hw/loongarch: Add LoongArch ls7a acpi device support
   target/loongarch: Add gdb support.
   tests/tcg/loongarch64: Add hello/memory test in loongarch64 system

  MAINTAINERS   |  26 +
  .../devices/loongarch64-softmmu/default.mak   |   3 +
  configs/targets/loongarch64-softmmu.mak   |   4 +
  docs/system/loongarch/loongson3.rst   |  41 +
  gdb-xml/loongarch-base64.xml  |  44 +
  gdb-xml/loongarch-fpu64.xml   |  57 ++
  hw/Kconfig|   1 +
  hw/acpi/Kconfig   |   4 +
  hw/acpi/ls7a.c| 374 
  hw/acpi/meson.build   |   1 +
  hw/intc/Kconfig   |  15 +
  hw/intc/loongarch_extioi.c| 248 +
  hw/intc/loongarch_ipi.c   | 242 +
  

Re: [PATCH v3 25/43] target/loongarch: Add LoongArch CSR instruction

2022-05-05 Thread yangxiaojuan

Hi Richard,

On 2022/5/1 上午1:22, Richard Henderson wrote:

On 4/29/22 03:07, Xiaojuan Yang wrote:

+    [LOONGARCH_CSR_CPUID] = {
+    .offset = -1,
+    .flags = CSRFL_READONLY,
+    .readfn = gen_helper_csrrd_cpuid,
+    .writefn = NULL
+    },


The offset should be

    (int)offsetof(CPUState, cpu_index) - (int)offsetof(LoongArchCPU, env)

at which point you don't need the readfn.


+target_ulong helper_csrrd_tval(CPULoongArchState *env)
+{
+    LoongArchCPU *cpu = LOONGARCH_CPU(env_cpu(env));


cpu = env_archcpu(env).

Several other instances in the file.

Otherwise it looks good.


r~ \

Sorry for the late reply,    I will correct it on v4,

Thanks.
Xiaojuan




Re: [PATCH v2 34/43] hw/intc: Add LoongArch extioi interrupt controller(EIOINTC)

2022-04-27 Thread yangxiaojuan

Hi Mark,

On 2022/4/26 上午12:27, Mark Cave-Ayland wrote:

On 25/04/2022 10:10, Xiaojuan Yang wrote:


This patch realize the EIOINTC interrupt controller.

Signed-off-by: Xiaojuan Yang 
Signed-off-by: Song Gao 
---
  hw/intc/Kconfig    |   3 +
  hw/intc/loongarch_extioi.c | 483 +
  hw/intc/meson.build    |   1 +
  hw/intc/trace-events   |   9 +
  hw/loongarch/Kconfig   |   1 +
  include/hw/intc/loongarch_extioi.h |  60 
  6 files changed, 557 insertions(+)
  create mode 100644 hw/intc/loongarch_extioi.c
  create mode 100644 include/hw/intc/loongarch_extioi.h

diff --git a/hw/intc/Kconfig b/hw/intc/Kconfig
index 71c04c328e..28bd1f185d 100644
--- a/hw/intc/Kconfig
+++ b/hw/intc/Kconfig
@@ -96,3 +96,6 @@ config LOONGARCH_PCH_MSI
  select MSI_NONBROKEN
  bool
  select UNIMP
+
+config LOONGARCH_EXTIOI
+    bool
diff --git a/hw/intc/loongarch_extioi.c b/hw/intc/loongarch_extioi.c
new file mode 100644
index 00..1d9317c5bd
--- /dev/null
+++ b/hw/intc/loongarch_extioi.c
@@ -0,0 +1,483 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Loongson 3A5000 ext interrupt controller emulation
+ *
+ * Copyright (C) 2021 Loongson Technology Corporation Limited
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/module.h"
+#include "qemu/log.h"
+#include "hw/irq.h"
+#include "hw/sysbus.h"
+#include "hw/loongarch/virt.h"
+#include "hw/qdev-properties.h"
+#include "exec/address-spaces.h"
+#include "hw/intc/loongarch_extioi.h"
+#include "migration/vmstate.h"
+#include "trace.h"
+
+static void extioi_update_irq(LoongArchExtIOI *s, int irq_num, int 
level)

+{
+    uint64_t ipnum, cpu;
+
+    /*
+ * Routing in group of 32 interrupts.
+ * The default value of csr[0x420][49]
+ * is 0 and nobody will change it,
+ * so 'ipmap' use bitmap function.
+ */
+    ipnum = ldub_p((void *)>ipmap + (irq_num / 32)) & 0xf;
+    ipnum = find_first_bit(, 4);
+    ipnum = (ipnum == 4) ? 0 : ipnum;
+
+    cpu = ldub_p((void *)s->coremap + irq_num) & 0xf;
+    cpu = find_first_bit(, 4);
+    cpu = (cpu == 4) ? 0 : cpu;
+
+    if (level) {
+    if (test_bit(irq_num, (unsigned long *)s->enable) == false) {
+    return;
+    }
+    bitmap_set((unsigned long *)s->coreisr[cpu], irq_num, 1);
+    qemu_set_irq(s->parent_irq[cpu][ipnum], level);
+    } else {
+    bitmap_clear((unsigned long *)s->coreisr[cpu], irq_num, 1);
+    qemu_set_irq(s->parent_irq[cpu][ipnum], level);
+    }
+}
+
+static void extioi_setirq(void *opaque, int irq, int level)
+{
+    LoongArchExtIOI *s = LOONGARCH_EXTIOI(opaque);
+    trace_loongarch_extioi_setirq(irq, level);
+    extioi_update_irq(s, irq, level);
+}
+
+static uint32_t extioi_readb(void *opaque, hwaddr addr)
+{
+    LoongArchExtIOI *s = LOONGARCH_EXTIOI(opaque);
+    unsigned long offset, reg_count;
+    uint8_t ret = 0;
+    int cpu;
+
+    offset = addr & 0x;
+    switch (offset) {
+    case EXTIOI_ENABLE_START ... EXTIOI_ENABLE_END - 1:
+    ret = ldub_p((void *)s->enable + (offset - 
EXTIOI_ENABLE_START));

+    break;
+    case EXTIOI_BOUNCE_START ... EXTIOI_BOUNCE_END - 1:
+    ret = ldub_p((void *)s->bounce + (offset - 
EXTIOI_BOUNCE_START));

+    break;
+    case EXTIOI_COREISR_START ... EXTIOI_COREISR_END - 1:
+    reg_count = ((offset - EXTIOI_COREISR_START) & 0x1f);
+    cpu = ((offset - EXTIOI_COREISR_START) >> 8) & 0x3;
+    ret = ldub_p((void *)s->coreisr[cpu] + reg_count);
+    break;
+    case EXTIOI_IPMAP_START ... EXTIOI_IPMAP_END - 1:
+    ret = ldub_p((void *)>ipmap + (offset - 
EXTIOI_IPMAP_START));

+    break;
+    case EXTIOI_COREMAP_START ... EXTIOI_COREMAP_END - 1:
+    ret = ldub_p((void *)s->coremap + (offset - 
EXTIOI_COREMAP_START));

+    break;
+    case EXTIOI_NODETYPE_START ... EXTIOI_NODETYPE_END - 1:
+    ret = ldub_p((void *)s->nodetype + (offset - 
EXTIOI_NODETYPE_START));

+    break;
+    default:
+    break;
+    }
+    trace_loongarch_extioi_readb((uint32_t)addr, ret);
+    return ret;
+}
+
+static uint32_t extioi_readw(void *opaque, hwaddr addr)
+{
+    LoongArchExtIOI *s = LOONGARCH_EXTIOI(opaque);
+    unsigned long offset, reg_count;
+    uint32_t ret = 0;
+    int cpu;
+
+    offset = addr & 0x;
+    switch (offset) {
+    case EXTIOI_ENABLE_START ... EXTIOI_ENABLE_END - 1:
+    ret = ldl_p((void *)s->enable + (offset - 
EXTIOI_ENABLE_START));

+    break;
+    case EXTIOI_BOUNCE_START ... EXTIOI_BOUNCE_END - 1:
+    ret = ldl_p((void *)s->bounce + (offset - 
EXTIOI_BOUNCE_START));

+    break;
+    case EXTIOI_COREISR_START ... EXTIOI_COREISR_END - 1:
+    reg_count = ((offset - EXTIOI_COREISR_START) & 0x1f);
+    cpu = ((offset - EXTIOI_COREISR_START) >> 8) & 0x3;
+    ret = ldl_p((void *)s->coreisr[cpu] + reg_count);
+    break;
+    case EXTIOI_IPMAP_START ... EXTIOI_IPMAP_END - 1:
+    ret = 

Re: [PATCH v2 25/43] target/loongarch: Add LoongArch CSR instruction

2022-04-26 Thread yangxiaojuan

Hi, Richard

On 2022/4/26 上午6:55, Richard Henderson wrote:

On 4/25/22 02:10, Xiaojuan Yang wrote:

+static void output_r_csr(DisasContext *ctx, arg_r_csr *a,
+ const char *mnemonic)
+{
+    output(ctx, mnemonic, "r%d, %d # %s", a->rd, a->csr, 
csr_names[a->csr]);

+}
+
+static void output_rr_csr(DisasContext *ctx, arg_rr_csr *a,
+  const char *mnemonic)
+{
+    output(ctx, mnemonic, "r%d, r%d, %d # %s",
+   a->rd, a->rj, a->csr, csr_names[a->csr]);


Need to check for csr not in csr_names.


I will re-used  get_csr() to check csr,

like this:
if (get_csr() == NULL)
{
    printf("Undefined CSR.\n");
} else {
    printf(" %s\n",  csr_names[a->csr]);
}

+    CSR_OFF_FLAGS(CPUID, CSRFL_READONLY),


You've dropped the special case from the previous version.  Why?


Hmm,   I readly lost this special case,  I will correct it on v3.

like this:
CSR_OFF_FUNCS(CPUID, CSRFL_READONLY,  gen_helper_csrrd_cpuid, NULL),

target_ulong helper_csrrd_cpuid(CPULoongArchState *env)
{
    return (env_cpu(env))->cpu_index;
}

Thanks.
Xiaojuan




Re: [PATCH v1 25/43] target/loongarch: Add LoongArch CSR instruction

2022-04-21 Thread yangxiaojuan



On 2022/4/20 上午1:05, Richard Henderson wrote:

You'd use a store, just like you were already doing in trans_csrwr.

But here's how I'd improve this.  For avoidance of doubt, all of this 
would go in trans_priviledged.c.inc -- there's no use of csr_offsets[] 
outside of that file.


Thanks you very much,  I had tested this with bios,  it worked well, and 
I have two questions.


1. CSRFL_IO,   how to use it.   I don't understand  CPUState options  
'can_do_Io',

2./* fall through */,   this may have warning,  should we care about this?

Thanks.
Xiaojuan

r~




typedef void (*GenCSRRead)(TCGv dest, TCGv_ptr env);
typedef void (*GenCSRWrite)(TCGv dest, TCGv_ptr env, TCGv src);

typedef struct {
    int offset;
    int flags;
    GenCSRRead readfn;
    GenCSRWrite writefn;
} CSRInfo;

enum {
    CSRFL_READONLY  = (1 << 0),
    CSRFL_EXITTB    = (1 << 1),
    CSRFL_IO    = (1 << 2),
};

#define CSR_OFF_FUNCS(NAME, FL, RD, WR) \
    [LOONGARCH_CSR_##NAME] = { \
    .offset = offsetof(CPULoongArchState, CSR_##NAME), \
    .flags = FL, .readfn = RD, .writefn = WR   \
    }
#define CSR_OFF_FLAGS(NAME, FL) \
    CSR_OFF_FUNCS(NAME, FL, NULL, NULL)
#define CSR_OFF(NAME) \
    CSR_OFF_FLAGS(NAME, 0)

static const CSRInfo csr_info[] = {
    CSR_OFF(CRMD),
    CSR_OFF_FLAGS(CPUID, CSRFL_READONLY),
    CSR_OFF_FUNCS(TCFG, CSRFL_IO, NULL, gen_helper_csrwr_tcfg),
    CSR_OFF_FUNCS(TVAL, CSRFL_READONLY | CSRFL_IO, 
gen_helper_csrrd_tval, NULL),

    CSR_OFF_FUNCS(TICLR, CSRFL_IO, NULL, gen_helper_csrwr_ticlr),
    CSR_OFF_FUNCS(ESTAT, CSRFL_EXITTB, NULL, gen_helper_csrwr_estat),
    ...
};

static const CSRInfo *get_csr(unsigned csr_num)
{
    const CSRInfo *csr;
    if (csr_num < ARRAY_SIZE(csr_info)) {
    return NULL;
    }
    csr = _info[csr_num];
    if (csr->offset == 0) {
    return NULL; /* undefined */
    }
    return csr;
}

static bool check_csr_flags(DisasContext *ctx, const CSRInfo *csr, 
bool write)

{
    if ((info->flags & CSRFL_READONLY) && write) {
    return false;
    }
    if ((info->flags & CSRFL_IO) &&
    (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT)) {
    gen_io_start();
    ctx->base.is_jmp = DISAS_EXIT_UPDATE;
    } else if ((info->flags & CSRFL_EXITTB) && write) {
    ctx->base.is_jmp = DISAS_EXIT_UPDATE;
    }
    return true;
}

static bool trans_csrrd(DisasContext *ctx, arg_csrrd *a)
{
    TCGv dest;
    const CSRInfo *csr;

    if (check_plv(ctx)) {
    return false;
    }
    csr = get_csr(a->csr);
    if (csr == NULL) {
    /* CSR is undefined: read as 0 */
    dest = tcg_constant_tl(0);
    } else {
    check_csr_flags(ctx, csr, false);
    dest = gpr_dst(ctx, a->rd, EXT_NONE);
    if (csr->readfn) {
    csr_readfn(dest, cpu_env);
    } else {
    tcg_gen_ld_tl(dest, cpu_env, csr->offset);
    }
    }
    gen_set_gpr(a->rd, dest, EXT_NONE);
    return true;
}

static bool trans_csrwr(DisasContext *ctx, arg_csrwr *a)
{
    TCGv dest, src1;
    const CSRInfo *csr;

    if (check_plv(ctx)) {
    return false;
    }
    csr = get_csr_info(a->csr);
    if (csr == NULL) {
    /* CSR is undefined: write ignored, read old value as 0. */
    gen_set_gpr(a->rd, tcg_constant_tl(0), EXT_NONE);
    return true;
    }
    if (!check_csr_flags(ctx, csr, true)) {
    /* CSR is readonly: trap. */
    return false;
    }
    src1 = gpr_src(ctx, a->rd, EXT_NONE);
    if (csr->writefn) {
    dest = gpr_dst(ctx, a->rd, EXT_NONE);
    csr->writefn(dest, cpu_env, src1);
    } else {
 dest = temp_new(ctx);
 tcg_gen_ld_tl(dest, cpu_env, csr->offset);
 tcg_gen_st_tl(src1, cpu_env, csr->offset);
    }
    gen_set_gpr(a->rd, dest, EXT_NONE);
    return true;
}

static bool trans_csrxchg(DisasContext *ctx, arg_csrxchg *a)
{
    TCGv src1, mask, oldv, newv, temp;
    const CSRInfo *csr;

    if (check_plv(ctx)) {
    return false;
    }
    csr = get_csr_info(a->csr);
    if (csr == NULL) {
    /* CSR is undefined: write ignored, read old value as 0. */
    gen_set_gpr(a->rd, tcg_constant_tl(0), EXT_NONE);
    return true;
    }
    if (!check_csr_flags(ctx, csr, true)) {
    /* CSR is readonly: trap. */
    return false;
    }

    /* So far only readonly csrs have readfn. */
    assert(csr->readfn == NULL);

    src1 = gpr_src(ctx, a->rd, EXT_NONE);
    mask = gpr_src(ctx, a->rj, EXT_NONE);
    oldv = tcg_temp_new();
    newv = tcg_temp_new();
    temp = tcg_temp_new();

    tcg_gen_ld_tl(oldv, cpu_env, csr->offset);
    tcg_gen_and_tl(newv, src1, mask);
    tcg_gen_andc_tl(temp, oldv, mask);
    tcg_gen_or_tl(newv, newv, temp);

    if (csr->writefn) {
    csr->writefn(oldv, cpu_env, newv);
    } else {
    tcg_gen_st_tl(newv, cpu_env, csr->offset);
    }
    gen_set_gpr(a->rd, oldv, EXT_NONE);

    tcg_temp_free(temp);
    tcg_temp_free(newv);
    tcg_temp_free(oldv);
    return true;
}

and then in 

Re: [PATCH v1 33/43] hw/intc: Add LoongArch ls7a interrupt controller support(PCH-PIC)

2022-04-20 Thread yangxiaojuan


On 2022/4/20 上午1:14, Richard Henderson wrote:


The emulate of PCH_PIC_CLR in qemu LoongArchPCHPIC struct member is 
intirr_lo/hi(we devide 64bits reg to two 32bits reg to match the 
linux kernel), it will be changed when we config clear reg or handler 
irq.


static void loongarch_pch_pic_low_writew(void *opaque, hwaddr addr,
                                      uint64_t data, unsigned size)
{
...
case PCH_PIC_INT_CLEAR_LO:
     if (s->intedge_lo & data) {
         s->intirr_lo &= (~data);
         pch_pic_update_irq(s, data, 0, 0);
         s->intisr_lo &= (~data);
      }
     break;
case PCH_PIC_INT_CLEAR_HI:
     if (s->intedge_hi & data) {
         s->intirr_hi &= (~data);
         pch_pic_update_irq(s, data, 0, 1);
         s->intisr_hi &= (~data);
      }
     break;


One can just as easily do

    case PCH_PIC_INT_CLEAR_LO:
    data = (uint32_t)data;
    goto do_clear;
    case PCH_PIC_INT_CLEAR_HI:
    data <<= 32;
    do_clear:
    s->intrr &= ~data;
    pch_pic_update_irq(s...);
    s->intrs &= ~data;

with the values internal to qemu be represented with uint64_t instead 
of a pair of uint32_t.  Which would in fact be *much* clearer to read, 
and would seem to cut down the number of code lines required by half. 

Sorry, I didn't understand your means before.
I will fix it in this way. Repalcing pch_pic uint32 registers with 
uint64 and fix its' reading/writing options to keep consistency with the 
document.


Thanks.
Xiaojuan


Re: [PATCH v1 25/43] target/loongarch: Add LoongArch CSR instruction

2022-04-19 Thread yangxiaojuan



On 2022/4/16 上午9:04, Richard Henderson wrote:

On 4/15/22 02:40, Xiaojuan Yang wrote:

...

+void  helper_csr_update(CPULoongArchState *env, target_ulong new_val,
+    target_ulong csr_offset)
+{
+    uint64_t *csr = (void *)env + csr_offset;
+
+    *csr = new_val;
+}

This function should not exist

...

+    switch (a->csr) {
+    case LOONGARCH_CSR_ESTAT:
+    gen_helper_csrwr_estat(dest, cpu_env, new_val);
+    break;
+    case LOONGARCH_CSR_ASID:
+    gen_helper_csrwr_asid(dest, cpu_env, new_val);
+    break;
+    case LOONGARCH_CSR_TCFG:
+    gen_helper_csrwr_tcfg(dest, cpu_env, new_val);
+    break;
+    case LOONGARCH_CSR_TICLR:
+    gen_helper_csrwr_ticlr(dest, cpu_env, new_val);
+    break;
+    default:
+    tcg_gen_mov_tl(dest, old_val);
+    }
+
+    gen_helper_csr_update(cpu_env, new_val, 
tcg_constant_tl(csr_offset));


Note that helper_csr_update is nothing more than the store to csr_offset.
On trans_csrxchg() , I am don't know how to use a TCGv value 'new_val 
'to update an uint64_t value "CSR_XXX",  So I use helper_csr_update(),


Thanks.
Xiaojuan

r~





Re: [PATCH v1 33/43] hw/intc: Add LoongArch ls7a interrupt controller support(PCH-PIC)

2022-04-19 Thread yangxiaojuan


On 2022/4/18 下午10:39, Richard Henderson wrote:

On 4/18/22 02:14, yangxiaojuan wrote:

Hi, Richard

On 2022/4/18 上午11:15, Richard Henderson wrote:

On 4/15/22 02:40, Xiaojuan Yang wrote:

+static void pch_pic_update_irq(LoongArchPCHPIC *s, uint32_t mask,
+   int level, int hi)
+{
+    uint32_t val, irq;
+
+    if (level == 1) {
+    if (hi) {
+    val = mask & s->intirr_hi & (~s->int_mask_hi);
+    irq = find_first_bit((unsigned long *), 32);


This does not work -- you're accessing beyond the end of the 
uint32_t for all LP64 hosts.  I think you just want ctz32()...




+    if (irq != 32) {
+    s->intisr_hi |= 1ULL << irq;
+ qemu_set_irq(s->parent_irq[s->htmsi_vector[irq + 32]], 1);
+    }


... which should in fact only be tested if val != 0, which is to 
what this IF equates.


Is there a good reason that this function is treating hi and lo 
separately, as opposed to simply doing all of the computation on 
uint64_t?




In the part of linux kernel, pch pic driver use 32 bits for reading 
and writing.
e.g in the drivers/irqchip/irq-loongson-pch-pic.c, pch_pic_mask_irq() 
function use writel() to write pch_pic mask reg.


So?  update_irq is not writel.

I expect that you should have done something (manual deposit64 or 
.impl.min_access_size = 8) with the actual writel, but by the time we 
arrive in this subroutine we have a complete uint64_t.




In the linux kernel pch_pic driver, pch_pic_unmask_irq() use writel to 
config PCH_PIC_CLR reg:


writel(BIT(PIC_REG_BIT(d->hwirq)),
                    priv->base + PCH_PIC_CLR + PIC_REG_IDX(d->hwirq) * 4);

And writel() need u32 value.

static inline void writel(u32 value, volatile void __iomem *addr)
{
    __io_bw();
    __raw_writel((u32 __force)__cpu_to_le32(value), addr);
    __io_aw();
}

The emulate of PCH_PIC_CLR in qemu LoongArchPCHPIC struct member is 
intirr_lo/hi(we devide 64bits reg to two 32bits reg to match the linux 
kernel), it will be changed when we config clear reg or handler irq.


static void loongarch_pch_pic_low_writew(void *opaque, hwaddr addr,
                                     uint64_t data, unsigned size)
{
...
case PCH_PIC_INT_CLEAR_LO:
    if (s->intedge_lo & data) {
        s->intirr_lo &= (~data);
        pch_pic_update_irq(s, data, 0, 0);
        s->intisr_lo &= (~data);
     }
    break;
case PCH_PIC_INT_CLEAR_HI:
    if (s->intedge_hi & data) {
        s->intirr_hi &= (~data);
        pch_pic_update_irq(s, data, 0, 1);
        s->intisr_hi &= (~data);
     }
    break;
...
}

static void pch_pic_irq_handler(void *opaque, int irq, int level)
{
...
hi = (irq >= 32) ? 1 : 0;
if (hi) {
    irq = irq - 32;
}

 mask = 1ULL << irq;


 if (hi) {
    if (s->intedge_hi & mask) {
        /* Edge triggered */
        if (level) {
            if ((s->last_intirr_hi & mask) == 0) {
                s->intirr_hi |= mask;
            }
            s->last_intirr_hi |= mask;
        } else {
            s->last_intirr_hi &= ~mask;
        }
...
pch_pic_update_irq(s, mask, level, hi);
}

And in update_irq(), we should judge intirr,which irq number is pending.
so we also need whether is intirr_lo or intirr_hi.

static void pch_pic_update_irq(LoongArchPCHPIC *s, uint32_t mask,
                               int level, int hi)
{
    uint32_t val, irq;

    if (level) {
        if (hi) {
            val = mask & s->intirr_hi & (~s->int_mask_hi);
            if (val) {
                irq = ctz32(val);
                s->intisr_hi |= 1ULL << irq;
qemu_set_irq(s->parent_irq[s->htmsi_vector[irq + 32]], 1);
            }
..
}

Thanks.
Xiaojuan


r~


Re: [PATCH v1 35/43] hw/intc: Add LoongArch extioi interrupt controller(EIOINTC)

2022-04-18 Thread yangxiaojuan


On 2022/4/18 下午4:57, Mark Cave-Ayland wrote:

On 18/04/2022 04:48, Richard Henderson wrote:


On 4/15/22 02:40, Xiaojuan Yang wrote:

+ memory_region_init(>mmio[cpu], OBJECT(s),
+   "loongarch_extioi", EXTIOI_SIZE);
+
+    memory_region_init_io(>mmio_nodetype[cpu], OBJECT(s),
+  _nodetype_ops, s,
+  EXTIOI_LINKNAME(.nodetype),
+  IPMAP_OFFSET - APIC_BASE);
+    memory_region_add_subregion(>mmio[cpu], 0, 
>mmio_nodetype[cpu]);

+
+ memory_region_init_io(>mmio_ipmap_enable[cpu], OBJECT(s),
+  _ipmap_enable_ops, s,
+  EXTIOI_LINKNAME(.ipmap_enable),
+  BOUNCE_OFFSET - IPMAP_OFFSET);
+    memory_region_add_subregion(>mmio[cpu], IPMAP_OFFSET - 
APIC_BASE,

+ >mmio_ipmap_enable[cpu]);
+
+ memory_region_init_io(>mmio_bounce_coreisr[cpu], OBJECT(s),
+  _bounce_coreisr_ops, s,
+ EXTIOI_LINKNAME(.bounce_coreisr),
+  COREMAP_OFFSET - BOUNCE_OFFSET);
+    memory_region_add_subregion(>mmio[cpu], BOUNCE_OFFSET - 
APIC_BASE,

+ >mmio_bounce_coreisr[cpu]);
+
+    memory_region_init_io(>mmio_coremap[cpu], OBJECT(s),
+  _coremap_ops, s,
+  EXTIOI_LINKNAME(.coremap),
+  EXTIOI_COREMAP_END);
+    memory_region_add_subregion(>mmio[cpu], COREMAP_OFFSET - 
APIC_BASE,

+ >mmio_coremap[cpu]);


Why are these separate memory regions, instead of one region? They're 
certainly described in a single table in section 11.2 of the 3A5000 
manual...


The reason it was done like this is because there were different 
access sizes in the relevant _ops definitions. Certainly when I looked 
at the patches previously I wasn't able to easily see how these could 
be consolidated without digging deeper into the documentation.


Would it be better to keep consistent with the content of the 3A5000 
manual document? And only one memory region is used to represent the 
extioi iocsr region


Thanks.
Xiaojuan


ATB,

Mark.


Re: [PATCH v1 25/43] target/loongarch: Add LoongArch CSR instruction

2022-04-18 Thread yangxiaojuan


On 2022/4/16 上午9:04, Richard Henderson wrote:

+int cpu_csr_offset(unsigned csr_num);

...

+static const uint64_t csr_offsets[] = {


There's no reason for this array to be uint64_t.
It really should match the function. 

Yes,  we shoud do this.

If we use 'int', we may get a warning:
../target/loongarch/csr_helper.c:49:30: warning: overflow in implicit 
constant conversion [-Woverflow]

  [LOONGARCH_CSR_CPUID] = offsetof(CPUState, cpu_index)
                    ^~~~
How about use 'long'?  I had tested it no warning.

Thanks.
Xiaojuan


Re: [PATCH v1 33/43] hw/intc: Add LoongArch ls7a interrupt controller support(PCH-PIC)

2022-04-18 Thread yangxiaojuan

Hi, Richard

On 2022/4/18 上午11:15, Richard Henderson wrote:

On 4/15/22 02:40, Xiaojuan Yang wrote:

+static void pch_pic_update_irq(LoongArchPCHPIC *s, uint32_t mask,
+   int level, int hi)
+{
+    uint32_t val, irq;
+
+    if (level == 1) {
+    if (hi) {
+    val = mask & s->intirr_hi & (~s->int_mask_hi);
+    irq = find_first_bit((unsigned long *), 32);


This does not work -- you're accessing beyond the end of the uint32_t 
for all LP64 hosts.  I think you just want ctz32()...




+    if (irq != 32) {
+    s->intisr_hi |= 1ULL << irq;
+ qemu_set_irq(s->parent_irq[s->htmsi_vector[irq + 32]], 1);
+    }


... which should in fact only be tested if val != 0, which is to what 
this IF equates.


Is there a good reason that this function is treating hi and lo 
separately, as opposed to simply doing all of the computation on 
uint64_t?




In the part of linux kernel, pch pic driver use 32 bits for reading and 
writing.
e.g in the drivers/irqchip/irq-loongson-pch-pic.c, pch_pic_mask_irq() 
function use writel() to write pch_pic mask reg.


Thanks.
Xiaojuan


r~


Re: [RFC PATCH v7 14/29] hw/loongarch: Add support loongson3 virt machine type.

2022-04-15 Thread yangxiaojuan

Hi,

On 2022/3/29 上午5:02, Mark Cave-Ayland wrote:



+static const MemoryRegionOps loongarch_qemu_ops = {
+    .read = loongarch_qemu_read,
+    .write = loongarch_qemu_write,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+    .valid = {
+    .min_access_size = 4,
+    .max_access_size = 8,
+    },
+    .impl = {
+    .min_access_size = 4,
+    .max_access_size = 8,
+    },


The implementation above doesn't actually support access size 4; it 
only supports 8.

It doesn't seem like this should be a io region at all, but a ROM.


Strangely enough I had a similar requirement for my q800 patches, and 
when I tried to implement a ROM memory region then the accesses didn't 
work as expected. I can't remember the exact problem however... 
It seems that iocsr_misc may not use rom region, because the 
MISC_FUNC_REG should be writen.
could I modify it as a device? and define its structure and memregion 
options.


Thanks
Xiaojuan


Re: [RFC PATCH v7 19/29] hw/intc: Add LoongArch extioi interrupt controller(EIOINTC)

2022-03-31 Thread yangxiaojuan



On 2022/3/29 上午6:43, Richard Henderson wrote:

On 3/28/22 06:57, Xiaojuan Yang wrote:

This patch realize the EIOINTC interrupt controller.

Signed-off-by: Xiaojuan Yang 
Signed-off-by: Song Gao 
---
  hw/intc/Kconfig    |   3 +
  hw/intc/loongarch_extioi.c | 408 +
  hw/intc/meson.build    |   1 +
  hw/intc/trace-events   |  11 +
  hw/loongarch/Kconfig   |   1 +
  include/hw/intc/loongarch_extioi.h |  77 ++
  6 files changed, 501 insertions(+)
  create mode 100644 hw/intc/loongarch_extioi.c
  create mode 100644 include/hw/intc/loongarch_extioi.h

diff --git a/hw/intc/Kconfig b/hw/intc/Kconfig
index 71c04c328e..28bd1f185d 100644
--- a/hw/intc/Kconfig
+++ b/hw/intc/Kconfig
@@ -96,3 +96,6 @@ config LOONGARCH_PCH_MSI
  select MSI_NONBROKEN
  bool
  select UNIMP
+
+config LOONGARCH_EXTIOI
+    bool
diff --git a/hw/intc/loongarch_extioi.c b/hw/intc/loongarch_extioi.c
new file mode 100644
index 00..af28e8d6e9
--- /dev/null
+++ b/hw/intc/loongarch_extioi.c
@@ -0,0 +1,408 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Loongson 3A5000 ext interrupt controller emulation
+ *
+ * Copyright (C) 2021 Loongson Technology Corporation Limited
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/module.h"
+#include "qemu/log.h"
+#include "hw/irq.h"
+#include "hw/sysbus.h"
+#include "hw/loongarch/loongarch.h"
+#include "hw/qdev-properties.h"
+#include "exec/address-spaces.h"
+#include "hw/intc/loongarch_extioi.h"
+#include "migration/vmstate.h"
+#include "trace.h"
+
+static void extioi_update_irq(void *opaque, int irq_num, int level)
+{
+    LoongArchExtIOI *s = LOONGARCH_EXTIOI(opaque);


I think this is not opaque anymore; you've already resolved it in the 
caller.

I think level should be 'bool'.


OK.

+    uint8_t  ipnum, cpu;
+    unsigned long found1, found2;
+
+    ipnum = s->sw_ipmap[irq_num];
+    cpu   = s->sw_coremap[irq_num];
+    if (level == 1) {


Just if (level).


+    if (test_bit(irq_num, (void *)s->enable) == false) {


This, and every other cast you're using for bitops.h functions, is 
wrong.  You would need to declare these bitmaps properly as 'unsigned 
long name[BITS_TO_LONGS(N)];'.


That said, I would definitely use uint64_t, because that matches up 
with the description of these registers in the manual.


we may not declare these bitmaps as 'unsigned long 
name[BITS_TO_LONGS(N)]. For example, ext_sw_ipisr 
sw_ipisr[MAX_CORES][LS3A_INTC_IP] is a two-dimensional array,

and it has a specific meaning, memregion options also restrict its size

+    return;
+    }
+    bitmap_set((void *)s->coreisr[cpu], irq_num, 1);
+    found1 = find_next_bit((void *)&(s->sw_ipisr[cpu][ipnum]),
+   EXTIOI_IRQS, 0);


find_next_bit with offset=0 is find_first_bit...


OK.

+    bitmap_set((void *)&(s->sw_ipisr[cpu][ipnum]), irq_num, 1);
+
+    if (found1 >= EXTIOI_IRQS) {
+    qemu_set_irq(s->parent_irq[cpu][ipnum], level);
+    }


... but what's the bitmap search doing?  It appears to be checking 
that there are *no* bits set between 0 and EXTIOI_IRQS, and then 
raising the irq if no bits set.  That seems wrong.


found1 >= EXTIOI_IRQS says there is no interrupt at present, then the 
new interrupt will be sent.



+    } else {
+    bitmap_clear((void *)s->coreisr[cpu], irq_num, 1);
+    found1 = find_next_bit((void *)&(s->sw_ipisr[cpu][ipnum]),
+   EXTIOI_IRQS, 0);
+    bitmap_clear((void *)&(s->sw_ipisr[cpu][ipnum]), irq_num, 1);
+    found2 = find_next_bit((void *)&(s->sw_ipisr[cpu][ipnum]),
+   EXTIOI_IRQS, 0);
+
+    if ((found1 < EXTIOI_IRQS) && (found2 >= EXTIOI_IRQS)) {
+    qemu_set_irq(s->parent_irq[cpu][ipnum], level);
+    }
+    }
+}


if ((found1 < EXTIOI_IRQS) && (found2 >= EXTIOI_IRQS)) says if the 
current interrupt number is equal to irq_num, then qemu_set_irq.

It *seems* like all of this should be

    uint64_t sum = 0;

    s->isr[ipnum / 64] = deposit64(s->isr[ipnum / 64], ipnum % 64, 1, 
level);


    for (int i = 0; i < ARRAY_SIZE(s->isr); i++) {
    sum |= s->isr[i] & s->ena[i];
    }
    qemu_set_irq(parent, sum != 0);

If that's not the case, you need many more comments.


Yes, we need more comments,

Thanks.
Xiaojuan


r~





Re: [RFC PATCH v7 11/29] target/loongarch: Add LoongArch interrupt and exception handle

2022-03-30 Thread yangxiaojuan



On 2022/3/29 上午4:19, Richard Henderson wrote:

On 3/28/22 06:57, Xiaojuan Yang wrote:

1.This patch Add loongarch interrupt and exception handle.
2.Rename the user excp to the exccode from the csr defintions.

Signed-off-by: Xiaojuan Yang 
Signed-off-by: Song Gao 
---
  linux-user/loongarch64/cpu_loop.c |   8 +-
  target/loongarch/cpu.c    | 260 +-
  target/loongarch/cpu.h    |  11 -
  target/loongarch/fpu_helper.c |   2 +-
  target/loongarch/insn_trans/trans_extra.c.inc |   4 +-
  target/loongarch/translate.c  |   2 +-
  6 files changed, 261 insertions(+), 26 deletions(-)


To repeat my response to the cover letter, the changes in this patch 
should be folded back into the original patches defining the base 
architecture.


Agreed,  I think we can use this patch in new series,  but we need 
remove '2. Rename the user excp to the excode from the csr defintions'.


Thanks.
Xiaojuan


r~





Re: [RFC PATCH v7 17/29] hw/intc: Add LoongArch ls7a interrupt controller support(PCH-PIC)

2022-03-30 Thread yangxiaojuan



On 2022/3/29 上午4:18, Mark Cave-Ayland wrote:

On 28/03/2022 13:57, Xiaojuan Yang wrote:

This patch realize the PCH-PIC interrupt controller.

Signed-off-by: Xiaojuan Yang 
Signed-off-by: Song Gao 
---
  hw/intc/Kconfig |   4 +
  hw/intc/loongarch_pch_pic.c | 488 
  hw/intc/meson.build |   1 +
  hw/intc/trace-events    |   9 +
  hw/loongarch/Kconfig    |   1 +
  include/hw/intc/loongarch_pch_pic.h |  80 +
  6 files changed, 583 insertions(+)
  create mode 100644 hw/intc/loongarch_pch_pic.c
  create mode 100644 include/hw/intc/loongarch_pch_pic.h

diff --git a/hw/intc/Kconfig b/hw/intc/Kconfig
index 6c7e82da64..1fbba2e728 100644
--- a/hw/intc/Kconfig
+++ b/hw/intc/Kconfig
@@ -87,3 +87,7 @@ config M68K_IRQC
    config LOONGARCH_IPI
  bool
+
+config LOONGARCH_PCH_PIC
+    bool
+    select UNIMP
diff --git a/hw/intc/loongarch_pch_pic.c b/hw/intc/loongarch_pch_pic.c
new file mode 100644
index 00..04b9bdce36
--- /dev/null
+++ b/hw/intc/loongarch_pch_pic.c
@@ -0,0 +1,488 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * QEMU Loongson 7A1000 I/O interrupt controller.
+ *
+ * Copyright (C) 2021 Loongson Technology Corporation Limited
+ */
+
+#include "qemu/osdep.h"
+#include "hw/sysbus.h"
+#include "hw/loongarch/loongarch.h"
+#include "hw/irq.h"
+#include "hw/intc/loongarch_pch_pic.h"
+#include "migration/vmstate.h"
+#include "trace.h"
+
+static void pch_pic_update_irq(LoongArchPCHPIC *s, uint32_t mask,
+   int level, int hi)
+{
+    uint32_t val, irq;
+
+    if (level == 1) {
+    if (hi) {
+    val = mask & s->intirr_hi & (~s->int_mask_hi);
+    irq = find_first_bit((void *), 32);
+    if (irq != 32) {
+    s->intisr_hi |= 1ULL << irq;
+ qemu_set_irq(s->parent_irq[s->htmsi_vector[irq + 32]], 1);
+    }
+    } else {
+    val = mask & s->intirr_lo & (~s->int_mask_lo);
+    irq = find_first_bit((void *), 32);
+    if (irq != 32) {
+    s->intisr_lo |= 1ULL << irq;
+ qemu_set_irq(s->parent_irq[s->htmsi_vector[irq]], 1);
+    }
+    }
+    } else {
+    if (hi) {
+    val = mask & s->intisr_hi;
+    irq = find_first_bit((void *), 32);
+    if (irq != 32) {
+    s->intisr_hi &= ~(0x1ULL << irq);
+ qemu_set_irq(s->parent_irq[s->htmsi_vector[irq + 32]], 0);
+    }
+    } else {
+    val = mask & s->intisr_lo;
+    irq = find_first_bit((void *), 32);
+    if (irq != 32) {
+    s->intisr_lo &= ~(0x1ULL << irq);
+ qemu_set_irq(s->parent_irq[s->htmsi_vector[irq]], 0);
+    }
+    }
+    }
+}
+
+static void pch_pic_irq_handler(void *opaque, int irq, int level)
+{
+    LoongArchPCHPIC *s = LOONGARCH_PCH_PIC(opaque);
+    int hi = 0;
+    uint32_t mask;
+
+    assert(irq < PCH_PIC_IRQ_NUM);
+    trace_pch_pic_irq_handler(irq, level);
+
+    hi = (irq >= 32) ? 1 : 0;
+    if (hi) {
+    irq = irq - 32;
+    }
+
+    mask = 1ULL << irq;
+
+    if (hi) {
+    if (s->intedge_hi & mask) {
+    /* Edge triggered */
+    if (level) {
+    if ((s->last_intirr_hi & mask) == 0) {
+    s->intirr_hi |= mask;
+    }
+    s->last_intirr_hi |= mask;
+    } else {
+    s->last_intirr_hi &= ~mask;
+    }
+    } else {
+    /* Level triggered */
+    if (level) {
+    s->intirr_hi |= mask;
+    s->last_intirr_hi |= mask;
+    } else {
+    s->intirr_hi &= ~mask;
+    s->last_intirr_hi &= ~mask;
+    }
+    }
+    } else {
+    if (s->intedge_lo & mask) {
+    /* Edge triggered */
+    if (level) {
+    if ((s->last_intirr_lo & mask) == 0) {
+    s->intirr_lo |= mask;
+    }
+    s->last_intirr_lo |= mask;
+    } else {
+    s->last_intirr_lo &= ~mask;
+    }
+    } else {
+    /* Level triggered */
+    if (level) {
+    s->intirr_lo |= mask;
+    s->last_intirr_lo |= mask;
+    } else {
+    s->intirr_lo &= ~mask;
+    s->last_intirr_lo &= ~mask;
+    }
+
+    }
+    }
+    pch_pic_update_irq(s, mask, level, hi);
+}
+
+static uint64_t loongarch_pch_pic_low_readw(void *opaque, hwaddr addr,
+    unsigned size)
+{
+    LoongArchPCHPIC *s = LOONGARCH_PCH_PIC(opaque);
+    uint64_t val = 0;
+    uint32_t offset = addr & 0xfff;
+
+    switch (offset) {
+    case PCH_PIC_INT_ID_LO:
+    val = PCH_PIC_INT_ID_VAL;
+    break;
+    case PCH_PIC_INT_ID_HI:
+    val = PCH_PIC_INT_ID_NUM;
+    break;
+    case PCH_PIC_INT_MASK_LO:
+    val = s->int_mask_lo;
+    break;
+    

Re: [RFC PATCH v7 10/29] target/loongarch: Add other core instructions support

2022-03-30 Thread yangxiaojuan



On 2022/3/29 上午4:16, Richard Henderson wrote:

On 3/28/22 06:57, Xiaojuan Yang wrote:

+void helper_idle(CPULoongArchState *env)
+{
+    CPUState *cs = env_cpu(env);
+
+    cs->halted = 1;
+    cpu_reset_interrupt(cs, CPU_INTERRUPT_WAKE);
+    do_raise_exception(env, EXCP_HLT, 0);
+}


Why are you messing with CPU_INTERRUPT_WAKE?
You only ever reset it, and never set it.


Useless code,  we just need set cs->halted.

Thanks
Xiaojuan


r~





Re: [RFC PATCH v7 16/29] hw/loongarch: Add LoongArch ipi interrupt support(IPI)

2022-03-30 Thread yangxiaojuan

Hi Mark,

On 2022/3/29 上午4:15, Mark Cave-Ayland wrote:

+
+#define TYPE_LOONGARCH_IPI "loongarch_ipi"
+OBJECT_DECLARE_SIMPLE_TYPE(LoongArchIPI, LOONGARCH_IPI)
+
+typedef struct IPICore {
+    uint32_t status;
+    uint32_t en;
+    uint32_t set;
+    uint32_t clear;
+    /* 64bit buf divide into 2 32bit buf */
+    uint32_t buf[MAX_IPI_MBX_NUM * 2];
+    qemu_irq irq;
+} IPICore;
+
+typedef struct LoongArchIPI {
+    SysBusDevice parent_obj;
+    IPICore core[MAX_IPI_CORE_NUM];
+    MemoryRegion ipi_mmio[MAX_IPI_CORE_NUM];
+} LoongArchIPI;
+
+#endif


You missed the part in my original comment on patch 14 about dropping 
the typedef for QOM structs that are defined using 
OBJECT_DECLARE_TYPE_SIMPLE() i.e.



#define TYPE_LOONGARCH_IPI "loongarch_ipi"
OBJECT_DECLARE_SIMPLE_TYPE(LoongArchIPI, LOONGARCH_IPI)

...
...

typedef struct IPICore {
   uint32_t status;
   uint32_t en;
   uint32_t set;
   uint32_t clear;
   /* 64bit buf divide into 2 32bit buf */
   uint32_t buf[MAX_IPI_MBX_NUM * 2];
   qemu_irq irq;
} IPICore;

struct LoongArchIPI {
    SysBusDevice parent_obj;
    IPICore core[MAX_IPI_CORE_NUM];
    MemoryRegion ipi_mmio[MAX_IPI_CORE_NUM];
};


Sorry for that,   I will be more carefully.

Thanks.
Xiaojuan


ATB,

Mark. 


Re: [RFC PATCH v7 09/29] target/loongarch: Add TLB instruction support

2022-03-30 Thread yangxiaojuan



On 2022/3/29 上午4:12, Richard Henderson wrote:

On 3/28/22 06:57, Xiaojuan Yang wrote:

+static void output_empty(DisasContext *ctx, arg_empty *a,
+ const char *mnemonic)
+{
+}


No, you must still do

    output(ctx, mnemonic, "");


+static bool trans_tlbwr(DisasContext *ctx, arg_tlbwr *a)
+{
+    if (check_plv(ctx)) {
+    return false;
+    }
+    gen_helper_tlbwr(cpu_env);
+
+    if (ctx->mem_idx != MMU_DA_IDX) {
+    tcg_gen_movi_tl(cpu_pc, ctx->base.pc_next + 4);
+    ctx->base.is_jmp = DISAS_EXIT;
+    }


You may want to create a helper function for this condition.


in translate.c, like this:
static void gen_disas_exit(DisasContext *ctx)
{
    tcg_gen_movi_tl(cpu_pc, ctx->base.pc_next + 4);
    ctx->base.is_jmp = DISAS_EXIT;
}

+static bool trans_tlbclr(DisasContext *ctx, arg_tlbclr *a)
+{
+    if (check_plv(ctx)) {
+    return false;
+    }
+    gen_helper_tlbclr(cpu_env);
+    tcg_gen_movi_tl(cpu_pc, ctx->base.pc_next + 4);
+    ctx->base.is_jmp = DISAS_EXIT;


Missing it here...


gen_disas_exit()

+static bool trans_tlbflush(DisasContext *ctx, arg_tlbflush *a)
+{
+    if (check_plv(ctx)) {
+    return false;
+    }
+    gen_helper_tlbflush(cpu_env);
+    tcg_gen_movi_tl(cpu_pc, ctx->base.pc_next + 4);
+    ctx->base.is_jmp = DISAS_EXIT;
+    return true;
+}


... and here.


The same.

Thanks.
Xiaojuan


r~





Re: [RFC PATCH v7 05/29] target/loongarch: Add constant timer support

2022-03-30 Thread yangxiaojuan



On 2022/3/29 上午3:46, Richard Henderson wrote:

On 3/28/22 06:57, Xiaojuan Yang wrote:

+void cpu_loongarch_store_constant_timer_config(LoongArchCPU *cpu,
+   uint64_t value)
+{
+    CPULoongArchState *env = >env;
+    uint64_t now, next;
+
+    env->CSR_TCFG = value;
+    if (value & CONSTANT_TIMER_ENABLE) {
+    now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+    next = now + (value & CONSTANT_TIMER_TICK_MASK) * TIMER_PERIOD;
+    timer_mod(>timer, next);
+    }


If CONSTANT_TIMER_ENABLE is not set, you need to use timer_del() to 
turn off any existing timer.



OK



+void loongarch_constant_timer_cb(void *opaque)
+{
+    LoongArchCPU *cpu  = opaque;
+    CPULoongArchState *env = >env;
+    uint64_t now, next;
+
+    if (FIELD_EX64(env->CSR_TCFG, CSR_TCFG, PERIODIC)) {
+    now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+    next = now + (env->CSR_TCFG & CONSTANT_TIMER_TICK_MASK) * 
TIMER_PERIOD;

+    timer_mod(>timer, next);
+    } else {
+    env->CSR_TCFG = FIELD_DP64(env->CSR_TCFG, CSR_TCFG, EN, 0);
+    }
+
+    env->CSR_ESTAT |= 1 << IRQ_TIMER;
+    cpu_interrupt(CPU(cpu), CPU_INTERRUPT_HARD);


I think this is wrong and you should be using loongarch_cpu_set_irq 
(which is misplaced for you to be able to do so).



reuse loongarch_cpu_set_irq?  like this:
void loongarch_constant_timer_cb(void *opaque)
{
    ...
    if (FIELD_EX64(...)) {
    ...
    } else {
    ...
    }
    loongarch_cpu_set_irq(opaque, IRQ_IMER, 1);
}

@@ -297,4 +302,9 @@ enum {
  #define LOONGARCH_CPU_TYPE_NAME(model) model LOONGARCH_CPU_TYPE_SUFFIX
  #define CPU_RESOLVING_TYPE TYPE_LOONGARCH_CPU
  +void loongarch_constant_timer_cb(void *opaque);
+uint64_t cpu_loongarch_get_constant_timer_counter(LoongArchCPU *cpu);
+uint64_t cpu_loongarch_get_constant_timer_ticks(LoongArchCPU *cpu);
+void cpu_loongarch_store_constant_timer_config(LoongArchCPU *cpu,
+   uint64_t value);


These can go in internals.h.


OK

Thanks.
Xiaojuan


r~





Re: [RFC PATCH v7 02/29] target/loongarch: Add CSRs definition

2022-03-30 Thread yangxiaojuan



On 2022/3/29 上午3:16, Richard Henderson wrote:

On 3/28/22 06:57, Xiaojuan Yang wrote:

+++ b/target/loongarch/cpu.h
@@ -11,6 +11,7 @@
  #include "exec/cpu-defs.h"
  #include "fpu/softfloat-types.h"
  #include "hw/registerfields.h"
+#include "cpu-csr.h"


Do you need this include here?

No.

I would hope that there would be only a few extra files that need this.


I will correct it.

Thanks.
Xiaojuan

Otherwise,
Reviewed-by: Richard Henderson 


r~





Re: [RFC PATCH v7 08/29] target/loongarch: Add LoongArch IOCSR instruction

2022-03-30 Thread yangxiaojuan



On 2022/3/29 上午2:55, Richard Henderson wrote:

On 3/28/22 06:57, Xiaojuan Yang wrote:

+uint64_t helper_iocsr_read(CPULoongArchState *env, target_ulong r_addr,
+   uint32_t size)
+{
+    int cpuid = env_cpu(env)->cpu_index;
+    CPUState  *cs = qemu_get_cpu(cpuid);
+    env = cs->env_ptr;
+    uint64_t ret;
+
+    /*
+ * Adjust the per core address such as 0x10xx(IPI)/0x18xx(EXTIOI)
+ */
+    if (((r_addr & 0xff00) == 0x1000) || ((r_addr & 0xff00) == 
0x1800)) {

+    r_addr = r_addr + ((target_ulong)(cpuid & 0x3) << 8);
+    }
+
+    switch (size) {
+    case 1:
+    ret = address_space_ldub(>address_space_iocsr, r_addr,
+ MEMTXATTRS_UNSPECIFIED, NULL);
+    break;
+    case 2:
+    ret = address_space_lduw(>address_space_iocsr, r_addr,
+ MEMTXATTRS_UNSPECIFIED, NULL);
+    break;
+    case 4:
+    ret = address_space_ldl(>address_space_iocsr, r_addr,
+    MEMTXATTRS_UNSPECIFIED, NULL);
+    break;
+    case 8:
+    ret = address_space_ldq(>address_space_iocsr, r_addr,
+    MEMTXATTRS_UNSPECIFIED, NULL);
+    break;
+    default:
+    break;
+    }
+
+    return ret;
+}


You should have seen an uninitialized use of 'ret' here.
The default case should be g_assert_not_reached().
And the same in helper_iocsr_write.


OK.

Thanks.
Xiaojuan


r~





Re: [RFC PATCH v7 07/29] target/loongarch: Add LoongArch CSR instruction

2022-03-30 Thread yangxiaojuan


On 2022/3/29 上午2:34, Richard Henderson wrote:

+target_ulong helper_csr_rdq(CPULoongArchState *env, uint64_t csr)
+{
+    LoongArchCPU *cpu;
+    int64_t v;
+
+    switch (csr) {
+    case LOONGARCH_CSR_PGD:
+    if (env->CSR_TLBRERA & 0x1) {
+    v = env->CSR_TLBRBADV;
+    } else {
+    v = env->CSR_BADV;
+    }
+
+    if ((v >> 63) & 0x1) {
+    v = env->CSR_PGDH;
+    } else {
+    v = env->CSR_PGDL;
+    }
+    break;
+    case LOONGARCH_CSR_CPUID:
+    v = (env_cpu(env))->cpu_index;
+    break;
+    case LOONGARCH_CSR_TVAL:
+    cpu = LOONGARCH_CPU(env_cpu(env));
+    v = cpu_loongarch_get_constant_timer_ticks(cpu);
+    break;
+    default:
+    break;
+    }
+
+    return v;
+}


You should have seen a compiler warning for 'v' uninitialized here, 
via the default path.


The default path should not be reachable, because of code in 
trans_csrrd, and so could be written as g_assert_not_reachable(). 
However, I strongly suggest you split this function so that you do not 
need a switch here at all.  With CPUID now handled via cpu_csr_offset, 
there are only two helpers needed. 

trans_csrrd {
...
    switch(a->csr) {
    case LOONGARCH_CSR_PGD:
        gen_helper_csrrd_pgd();
        break;
    case LOONGARCH_CSR_TVAL:
        gen_helper_csrrd_tval();
        break;
    case LOONGARCH_CSR_CPUID:
        ...
    default:
        ...
    }
}
And the same in trans_csrwr, is this right?

Thanks.
Xiaojuan


Re: [RFC PATCH v7 00/29] Add LoongArch softmmu support

2022-03-30 Thread yangxiaojuan

Hi Richard.

On 2022/3/29 上午2:13, Richard Henderson wrote:

On 3/28/22 06:57, Xiaojuan Yang wrote:

This series patch add softmmu support for LoongArch.
The latest kernel:
   * https://github.com/loongson/linux/tree/loongarch-next
The latest uefi:
   * https://github.com/loongson/edk2
   * https://github.com/loongson/edk2-platforms
The manual:
   * 
https://github.com/loongson/LoongArch-Documentation/releases/tag/2021.10.11


You can get LoongArch qemu series like this:
    git clone https://github.com/loongson/qemu.git
    git checkout tcg-dev


I strongly suggest that you rebase *without* the patches for 
linux-user/loongarch64/.
If you do not do this, you will be stalled until your linux kernel 
patches are merged.


Testing this rebase locally, there is only a minor patch conflict in

    target/loongarch: Add LoongArch interrupt and exception handle

which suggests that the EXCP_* names be removed from

    target/loongarch: Add core definition

The EXCCODE_* names be introduced early instead, and all of the uses 
updated.


OK,  Thanks for your advice,  We will send a new series after solving 
all problems.


Thanks.
Xiaojuan


r~





Re: [RFC PATCH v6 00/29] Add LoongArch softmmu support

2022-03-10 Thread yangxiaojuan

Ping !!!

On 2022/2/25 下午4:02, Xiaojuan Yang wrote:

This series patch add softmmu support for LoongArch.
The latest kernel:
   * https://github.com/loongson/linux/tree/loongarch-next
The latest uefi:
   * https://github.com/loongson/edk2
   * https://github.com/loongson/edk2-platforms
The manual:
   * https://github.com/loongson/LoongArch-Documentation/releases/tag/2021.10.11

You can get LoongArch qemu series like this:
git clone https://github.com/loongson/qemu.git
git checkout tcg-dev


Changes for v6:

1. Add the new loongarch_cpu_init function.
2. Improved extioi memory region.
3. Replaced the original LS7A bridge with a new GPEX bridge.


Changes for v5:

1. Fix host bridge map irq function.
2. Move cpu timer init function into machine init.
3. Adjust memory region layout.
4. Add the documentation at docs/system/loongarch/loongson3.rst.
- Introduction to 3a5000 virt.
- Output of "info mtree".


Changes for v4:
1. Uefi code is open and add some fdt interface to pass info between qemu and 
uefi.
2. Use a per cpu address space for iocsr.
3. Modify the tlb emulation.
4. Machine and board code mainly follow Mark's advice.
5. Adjust pci host space map.
6. Use more memregion to simplify the interrupt controller's emulate.


Changes for v3:
1.Target code mainly follow Richard's code review comments.
2.Put the csr and iocsr read/write instruction emulate into 2 different patch.
3.Simply the tlb emulation.
4.Delete some unused csr registers defintion.
5.Machine and board code mainly follow Mark's advice, discard the obsolete 
interface.
6.NUMA function is removed for it is not completed.
7.Adjust some format problem and the Naming problem


Changes for v3:
1.Target code mainly follow Richard's code review comments.
2.Put the csr and iocsr read/write instruction emulate into 2 different patch.
3.Simply the tlb emulation.
4.Delete some unused csr registers defintion.
5.Machine and board code mainly follow Mark's advice, discard the obsolete 
interface.
6.NUMA function is removed for it is not completed.
7.Adjust some format problem and the Naming problem


Changes for v2:
1.Combine patch 2 and 3 into one.
2.Adjust the order of the patch.
3.Put all the binaries on the github.
4.Modify some emulate errors when use the kernel from the github.
5.Adjust some format problem and the Naming problem
6.Others mainly follow Richard's code review comments.

Please help review!

Thanks


Xiaojuan Yang (29):
   target/loongarch: Add system emulation introduction
   target/loongarch: Add CSRs definition
   target/loongarch: Add basic vmstate description of CPU.
   target/loongarch: Implement qmp_query_cpu_definitions()
   target/loongarch: Add constant timer support
   target/loongarch: Add MMU support for LoongArch CPU.
   target/loongarch: Add LoongArch CSR instruction
   target/loongarch: Add LoongArch IOCSR instruction
   target/loongarch: Add TLB instruction support
   target/loongarch: Add other core instructions support
   target/loongarch: Add LoongArch interrupt and exception handle
   target/loongarch: Add timer related instructions support.
   target/loongarch: Add gdb support.
   hw/loongarch: Add support loongson3 virt machine type.
   hw/loongarch: Add LoongArch cpu interrupt support(CPUINTC)
   hw/loongarch: Add LoongArch ipi interrupt support(IPI)
   hw/intc: Add LoongArch ls7a interrupt controller support(PCH-PIC)
   hw/intc: Add LoongArch ls7a msi interrupt controller support(PCH-MSI)
   hw/intc: Add LoongArch extioi interrupt controller(EIOINTC)
   hw/loongarch: Add irq hierarchy for the system
   Enable common virtio pci support for LoongArch
   hw/loongarch: Add some devices support for 3A5000.
   hw/loongarch: Add LoongArch ls7a rtc device support
   hw/loongarch: Add default bios startup support.
   hw/loongarch: Add -kernel and -initrd options support
   hw/loongarch: Add LoongArch smbios support
   hw/loongarch: Add LoongArch acpi support
   hw/loongarch: Add fdt support.
   tests/tcg/loongarch64: Add hello/memory test in loongarch64 system

  MAINTAINERS   |  20 +
  .../devices/loongarch64-softmmu/default.mak   |   3 +
  configs/targets/loongarch64-softmmu.mak   |   4 +
  docs/system/loongarch/loongson3.rst   |  37 +
  gdb-xml/loongarch-base64.xml  |  43 +
  gdb-xml/loongarch-fpu64.xml   |  57 ++
  hw/Kconfig|   1 +
  hw/acpi/Kconfig   |   4 +
  hw/acpi/ls7a.c| 374 +
  hw/acpi/meson.build   |   1 +
  hw/intc/Kconfig   |  15 +
  hw/intc/loongarch_extioi.c| 417 ++
  hw/intc/loongarch_ipi.c   | 164 
  hw/intc/loongarch_pch_msi.c   |  75 ++
  hw/intc/loongarch_pch_pic.c   | 488 +++
  hw/intc/meson.build   |   4 +
  hw/intc/trace-events   

Re: [RFC PATCH v5 00/30] Add LoongArch softmmu support

2022-02-10 Thread yangxiaojuan
Hi, Mark

On 02/05/2022 09:32 PM, Mark Cave-Ayland wrote:
> On 28/01/2022 03:40, Xiaojuan Yang wrote:
> 
>> This series patch add softmmu support for LoongArch.
>> The latest kernel:
>>* https://github.com/loongson/linux/tree/loongarch-next
>> The latest uefi:
>>* https://github.com/loongson/edk2
>>* https://github.com/loongson/edk2-platforms
>> The manual:
>>* 
>> https://github.com/loongson/LoongArch-Documentation/releases/tag/2021.10.11
>>
>> You can get LoongArch qemu series like this:
>> git clone https://github.com/loongson/qemu.git
>> git checkout tcg-dev
>>
>> Changes for v5:
>>
>> 1. Fix host bridge map irq function.
>> 2. Move cpu timer init function into machine init.
>> 3. Adjust memory region layout.
>> 4. Add the documentation at docs/system/loongarch/loongson3.rst.
>> - Introduction to 3a5000 virt.
>> - Output of "info mtree".
>>
>> Changes for v4:
>> 1. Uefi code is open and add some fdt interface to pass info between qemu 
>> and uefi.
>> 2. Use a per cpu address space for iocsr.
>> 3. Modify the tlb emulation.
>> 4. Machine and board code mainly follow Mark's advice.
>> 5. Adjust pci host space map.
>> 6. Use more memregion to simplify the interrupt controller's emulate.
>>
>>
>> Changes for v3:
>> 1.Target code mainly follow Richard's code review comments.
>> 2.Put the csr and iocsr read/write instruction emulate into 2 different 
>> patch.
>> 3.Simply the tlb emulation.
>> 4.Delete some unused csr registers defintion.
>> 5.Machine and board code mainly follow Mark's advice, discard the obsolete 
>> interface.
>> 6.NUMA function is removed for it is not completed.
>> 7.Adjust some format problem and the Naming problem
>>
>>
>> Changes for v3:
>> 1.Target code mainly follow Richard's code review comments.
>> 2.Put the csr and iocsr read/write instruction emulate into 2 different 
>> patch.
>> 3.Simply the tlb emulation.
>> 4.Delete some unused csr registers defintion.
>> 5.Machine and board code mainly follow Mark's advice, discard the obsolete 
>> interface.
>> 6.NUMA function is removed for it is not completed.
>> 7.Adjust some format problem and the Naming problem
>>
>>
>> Changes for v2:
>> 1.Combine patch 2 and 3 into one.
>> 2.Adjust the order of the patch.
>> 3.Put all the binaries on the github.
>> 4.Modify some emulate errors when use the kernel from the github.
>> 5.Adjust some format problem and the Naming problem
>> 6.Others mainly follow Richard's code review comments.
>>
>> Please help review!
>>
>> Thanks
>>
>>
>> Xiaojuan Yang (30):
>>target/loongarch: Add system emulation introduction
>>target/loongarch: Add CSRs definition
>>target/loongarch: Add basic vmstate description of CPU.
>>target/loongarch: Implement qmp_query_cpu_definitions()
>>target/loongarch: Add constant timer support
>>target/loongarch: Add MMU support for LoongArch CPU.
>>target/loongarch: Add LoongArch CSR instruction
>>target/loongarch: Add LoongArch IOCSR instruction
>>target/loongarch: Add TLB instruction support
>>target/loongarch: Add other core instructions support
>>target/loongarch: Add LoongArch interrupt and exception handle
>>target/loongarch: Add timer related instructions support.
>>target/loongarch: Add gdb support.
>>hw/pci-host: Add ls7a1000 PCIe Host bridge support for Loongson3
>>  Platform
>>hw/loongarch: Add support loongson3-ls7a machine type.
>>hw/loongarch: Add LoongArch cpu interrupt support(CPUINTC)
>>hw/loongarch: Add LoongArch ipi interrupt support(IPI)
>>hw/intc: Add LoongArch ls7a interrupt controller support(PCH-PIC)
>>hw/intc: Add LoongArch ls7a msi interrupt controller support(PCH-MSI)
>>hw/intc: Add LoongArch extioi interrupt controller(EIOINTC)
>>hw/loongarch: Add irq hierarchy for the system
>>Enable common virtio pci support for LoongArch
>>hw/loongarch: Add some devices support for 3A5000.
>>hw/loongarch: Add LoongArch ls7a rtc device support
>>hw/loongarch: Add default bios startup support.
>>hw/loongarch: Add -kernel and -initrd options support
>>hw/loongarch: Add LoongArch smbios support
>>hw/loongarch: Add LoongArch acpi support
>>hw/loongarch: Add fdt support.
>>tests/tcg/loongarch64: Add hello/memory test in loongarch64 system
>>
>>   .../devices/loongarch64-softmmu/default.mak   |   3 +
>>   configs/targets/loongarch64-softmmu.mak   |   4 +
>>   docs/system/loongarch/loongson3.rst   |  78 ++
>>   gdb-xml/loongarch-base64.xml  |  43 +
>>   gdb-xml/loongarch-fpu64.xml   |  57 ++
>>   hw/Kconfig|   1 +
>>   hw/acpi/Kconfig   |   4 +
>>   hw/acpi/ls7a.c| 374 +
>>   hw/acpi/meson.build   |   1 +
>>   hw/intc/Kconfig   |  15 +
>>   hw/intc/loongarch_extioi.c| 409 ++
>>   

Re: [RFC PATCH v4 03/30] target/loongarch: Add basic vmstate description of CPU.

2022-01-27 Thread yangxiaojuan
Hi, Mark

在 2022年01月15日 20:52, Mark Cave-Ayland 写道:
> On 08/01/2022 09:13, Xiaojuan Yang wrote:
> 
>> This patch introduce vmstate_loongarch_cpu
>>
>> Signed-off-by: Xiaojuan Yang 
>> Signed-off-by: Song Gao 
>> Reviewed-by: Richard Henderson 
>> ---
>>   target/loongarch/cpu.c   |  3 ++
>>   target/loongarch/internals.h |  4 ++
>>   target/loongarch/machine.c   | 84 
>>   target/loongarch/meson.build |  6 +++
>>   4 files changed, 97 insertions(+)
>>   create mode 100644 target/loongarch/machine.c
>>
>> diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c
>> index ed03ec2986..6e3dc5e6fa 100644
>> --- a/target/loongarch/cpu.c
>> +++ b/target/loongarch/cpu.c
>> @@ -320,6 +320,9 @@ static void loongarch_cpu_class_init(ObjectClass *c, 
>> void *data)
>>   cc->has_work = loongarch_cpu_has_work;
>>   cc->dump_state = loongarch_cpu_dump_state;
>>   cc->set_pc = loongarch_cpu_set_pc;
>> +#ifndef CONFIG_USER_ONLY
>> +dc->vmsd = _loongarch_cpu;
>> +#endif
> 
> Do we need CONFIG_USER_ONLY guards around dc->vmsd? I'd expect this to simply 
> be ignored in linux-user mode. Again it's a bit hard to see the full context 
> without having the complete series available in git somewhere.

I have reorganized the patch and prepare to send the V5 patch.
Here we define the vmsd struct in the machine.c which only used by system mode. 
So we need the CONFIG_USER_ONLY.

You can get LoongArch qemu series code like this:
git clone https://github.com/loongson/qemu.git
git checkout branch tcg-dev

xiaojuan,
thanks 

> 
>>   cc->disas_set_info = loongarch_cpu_disas_set_info;
>>   #ifdef CONFIG_TCG
>>   cc->tcg_ops = _tcg_ops;
>> diff --git a/target/loongarch/internals.h b/target/loongarch/internals.h
>> index 774a87ec80..c8e6f7012c 100644
>> --- a/target/loongarch/internals.h
>> +++ b/target/loongarch/internals.h
>> @@ -25,4 +25,8 @@ const char *loongarch_exception_name(int32_t exception);
>> void restore_fp_status(CPULoongArchState *env);
>>   +#ifndef CONFIG_USER_ONLY
>> +extern const VMStateDescription vmstate_loongarch_cpu;
>> +#endif
>> +
>>   #endif
>> diff --git a/target/loongarch/machine.c b/target/loongarch/machine.c
>> new file mode 100644
>> index 00..b9effe6db2
>> --- /dev/null
>> +++ b/target/loongarch/machine.c
>> @@ -0,0 +1,84 @@
>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>> +/*
>> + * QEMU LoongArch machine State
>> + *
>> + * Copyright (c) 2021 Loongson Technology Corporation Limited
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "cpu.h"
>> +#include "migration/cpu.h"
>> +
>> +/* LoongArch CPU state */
>> +
>> +const VMStateDescription vmstate_loongarch_cpu = {
>> +.name = "cpu",
>> +.version_id = 0,
>> +.minimum_version_id = 0,
>> +.fields = (VMStateField[]) {
>> +
>> +VMSTATE_UINTTL_ARRAY(env.gpr, LoongArchCPU, 32),
>> +VMSTATE_UINTTL(env.pc, LoongArchCPU),
>> +VMSTATE_UINT64_ARRAY(env.fpr, LoongArchCPU, 32),
>> +VMSTATE_UINT32(env.fcsr0, LoongArchCPU),
>> +
>> +/* Remaining CSR registers */
>> +VMSTATE_UINT64(env.CSR_CRMD, LoongArchCPU),
>> +VMSTATE_UINT64(env.CSR_PRMD, LoongArchCPU),
>> +VMSTATE_UINT64(env.CSR_EUEN, LoongArchCPU),
>> +VMSTATE_UINT64(env.CSR_MISC, LoongArchCPU),
>> +VMSTATE_UINT64(env.CSR_ECFG, LoongArchCPU),
>> +VMSTATE_UINT64(env.CSR_ESTAT, LoongArchCPU),
>> +VMSTATE_UINT64(env.CSR_ERA, LoongArchCPU),
>> +VMSTATE_UINT64(env.CSR_BADV, LoongArchCPU),
>> +VMSTATE_UINT64(env.CSR_BADI, LoongArchCPU),
>> +VMSTATE_UINT64(env.CSR_EENTRY, LoongArchCPU),
>> +VMSTATE_UINT64(env.CSR_TLBIDX, LoongArchCPU),
>> +VMSTATE_UINT64(env.CSR_TLBEHI, LoongArchCPU),
>> +VMSTATE_UINT64(env.CSR_TLBELO0, LoongArchCPU),
>> +VMSTATE_UINT64(env.CSR_TLBELO1, LoongArchCPU),
>> +VMSTATE_UINT64(env.CSR_ASID, LoongArchCPU),
>> +VMSTATE_UINT64(env.CSR_PGDL, LoongArchCPU),
>> +VMSTATE_UINT64(env.CSR_PGDH, LoongArchCPU),
>> +VMSTATE_UINT64(env.CSR_PGD, LoongArchCPU),
>> +VMSTATE_UINT64(env.CSR_PWCL, LoongArchCPU),
>> +VMSTATE_UINT64(env.CSR_PWCH, LoongArchCPU),
>> +VMSTATE_UINT64(env.CSR_STLBPS, LoongArchCPU),
>> +VMSTATE_UINT64(env.CSR_RVACFG, LoongArchCPU),
>> +VMSTATE_UINT64(env.CSR_CPUID, LoongArchCPU),
>> +VMSTATE_UINT64(env.CSR_PRCFG1, LoongArchCPU),
>> +VMSTATE_UINT64(env.CSR_PRCFG2, LoongArchCPU),
>> +VMSTATE_UINT64(env.CSR_PRCFG3, LoongArchCPU),
>> +VMSTATE_UINT64_ARRAY(env.CSR_SAVE, LoongArchCPU, 16),
>> +VMSTATE_UINT64(env.CSR_TID, LoongArchCPU),
>> +VMSTATE_UINT64(env.CSR_TCFG, LoongArchCPU),
>> +VMSTATE_UINT64(env.CSR_TVAL, LoongArchCPU),
>> +VMSTATE_UINT64(env.CSR_CNTC, LoongArchCPU),
>> +VMSTATE_UINT64(env.CSR_TICLR, LoongArchCPU),
>> +VMSTATE_UINT64(env.CSR_LLBCTL, LoongArchCPU),
>> +

Re: [RFC PATCH v4 00/30] Add LoongArch softmmu support.

2022-01-16 Thread yangxiaojuan
Hi, Mark

On 01/15/2022 10:11 PM, Mark Cave-Ayland wrote:
> On 08/01/2022 09:13, Xiaojuan Yang wrote:
> 
>> This series patch add softmmu support for LoongArch.
>> Base on the linux-user emulation support V14 patch.
>>* https://patchew.org/QEMU/20220106094200.1801206-1-gaos...@loongson.cn/
>> The latest kernel:
>>* https://github.com/loongson/linux/tree/loongarch-next
>> The latest uefi:
>>* https://github.com/loongson/edk2
>>* https://github.com/loongson/edk2-platforms
>> The manual:
>>* 
>> https://github.com/loongson/LoongArch-Documentation/releases/tag/2021.10.11
>>
>>
>> Changes for v4:
>> 1. Uefi code is open and add some fdt interface to pass info between qemu 
>> and uefi.
>> 2. Use a per cpu address space for iocsr.
>> 3. Modify the tlb emulation.
>> 4. Machine and board code mainly follow Mark's advice.
>> 5. Adjust pci host space map.
>> 6. Use more memregion to simplify the interrupt controller's emulate.
>>
>>
>> Changes for v3:
>> 1.Target code mainly follow Richard's code review comments.
>> 2.Put the csr and iocsr read/write instruction emulate into 2 different 
>> patch.
>> 3.Simply the tlb emulation.
>> 4.Delete some unused csr registers defintion.
>> 5.Machine and board code mainly follow Mark's advice, discard the obsolete 
>> interface.
>> 6.NUMA function is removed for it is not completed.
>> 7.Adjust some format problem and the Naming problem
>>
>>
>> Changes for v2:
>> 1.Combine patch 2 and 3 into one.
>> 2.Adjust the order of the patch.
>> 3.Put all the binaries on the github.
>> 4.Modify some emulate errors when use the kernel from the github.
>> 5.Adjust some format problem and the Naming problem
>> 6.Others mainly follow Richard's code review comments.
>>
>> Please help review!
>>
>> Thanks
>>
>> Xiaojuan Yang (30):
>>target/loongarch: Update README
>>target/loongarch: Add CSR registers definition
>>target/loongarch: Add basic vmstate description of CPU.
>>target/loongarch: Implement qmp_query_cpu_definitions()
>>target/loongarch: Add constant timer support
>>target/loongarch: Add MMU support for LoongArch CPU.
>>target/loongarch: Add LoongArch CSR instruction
>>target/loongarch: Add LoongArch IOCSR instruction
>>target/loongarch: Add TLB instruction support
>>target/loongarch: Add other core instructions support
>>target/loongarch: Add LoongArch interrupt and exception handle
>>target/loongarch: Add timer related instructions support.
>>target/loongarch: Add gdb support.
>>hw/pci-host: Add ls7a1000 PCIe Host bridge support for Loongson3
>>  Platform
>>hw/loongarch: Add support loongson3-ls7a machine type.
>>hw/loongarch: Add LoongArch cpu interrupt support(CPUINTC)
>>hw/loongarch: Add LoongArch ipi interrupt support(IPI)
>>hw/intc: Add LoongArch ls7a interrupt controller support(PCH-PIC)
>>hw/intc: Add LoongArch ls7a msi interrupt controller support(PCH-MSI)
>>hw/intc: Add LoongArch extioi interrupt controller(EIOINTC)
>>hw/loongarch: Add irq hierarchy for the system
>>Enable common virtio pci support for LoongArch
>>hw/loongarch: Add some devices support for 3A5000.
>>hw/loongarch: Add LoongArch ls7a rtc device support
>>hw/loongarch: Add default bios startup support.
>>hw/loongarch: Add -kernel and -initrd options support
>>hw/loongarch: Add LoongArch smbios support
>>hw/loongarch: Add LoongArch acpi support
>>hw/loongarch: Add fdt support.
>>tests/tcg/loongarch64: Add hello/memory test in loongarch64 system
>>
>>   .../devices/loongarch64-softmmu/default.mak   |   3 +
>>   configs/targets/loongarch64-softmmu.mak   |   4 +
>>   gdb-xml/loongarch-base64.xml  |  43 +
>>   gdb-xml/loongarch-fpu64.xml   |  57 ++
>>   hw/Kconfig|   1 +
>>   hw/acpi/Kconfig   |   4 +
>>   hw/acpi/ls7a.c| 374 +
>>   hw/acpi/meson.build   |   1 +
>>   hw/intc/Kconfig   |  15 +
>>   hw/intc/loongarch_extioi.c| 376 +
>>   hw/intc/loongarch_ipi.c   | 164 
>>   hw/intc/loongarch_pch_msi.c   |  75 ++
>>   hw/intc/loongarch_pch_pic.c   | 428 ++
>>   hw/intc/meson.build   |   4 +
>>   hw/intc/trace-events  |  25 +
>>   hw/loongarch/Kconfig  |  22 +
>>   hw/loongarch/acpi-build.c | 636 ++
>>   hw/loongarch/fw_cfg.c |  33 +
>>   hw/loongarch/fw_cfg.h |  15 +
>>   hw/loongarch/loongson3.c  | 685 +++
>>   hw/loongarch/meson.build  |   6 +
>>   hw/meson.build|   1 +
>>   hw/pci-host/Kconfig   |   4 +
>>   hw/pci-host/ls7a.c  

Re: [RFC PATCH v4 00/30] Add LoongArch softmmu support.

2022-01-09 Thread yangxiaojuan
Hi, Xuerui

Thank you for all you advice, I will modify the target part carefully.

Xiaojuan


On 01/09/2022 05:26 PM, WANG Xuerui wrote:
> Hi Xiaojuan,
> 
> I've just finished reviewing the first part (target modifications) as I'm not 
> familiar with QEMU device emulation. You may have to revise the target part 
> carefully, and re-organize at the series level to accelerate upstreaming 
> though, as Richard pointed out in the other patch series (Song Gao's 
> LoongArch linux-user support series) that the series as a whole is blocked.
> 
> 
> On 1/8/22 17:13, Xiaojuan Yang wrote:
>> This series patch add softmmu support for LoongArch.
>> Base on the linux-user emulation support V14 patch.
>>*https://patchew.org/QEMU/20220106094200.1801206-1-gaos...@loongson.cn/
> 
> There's a recognized syntax for marking patch series dependency [1], so that 
> your series could be auto-applied by Patchew for ease of consumption. You can 
> look at Song Gao's v14 LoongArch linux-user series for example usage.
> 
> [1]: 
> https://www.qemu.org/docs/master/devel/submitting-a-patch.html#base-patches-against-current-git-master
> 
>> The latest kernel:
>>*https://github.com/loongson/linux/tree/loongarch-next
>> The latest uefi:
>>*https://github.com/loongson/edk2
>>*https://github.com/loongson/edk2-platforms
>> The manual:
>>
>> *https://github.com/loongson/LoongArch-Documentation/releases/tag/2021.10.11
>>
>>
>> Changes for v4:
>> 1. Uefi code is open and add some fdt interface to pass info between qemu 
>> and uefi.
>> 2. Use a per cpu address space for iocsr.
>> 3. Modify the tlb emulation.
>> 4. Machine and board code mainly follow Mark's advice.
>> 5. Adjust pci host space map.
>> 6. Use more memregion to simplify the interrupt controller's emulate.
>>
>>
>> Changes for v3:
>> 1.Target code mainly follow Richard's code review comments.
>> 2.Put the csr and iocsr read/write instruction emulate into 2 different 
>> patch.
>> 3.Simply the tlb emulation.
>> 4.Delete some unused csr registers defintion.
>> 5.Machine and board code mainly follow Mark's advice, discard the obsolete 
>> interface.
>> 6.NUMA function is removed for it is not completed.
>> 7.Adjust some format problem and the Naming problem
>>
>>
>> Changes for v2:
>> 1.Combine patch 2 and 3 into one.
>> 2.Adjust the order of the patch.
>> 3.Put all the binaries on the github.
>> 4.Modify some emulate errors when use the kernel from the github.
>> 5.Adjust some format problem and the Naming problem
>> 6.Others mainly follow Richard's code review comments.
>>
>> Please help review!
>>
>> Thanks
>>
>> Xiaojuan Yang (30):
>>target/loongarch: Update README
>>target/loongarch: Add CSR registers definition
>>target/loongarch: Add basic vmstate description of CPU.
> 
> There are serious issues with your commit message...
> 
> First of all, some of your commit message titles end with a period, while 
> some don't; the QEMU convention is to NOT use one. So please fix all commits 
> like this to remove the trailing period.
> 
>>target/loongarch: Implement qmp_query_cpu_definitions()
>>target/loongarch: Add constant timer support
> "Implement the constant timer" would be more concise and idiomatic English.
>>target/loongarch: Add MMU support for LoongArch CPU.
>>target/loongarch: Add LoongArch CSR instruction
>>target/loongarch: Add LoongArch IOCSR instruction
> You don't need to emphasize "LoongArch" because the component prefix 
> "target/loongarch" says it all. Also all of these commits add support for 
> multiple instructions at once, so you would say "instructions". You may need 
> to check all places for simple plural form mistakes like these.
>>target/loongarch: Add TLB instruction support
>>target/loongarch: Add other core instructions support
>>target/loongarch: Add LoongArch interrupt and exception handle
> "handlers"?
>>target/loongarch: Add timer related instructions support.
>>target/loongarch: Add gdb support.
>>hw/pci-host: Add ls7a1000 PCIe Host bridge support for Loongson3
>>  Platform
> "Add the LS7A1000 PCIe host bridge" would be enough; although currently the 
> LS7A chip is only paired with Loongson 3 CPUs, there's no intrinsic reasons 
> to only support this combination ever.
>>hw/loongarch: Add support loongson3-ls7a machine type.
> "Support the loongson3-ls7a machine type"
>>hw/loongarch: Add LoongArch cpu interrupt support(CPUINTC)
> You may just say "Implement the LoongArch CPUINTC"; people naturally look in 
> the diff to get what CPUINTC means. Same for other following commits with 
> similar commit messages.
>>hw/loongarch: Add LoongArch ipi interrupt support(IPI)
>>hw/intc: Add LoongArch ls7a interrupt controller support(PCH-PIC)
>>hw/intc: Add LoongArch ls7a msi interrupt controller support(PCH-MSI)
>>hw/intc: Add LoongArch extioi interrupt controller(EIOINTC)
>>hw/loongarch: Add irq hierarchy for the system
>>Enable common virtio 

Re: [RFC PATCH v3 22/27] hw/loongarch: Add some devices support for 3A5000.

2022-01-09 Thread yangxiaojuan
Hi, Mark

On 12/23/2021 06:52 PM, Mark Cave-Ayland wrote:
> On 22/12/2021 08:26, yangxiaojuan wrote:
> 
>> Hi, Mark
>>
>> On 12/18/2021 06:02 PM, Mark Cave-Ayland wrote:
>>> On 04/12/2021 12:07, Xiaojuan Yang wrote:
>>>
>>>> 1.Add uart,virtio-net,vga and usb for 3A5000.
>>>> 2.Add irq set and map for the pci host. Non pci device
>>>> use irq 0-16, pci device use 16-64.
>>>> 3.Add some unimplented device to emulate guest unused
>>>> memory space.
>>>>
>>>> Signed-off-by: Xiaojuan Yang 
>>>> Signed-off-by: Song Gao 
>>>> ---
>>>>hw/loongarch/Kconfig|  8 +
>>>>hw/loongarch/loongson3.c| 63 +++--
>>>>hw/pci-host/ls7a.c  | 42 +-
>>>>include/hw/intc/loongarch_ipi.h |  2 ++
>>>>include/hw/pci-host/ls7a.h  |  4 +++
>>>>softmmu/qdev-monitor.c  |  3 +-
>>>>6 files changed, 117 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/hw/loongarch/Kconfig b/hw/loongarch/Kconfig
>>>> index 468e3acc74..9ea3b92708 100644
>>>> --- a/hw/loongarch/Kconfig
>>>> +++ b/hw/loongarch/Kconfig
>>>> @@ -1,5 +1,13 @@
>>>>config LOONGSON3_LS7A
>>>>bool
>>>> +imply VGA_PCI
>>>> +imply VIRTIO_VGA
>>>> +imply PARALLEL
>>>> +imply PCI_DEVICES
>>>> +select ISA_BUS
>>>> +select SERIAL
>>>> +select SERIAL_ISA
>>>> +select VIRTIO_PCI
>>>>select PCI_EXPRESS_7A
>>>>select LOONGARCH_IPI
>>>>select LOONGARCH_PCH_PIC
>>>> diff --git a/hw/loongarch/loongson3.c b/hw/loongarch/loongson3.c
>>>> index c42f830208..e4a02e7c18 100644
>>>> --- a/hw/loongarch/loongson3.c
>>>> +++ b/hw/loongarch/loongson3.c
>>>> @@ -10,8 +10,11 @@
>>>>#include "qemu/datadir.h"
>>>>#include "qapi/error.h"
>>>>#include "hw/boards.h"
>>>> +#include "hw/char/serial.h"
>>>>#include "sysemu/sysemu.h"
>>>>#include "sysemu/qtest.h"
>>>> +#include "hw/irq.h"
>>>> +#include "net/net.h"
>>>>#include "sysemu/runstate.h"
>>>>#include "sysemu/reset.h"
>>>>#include "hw/loongarch/loongarch.h"
>>>> @@ -20,6 +23,7 @@
>>>>#include "hw/intc/loongarch_pch_pic.h"
>>>>#include "hw/intc/loongarch_pch_msi.h"
>>>>#include "hw/pci-host/ls7a.h"
>>>> +#include "hw/misc/unimp.h"
>>>>static void loongarch_cpu_reset(void *opaque)
>>>> @@ -91,11 +95,12 @@ static void sysbus_mmio_map_loongarch(SysBusDevice 
>>>> *dev, int n,
>>>>memory_region_add_subregion(iocsr, addr, dev->mmio[n].memory);
>>>>}
>>>>-static void loongson3_irq_init(MachineState *machine)
>>>> +static PCIBus *loongson3_irq_init(MachineState *machine)
>>>>{
>>>>LoongArchMachineState *lams = LOONGARCH_MACHINE(machine);
>>>> -DeviceState *ipi, *extioi, *pch_pic, *pch_msi, *cpudev;
>>>> +DeviceState *ipi, *extioi, *pch_pic, *pch_msi, *cpudev, *pciehost;
>>>>SysBusDevice *d;
>>>> +PCIBus *pci_bus;
>>>>int cpu, pin, i;
>>>>unsigned long ipi_addr;
>>>>@@ -135,6 +140,10 @@ static void loongson3_irq_init(MachineState 
>>>> *machine)
>>>>sysbus_realize_and_unref(d, _fatal);
>>>>sysbus_mmio_map(d, 0, LS7A_IOAPIC_REG_BASE);
>>>>+serial_mm_init(get_system_memory(), LS7A_UART_BASE, 0,
>>>> +   qdev_get_gpio_in(pch_pic, LS7A_UART_IRQ - 
>>>> PCH_PIC_IRQ_OFFSET),
>>>> +   115200, serial_hd(0), DEVICE_LITTLE_ENDIAN);
>>>> +
>>>>/* Connect 64 pch_pic irqs to extioi */
>>>>for (int i = 0; i < PCH_PIC_IRQ_NUM; i++) {
>>>>sysbus_connect_irq(d, i, qdev_get_gpio_in(extioi, i));
>>>> @@ -149,6 +158,35 @@ static void loongson3_irq_init(MachineState *machine)
>>>>sysbus_connect_irq(d, i,
>>>>   qdev_get_gpio_in(extioi, i + 
>>

Re: [RFC PATCH v3 18/27] hw/intc: Add LoongArch ls7a interrupt controller support(PCH-PIC)

2022-01-08 Thread yangxiaojuan
Hi,Mark:

  Sorry for the late reply. I just saw the mail after I send the v4 patch. I 
sorted the mail into different folders from
the qemu-devel, so I didn't see the mail in time. Sorry again.

Xiaojuan

On 12/23/2021 06:21 PM, Mark Cave-Ayland wrote:
> On 22/12/2021 02:38, yangxiaojuan wrote:
> 
>> Hi, Mark
>>
>> On 12/18/2021 08:33 AM, Mark Cave-Ayland wrote:
>>> On 04/12/2021 12:07, Xiaojuan Yang wrote:
>>>
>>>> This patch realize the PCH-PIC interrupt controller.
>>>>
>>>> Signed-off-by: Xiaojuan Yang 
>>>> Signed-off-by: Song Gao 
>>>> ---
>>>>hw/intc/Kconfig |   4 +
>>>>hw/intc/loongarch_pch_pic.c | 357 
>>>>hw/intc/meson.build |   1 +
>>>>hw/intc/trace-events|   5 +
>>>>hw/loongarch/Kconfig|   1 +
>>>>include/hw/intc/loongarch_pch_pic.h |  61 +
>>>>6 files changed, 429 insertions(+)
>>>>create mode 100644 hw/intc/loongarch_pch_pic.c
>>>>create mode 100644 include/hw/intc/loongarch_pch_pic.h
>>>>
>>>> diff --git a/hw/intc/Kconfig b/hw/intc/Kconfig
>>>> index 511dcac537..96da13ad1d 100644
>>>> --- a/hw/intc/Kconfig
>>>> +++ b/hw/intc/Kconfig
>>>> @@ -76,3 +76,7 @@ config M68K_IRQC
>>>>  config LOONGARCH_IPI
>>>>bool
>>>> +
>>>> +config LOONGARCH_PCH_PIC
>>>> +bool
>>>> +select UNIMP
>>>> diff --git a/hw/intc/loongarch_pch_pic.c b/hw/intc/loongarch_pch_pic.c
>>>> new file mode 100644
>>>> index 00..2ede29ceb0
>>>> --- /dev/null
>>>> +++ b/hw/intc/loongarch_pch_pic.c
>>>> @@ -0,0 +1,357 @@
>>>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>>>> +/*
>>>> + * QEMU Loongson 7A1000 I/O interrupt controller.
>>>> + *
>>>> + * Copyright (C) 2021 Loongson Technology Corporation Limited
>>>> + */
>>>> +
>>>> +#include "qemu/osdep.h"
>>>> +#include "hw/sysbus.h"
>>>> +#include "hw/loongarch/loongarch.h"
>>>> +#include "hw/irq.h"
>>>> +#include "hw/intc/loongarch_pch_pic.h"
>>>> +#include "migration/vmstate.h"
>>>> +#include "trace.h"
>>>> +
>>>> +#define for_each_set_bit(bit, addr, size) \
>>>> + for ((bit) = find_first_bit((addr), (size));\
>>>> +  (bit) < (size);\
>>>> +  (bit) = find_next_bit((addr), (size), (bit) + 1))
>>>> +
>>>> +static void pch_pic_update_irq(loongarch_pch_pic *s, uint64_t mask, int 
>>>> level)
>>>> +{
>>>> +int i;
>>>> +uint64_t val;
>>>> +val = mask & s->intirr & (~s->int_mask);
>>>> +
>>>> +for_each_set_bit(i, , 64) {
>>>> +if (level == 1) {
>>>> +if ((s->intisr & (0x1ULL << i)) == 0) {
>>>> +s->intisr |= 1ULL << i;
>>>> +qemu_set_irq(s->parent_irq[s->htmsi_vector[i]], 1);
>>>> +}
>>>> +} else if (level == 0) {
>>>> +if (s->intisr & (0x1ULL << i)) {
>>>> +s->intisr &= ~(0x1ULL << i);
>>>> +qemu_set_irq(s->parent_irq[s->htmsi_vector[i]], 0);
>>>> +}
>>>> +}
>>>> +}
>>>> +}
>>>
>>> The normal pattern would be to use something like:
>>>
>>> for (i = 0; i < 64; i++) {
>>>  if (level) {
>>>  s->intisr |= 1ULL << i;
>>>  } else {
>>>  s->intisr &= ~(0x1ULL << i);
>>>  }
>>>
>>>  qemu_set_irq(s->parent_irq[s->htmsi_vector[i]], level);
>>> }
>>>
>>> Why is it necessary to check the previous value of (s->intisr & (0x1ULL << 
>>> i)) here?
>>
>> Here check the previous value to avoid Unnecessary write. It seems make 
>> things more complicated. I will modify
> 
> In general a *_update_irq() function should be fine to propagate the IRQ up 
> to the parent directly: I think this is fine in this case because you

Re: [RFC PATCH v3 22/27] hw/loongarch: Add some devices support for 3A5000.

2021-12-22 Thread yangxiaojuan
Hi, Mark

On 12/18/2021 06:02 PM, Mark Cave-Ayland wrote:
> On 04/12/2021 12:07, Xiaojuan Yang wrote:
> 
>> 1.Add uart,virtio-net,vga and usb for 3A5000.
>> 2.Add irq set and map for the pci host. Non pci device
>> use irq 0-16, pci device use 16-64.
>> 3.Add some unimplented device to emulate guest unused
>> memory space.
>>
>> Signed-off-by: Xiaojuan Yang 
>> Signed-off-by: Song Gao 
>> ---
>>   hw/loongarch/Kconfig|  8 +
>>   hw/loongarch/loongson3.c| 63 +++--
>>   hw/pci-host/ls7a.c  | 42 +-
>>   include/hw/intc/loongarch_ipi.h |  2 ++
>>   include/hw/pci-host/ls7a.h  |  4 +++
>>   softmmu/qdev-monitor.c  |  3 +-
>>   6 files changed, 117 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/loongarch/Kconfig b/hw/loongarch/Kconfig
>> index 468e3acc74..9ea3b92708 100644
>> --- a/hw/loongarch/Kconfig
>> +++ b/hw/loongarch/Kconfig
>> @@ -1,5 +1,13 @@
>>   config LOONGSON3_LS7A
>>   bool
>> +imply VGA_PCI
>> +imply VIRTIO_VGA
>> +imply PARALLEL
>> +imply PCI_DEVICES
>> +select ISA_BUS
>> +select SERIAL
>> +select SERIAL_ISA
>> +select VIRTIO_PCI
>>   select PCI_EXPRESS_7A
>>   select LOONGARCH_IPI
>>   select LOONGARCH_PCH_PIC
>> diff --git a/hw/loongarch/loongson3.c b/hw/loongarch/loongson3.c
>> index c42f830208..e4a02e7c18 100644
>> --- a/hw/loongarch/loongson3.c
>> +++ b/hw/loongarch/loongson3.c
>> @@ -10,8 +10,11 @@
>>   #include "qemu/datadir.h"
>>   #include "qapi/error.h"
>>   #include "hw/boards.h"
>> +#include "hw/char/serial.h"
>>   #include "sysemu/sysemu.h"
>>   #include "sysemu/qtest.h"
>> +#include "hw/irq.h"
>> +#include "net/net.h"
>>   #include "sysemu/runstate.h"
>>   #include "sysemu/reset.h"
>>   #include "hw/loongarch/loongarch.h"
>> @@ -20,6 +23,7 @@
>>   #include "hw/intc/loongarch_pch_pic.h"
>>   #include "hw/intc/loongarch_pch_msi.h"
>>   #include "hw/pci-host/ls7a.h"
>> +#include "hw/misc/unimp.h"
>>   static void loongarch_cpu_reset(void *opaque)
>> @@ -91,11 +95,12 @@ static void sysbus_mmio_map_loongarch(SysBusDevice *dev, 
>> int n,
>>   memory_region_add_subregion(iocsr, addr, dev->mmio[n].memory);
>>   }
>>   -static void loongson3_irq_init(MachineState *machine)
>> +static PCIBus *loongson3_irq_init(MachineState *machine)
>>   {
>>   LoongArchMachineState *lams = LOONGARCH_MACHINE(machine);
>> -DeviceState *ipi, *extioi, *pch_pic, *pch_msi, *cpudev;
>> +DeviceState *ipi, *extioi, *pch_pic, *pch_msi, *cpudev, *pciehost;
>>   SysBusDevice *d;
>> +PCIBus *pci_bus;
>>   int cpu, pin, i;
>>   unsigned long ipi_addr;
>>   @@ -135,6 +140,10 @@ static void loongson3_irq_init(MachineState *machine)
>>   sysbus_realize_and_unref(d, _fatal);
>>   sysbus_mmio_map(d, 0, LS7A_IOAPIC_REG_BASE);
>>   +serial_mm_init(get_system_memory(), LS7A_UART_BASE, 0,
>> +   qdev_get_gpio_in(pch_pic, LS7A_UART_IRQ - 
>> PCH_PIC_IRQ_OFFSET),
>> +   115200, serial_hd(0), DEVICE_LITTLE_ENDIAN);
>> +
>>   /* Connect 64 pch_pic irqs to extioi */
>>   for (int i = 0; i < PCH_PIC_IRQ_NUM; i++) {
>>   sysbus_connect_irq(d, i, qdev_get_gpio_in(extioi, i));
>> @@ -149,6 +158,35 @@ static void loongson3_irq_init(MachineState *machine)
>>   sysbus_connect_irq(d, i,
>>  qdev_get_gpio_in(extioi, i + 
>> PCH_MSI_IRQ_START));
>>   }
>> +
>> +pciehost = qdev_new(TYPE_LS7A_HOST_DEVICE);
>> +d = SYS_BUS_DEVICE(pciehost);
>> +sysbus_realize_and_unref(d, _fatal);
>> +pci_bus = PCI_HOST_BRIDGE(pciehost)->bus;
>> +
>> +/* Connect 48 pci irq to pch_pic */
>> +for (i = 0; i < LS7A_PCI_IRQS; i++) {
>> +qdev_connect_gpio_out(pciehost, i,
>> +  qdev_get_gpio_in(pch_pic, i + 
>> LS7A_DEVICE_IRQS));
>> +}
>> +
>> +return pci_bus;
>> +}
>> +
>> +/* Network support */
>> +static void network_init(PCIBus *pci_bus)
>> +{
>> +int i;
>> +
>> +for (i = 0; i < nb_nics; i++) {
>> +NICInfo *nd = _table[i];
>> +
>> +if (!nd->model) {
>> +nd->model = g_strdup("virtio");
>> +}
>> +
>> +pci_nic_init_nofail(nd, pci_bus, nd->model, NULL);
>> +}
>>   }
>> static void loongson3_init(MachineState *machine)
>> @@ -161,6 +199,7 @@ static void loongson3_init(MachineState *machine)
>>   MemoryRegion *address_space_mem = get_system_memory();
>>   LoongArchMachineState *lams = LOONGARCH_MACHINE(machine);
>>   int i;
>> +PCIBus *pci_bus = NULL;
>> if (!cpu_model) {
>>   cpu_model = LOONGARCH_CPU_TYPE_NAME("Loongson-3A5000");
>> @@ -207,8 +246,26 @@ static void loongson3_init(MachineState *machine)
>>   memory_region_add_subregion(address_space_mem, 0x9000, 
>> >highmem);
>>   offset += highram_size;
>>   +/*
>> + * There are some invalid guest memory access.
>> + * Create some unimplemented devices to 

Re: [RFC PATCH v3 18/27] hw/intc: Add LoongArch ls7a interrupt controller support(PCH-PIC)

2021-12-21 Thread yangxiaojuan
Hi, Mark

On 12/18/2021 08:33 AM, Mark Cave-Ayland wrote:
> On 04/12/2021 12:07, Xiaojuan Yang wrote:
> 
>> This patch realize the PCH-PIC interrupt controller.
>>
>> Signed-off-by: Xiaojuan Yang 
>> Signed-off-by: Song Gao 
>> ---
>>   hw/intc/Kconfig |   4 +
>>   hw/intc/loongarch_pch_pic.c | 357 
>>   hw/intc/meson.build |   1 +
>>   hw/intc/trace-events|   5 +
>>   hw/loongarch/Kconfig|   1 +
>>   include/hw/intc/loongarch_pch_pic.h |  61 +
>>   6 files changed, 429 insertions(+)
>>   create mode 100644 hw/intc/loongarch_pch_pic.c
>>   create mode 100644 include/hw/intc/loongarch_pch_pic.h
>>
>> diff --git a/hw/intc/Kconfig b/hw/intc/Kconfig
>> index 511dcac537..96da13ad1d 100644
>> --- a/hw/intc/Kconfig
>> +++ b/hw/intc/Kconfig
>> @@ -76,3 +76,7 @@ config M68K_IRQC
>> config LOONGARCH_IPI
>>   bool
>> +
>> +config LOONGARCH_PCH_PIC
>> +bool
>> +select UNIMP
>> diff --git a/hw/intc/loongarch_pch_pic.c b/hw/intc/loongarch_pch_pic.c
>> new file mode 100644
>> index 00..2ede29ceb0
>> --- /dev/null
>> +++ b/hw/intc/loongarch_pch_pic.c
>> @@ -0,0 +1,357 @@
>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>> +/*
>> + * QEMU Loongson 7A1000 I/O interrupt controller.
>> + *
>> + * Copyright (C) 2021 Loongson Technology Corporation Limited
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "hw/sysbus.h"
>> +#include "hw/loongarch/loongarch.h"
>> +#include "hw/irq.h"
>> +#include "hw/intc/loongarch_pch_pic.h"
>> +#include "migration/vmstate.h"
>> +#include "trace.h"
>> +
>> +#define for_each_set_bit(bit, addr, size) \
>> + for ((bit) = find_first_bit((addr), (size));\
>> +  (bit) < (size);\
>> +  (bit) = find_next_bit((addr), (size), (bit) + 1))
>> +
>> +static void pch_pic_update_irq(loongarch_pch_pic *s, uint64_t mask, int 
>> level)
>> +{
>> +int i;
>> +uint64_t val;
>> +val = mask & s->intirr & (~s->int_mask);
>> +
>> +for_each_set_bit(i, , 64) {
>> +if (level == 1) {
>> +if ((s->intisr & (0x1ULL << i)) == 0) {
>> +s->intisr |= 1ULL << i;
>> +qemu_set_irq(s->parent_irq[s->htmsi_vector[i]], 1);
>> +}
>> +} else if (level == 0) {
>> +if (s->intisr & (0x1ULL << i)) {
>> +s->intisr &= ~(0x1ULL << i);
>> +qemu_set_irq(s->parent_irq[s->htmsi_vector[i]], 0);
>> +}
>> +}
>> +}
>> +}
> 
> The normal pattern would be to use something like:
> 
> for (i = 0; i < 64; i++) {
> if (level) {
> s->intisr |= 1ULL << i;
> } else {
> s->intisr &= ~(0x1ULL << i);
> }
> 
> qemu_set_irq(s->parent_irq[s->htmsi_vector[i]], level);
> }
> 
> Why is it necessary to check the previous value of (s->intisr & (0x1ULL << 
> i)) here?

Here check the previous value to avoid Unnecessary write. It seems make things 
more complicated. I will modify

> 
>> +static void pch_pic_irq_handler(void *opaque, int irq, int level)
>> +{
>> +loongarch_pch_pic *s = LOONGARCH_PCH_PIC(opaque);
>> +
>> +assert(irq < PCH_PIC_IRQ_NUM);
>> +uint64_t mask = 1ULL << irq;
>> +
>> +trace_pch_pic_irq_handler(s->intedge, irq, level);
>> +
>> +if (s->intedge & mask) {
>> +/* Edge triggered */
>> +if (level) {
>> +if ((s->last_intirr & mask) == 0) {
>> +s->intirr |= mask;
>> +}
>> +s->last_intirr |= mask;
>> +} else {
>> +s->last_intirr &= ~mask;
>> +}
>> +} else {
>> +/* Level triggered */
>> +if (level) {
>> +s->intirr |= mask;
>> +s->last_intirr |= mask;
>> +} else {
>> +s->intirr &= ~mask;
>> +s->last_intirr &= ~mask;
>> +}
>> +
>> +}
>> +pch_pic_update_irq(s, mask, level);
>> +}
>> +
>> +static uint64_t loongarch_pch_pic_reg_read(void *opaque, hwaddr addr,
>> +   unsigned size)
>> +{
>> +loongarch_pch_pic *s = LOONGARCH_PCH_PIC(opaque);
>> +uint64_t val = 0;
>> +uint32_t offset = addr & 0xfff;
>> +int64_t offset_tmp;
>> +
>> +if (size == 8) {
>> +switch (offset) {
>> +case PCH_PIC_INT_ID_OFFSET:
>> +val = (PCH_PIC_INT_ID_NUM << 32) | PCH_PIC_INT_ID_VAL;
>> +break;
>> +case PCH_PIC_INT_MASK_OFFSET:
>> +val =  s->int_mask;
>> +break;
>> +case PCH_PIC_INT_STATUS_OFFSET:
>> +val = s->intisr & (~s->int_mask);
>> +break;
>> +case PCH_PIC_INT_EDGE_OFFSET:
>> +val = s->intedge;
>> +break;
>> +case PCH_PIC_INT_POL_OFFSET:
>> +val = s->int_polarity;
>> +break;
>> +case PCH_PIC_HTMSI_EN_OFFSET...PCH_PIC_HTMSI_EN_END:
>> +val = 

Re: [RFC PATCH v3 16/27] hw/loongarch: Add LoongArch cpu interrupt support(CPUINTC)

2021-12-20 Thread yangxiaojuan
Hi,Mark

On 12/18/2021 07:54 AM, Mark Cave-Ayland wrote:
> On 04/12/2021 12:07, Xiaojuan Yang wrote:
> 
>> Loongson-3A5000 support 14 interrupts from 64 - 77(Timer->75 IPI->76)
>> Loongson-3A5000 and ls7a form a legacy model and extended model irq
>> hierarchy.Tcg mode emulate a simplified extended model which
>> has no Legacy I/O Interrupt Controller(LIOINTC) and LPC.
>> e.g:
>>
>>   |+-++-+ +---+ |
>>   || IPI |--> | CPUINTC | <-- | Timer | |
>>   |+-++-+ +---+ |
>>   |^|
>>   |||
>>   |   +-+
>>   |   | EIOINTC |
>>   |   +-+
>>   |^   ^|
>>   ||   ||
>>   | +-+ +-+ |
>>   | | PCH-PIC | | PCH-MSI | |
>>   | +-+ +-+ |
>>   |   ^ ^   ^   |
>>   |   | |   |   |
>>   |   +-+ +-+ +-+   |
>>   |   | UARTs | | Devices | | Devices | |
>>   |   +-+ +-+ +-+   |
>>   |^|
>>
>> The following series patch will realize the interrupt
>> controller in this model.
>>
>> More detailed info can be found at the kernel doc or manual
>> 1.https://git.kernel.org/pub/scm/linux/kernel/git/chenhuacai/
>> linux-loongson.git/tree/Documentation/loongarch?h=loongarch-next
>> 2.https://github.com/loongson/LoongArch-Documentation
>>
>> Signed-off-by: Xiaojuan Yang 
>> Signed-off-by: Song Gao 
>> ---
>>   target/loongarch/cpu.c | 28 
>>   1 file changed, 28 insertions(+)
>>
>> diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c
>> index 62c2a4d813..afa550c950 100644
>> --- a/target/loongarch/cpu.c
>> +++ b/target/loongarch/cpu.c
>> @@ -504,11 +504,39 @@ static void loongarch_cpu_realizefn(DeviceState *dev, 
>> Error **errp)
>>   lacc->parent_realize(dev, errp);
>>   }
>>   +#ifndef CONFIG_USER_ONLY
>> +static void loongarch_cpu_set_irq(void *opaque, int irq, int level)
>> +{
>> +LoongArchCPU *cpu = opaque;
>> +CPULoongArchState *env = >env;
>> +CPUState *cs = CPU(cpu);
>> +
>> +if (irq < 0 || irq > N_IRQS) {
>> +return;
>> +}
>> +
>> +if (level) {
>> +env->CSR_ESTAT |= 1 << irq;
>> +} else {
>> +env->CSR_ESTAT &= ~(1 << irq);
>> +}
>> +
>> +if (FIELD_EX64(env->CSR_ESTAT, CSR_ESTAT, IS)) {
>> +cpu_interrupt(cs, CPU_INTERRUPT_HARD);
>> +} else {
>> +cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
>> +}
>> +}
>> +#endif
>> +
>>   static void loongarch_cpu_initfn(Object *obj)
>>   {
>>   LoongArchCPU *cpu = LOONGARCH_CPU(obj);
>> cpu_set_cpustate_pointers(cpu);
>> +#ifndef CONFIG_USER_ONLY
>> +qdev_init_gpio_in(DEVICE(cpu), loongarch_cpu_set_irq, N_IRQS);
>> +#endif
>>   }
>> static ObjectClass *loongarch_cpu_class_by_name(const char *cpu_model)
> 
> Rather than use defines to split out user mode, I would suggest using a 
> separate function in a similar way to sparc64_cpu_devinit() in 
> hw/sparc64/sparc64.c to set up the parts of the CPU that are only required in 
> system mode. This function can then be called as part of the board setup.
> 
yes, put the code to the board setup stage is more suitable, thank you!
> 
> ATB,
> 
> Mark.




Re: [RFC PATCH v3 14/27] hw/pci-host: Add ls7a1000 PCIe Host bridge support for Loongson3 Platform

2021-12-20 Thread yangxiaojuan
Hi, Mark

On 12/18/2021 07:39 AM, Mark Cave-Ayland wrote:
> On 04/12/2021 12:07, Xiaojuan Yang wrote:
> 
>> This is a model of the PCIe Host Bridge found on a Loongson-5000
>> processor. It includes a interrupt controller, some interface for
>> pci and nonpci devices. Mainly emulate part of it that is not
>> exactly the same as the host and only use part devices for
>> tcg mode. It support for MSI and MSIX interrupt sources.
>>
>> For more detailed info about ls7a1000 you can see the doc at
>> https://github.com/loongson/LoongArch-Documentation/releases/latest/
>> download/Loongson-7A1000-usermanual-2.00-EN.pdf
>>
>> Signed-off-by: Xiaojuan Yang 
>> Signed-off-by: Song Gao 
>> ---
>>   hw/pci-host/Kconfig|   4 +
>>   hw/pci-host/ls7a.c | 174 +
>>   hw/pci-host/meson.build|   1 +
>>   include/hw/pci-host/ls7a.h |  51 +++
>>   4 files changed, 230 insertions(+)
>>   create mode 100644 hw/pci-host/ls7a.c
>>   create mode 100644 include/hw/pci-host/ls7a.h
>>
>> diff --git a/hw/pci-host/Kconfig b/hw/pci-host/Kconfig
>> index 2b5f7d58cc..b02a9d1454 100644
>> --- a/hw/pci-host/Kconfig
>> +++ b/hw/pci-host/Kconfig
>> @@ -77,3 +77,7 @@ config MV64361
>>   bool
>>   select PCI
>>   select I8259
>> +
>> +config PCI_EXPRESS_7A
>> +bool
>> +select PCI_EXPRESS
>> diff --git a/hw/pci-host/ls7a.c b/hw/pci-host/ls7a.c
>> new file mode 100644
>> index 00..a783fb2eda
>> --- /dev/null
>> +++ b/hw/pci-host/ls7a.c
>> @@ -0,0 +1,174 @@
>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>> +/*
>> + * QEMU Loongson 7A1000 North Bridge Emulation
>> + *
>> + * Copyright (C) 2021 Loongson Technology Corporation Limited
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +
>> +#include "hw/pci/pci.h"
>> +#include "hw/pci/pcie_host.h"
>> +#include "hw/qdev-properties.h"
>> +#include "qapi/error.h"
>> +#include "hw/irq.h"
>> +#include "hw/pci/pci_bridge.h"
>> +#include "hw/pci/pci_bus.h"
>> +#include "sysemu/reset.h"
>> +#include "hw/pci-host/ls7a.h"
>> +#include "migration/vmstate.h"
>> +
>> +static const VMStateDescription vmstate_ls7a_pcie = {
>> +.name = "LS7A_PCIE",
>> +.version_id = 1,
>> +.minimum_version_id = 1,
>> +.fields = (VMStateField[]) {
>> +VMSTATE_PCI_DEVICE(parent_obj, LS7APCIState),
>> +VMSTATE_END_OF_LIST()
>> +}
>> +};
>> +
>> +static void pci_ls7a_config_write(void *opaque, hwaddr addr,
>> +  uint64_t val, unsigned size)
>> +{
>> +pci_data_write(opaque, addr, val, size);
>> +}
>> +
>> +static uint64_t pci_ls7a_config_read(void *opaque,
>> + hwaddr addr, unsigned size)
>> +{
>> +uint64_t val;
>> +
>> +val = pci_data_read(opaque, addr, size);
>> +
>> +return val;
>> +}
>> +
>> +static const MemoryRegionOps pci_ls7a_config_ops = {
>> +.read = pci_ls7a_config_read,
>> +.write = pci_ls7a_config_write,
>> +.valid = {
>> +.min_access_size = 1,
>> +.max_access_size = 4,
>> +},
>> +.impl = {
>> +.min_access_size = 1,
>> +.max_access_size = 4,
>> +},
>> +.endianness = DEVICE_LITTLE_ENDIAN,
>> +};
>> +
>> +static void ls7a_pciehost_realize(DeviceState *dev, Error **errp)
>> +{
>> +PCIHostState *pci = PCI_HOST_BRIDGE(dev);
>> +LS7APCIEHost *s = LS7A_HOST_DEVICE(dev);
>> +PCIExpressHost *pex = PCIE_HOST_BRIDGE(dev);
> 
> SysbusDevice *sbd = SYS_BUS_DEVICE(dev) will be needed for later use.
> 
>> +pci->bus = pci_register_root_bus(dev, "pcie.0", NULL, NULL, s,
>> + get_system_memory(), get_system_io(),
>> + PCI_DEVFN(1, 0), 128, TYPE_PCIE_BUS);
> 
> A device shouldn't map itself into an address space: that is the job of the 
> board. To achieve this LS7APCIEHost should have separate mmio and io memory 
> regions defined and pci_register_root_bus() configured to use these i.e.

OK, I get it and have modified all the region map. Thank you

> 
> pci->bus = pci_register_root_bus(dev, "pcie.0", NULL, NULL, s,
>  >pci_mmio, >pci_io,
>  PCI_DEVFN(1, 0), 128, TYPE_PCIE_BUS);
> 
>> +memory_region_init_io(>pci_conf, OBJECT(dev),
>> +  _ls7a_config_ops, pci->bus,
>> +  "ls7a_pci_conf", HT1LO_PCICFG_SIZE);
>> +memory_region_add_subregion(get_system_memory(), HT1LO_PCICFG_BASE,
>> +>pci_conf);
> 
> Here add sysbus_init_mmio(sbd, >pci_conf) and remove 
> memory_region_add_subregion().
> 
>> +/* Add ls7a pci-io */
>> +memory_region_init_alias(>pci_io, OBJECT(dev), "ls7a-pci-io",
>> + get_system_io(), 0, LS7A_PCI_IO_SIZE);
>> +memory_region_add_subregion(get_system_memory(), LS7A_PCI_IO_BASE,
>> +>pci_io);
> 
> Remove the alias onto the system io memory region and 

Re: [RFC PATCH v3 00/27] Add LoongArch softmmu support.

2021-12-13 Thread yangxiaojuan
thank you!

On 12/14/2021 06:43 AM, Mark Cave-Ayland wrote:
> On 13/12/2021 03:13, yangxiaojuan wrote:
> 
>> Ping!
>>
>> Please help review the V3 patch, thank you!
> 
> I've been fairly busy recently, but I will try and find some time to look at 
> the v3 sometime during the week.
> 
> 
> ATB,
> 
> Mark.




Re: [RFC PATCH v3 00/27] Add LoongArch softmmu support.

2021-12-12 Thread yangxiaojuan
Ping!

Please help review the V3 patch, thank you!

On 12/04/2021 08:06 PM, Xiaojuan Yang wrote:
> This series patch add softmmu support for LoongArch.
> Base on the linux-user emulation support V13 patch.
>   * 
> https://patchew.org/QEMU/1638610165-15036-1-git-send-email-gaos...@loongson.cn/
> The latest kernel:
>   * https://github.com/loongson/linux/tree/loongarch-next
> The manual:
>   * 
> https://github.com/loongson/LoongArch-Documentation/releases/tag/2021.10.11
> 
> Changes for v3:
> 1.Target code mainly follow Richard's code review comments.
> 2.Put the csr and iocsr read/write instruction emulate into 2 different patch.
> 3.Simply the tlb emulation.
> 4.Delete some unused csr registers defintion.
> 5.Machine and board code mainly follow Mark's advice, discard the obsolete 
> interface.
> 6.NUMA function is removed for it is not completed.
> 7.Adjust some format problem and the Naming problem 
> 
> Changes for v2:
> 1.Combine patch 2 and 3 into one.
> 2.Adjust the order of the patch.
> 3.Put all the binaries on the github.
> 4.Modify some emulate errors when use the kernel from the github.
> 5.Adjust some format problem and the Naming problem 
> 6.Others mainly follow Richard's code review comments.
> 
> Please help review!
> 
> Thanks
> 
> Xiaojuan Yang (27):
>   target/loongarch: Update README
>   target/loongarch: Add CSR registers definition
>   target/loongarch: Add basic vmstate description of CPU.
>   target/loongarch: Implement qmp_query_cpu_definitions()
>   target/loongarch: Add stabletimer support
>   target/loongarch: Add MMU support for LoongArch CPU.
>   target/loongarch: Add LoongArch CSR instruction
>   target/loongarch: Add LoongArch IOCSR instruction
>   target/loongarch: Add TLB instruction support
>   target/loongarch: Add other core instructions support
>   target/loongarch: Add LoongArch interrupt and exception handle
>   target/loongarch: Add timer related instructions support.
>   target/loongarch: Add gdb support.
>   hw/pci-host: Add ls7a1000 PCIe Host bridge support for Loongson3
> Platform
>   hw/loongarch: Add support loongson3-ls7a machine type.
>   hw/loongarch: Add LoongArch cpu interrupt support(CPUINTC)
>   hw/loongarch: Add LoongArch ipi interrupt support(IPI)
>   hw/intc: Add LoongArch ls7a interrupt controller support(PCH-PIC)
>   hw/intc: Add LoongArch ls7a msi interrupt controller support(PCH-MSI)
>   hw/intc: Add LoongArch extioi interrupt controller(EIOINTC)
>   hw/loongarch: Add irq hierarchy for the system
>   hw/loongarch: Add some devices support for 3A5000.
>   hw/loongarch: Add LoongArch ls7a rtc device support
>   hw/loongarch: Add default bios startup support.
>   hw/loongarch: Add -kernel and -initrd options support
>   hw/loongarch: Add LoongArch smbios support
>   hw/loongarch: Add LoongArch acpi support
> 
>  .../devices/loongarch64-softmmu/default.mak   |   3 +
>  configs/targets/loongarch64-softmmu.mak   |   4 +
>  gdb-xml/loongarch-base64.xml  |  43 +
>  gdb-xml/loongarch-fpu64.xml   |  57 ++
>  hw/Kconfig|   1 +
>  hw/acpi/Kconfig   |   4 +
>  hw/acpi/ls7a.c| 349 
>  hw/acpi/meson.build   |   1 +
>  hw/intc/Kconfig   |  15 +
>  hw/intc/loongarch_extioi.c| 499 +++
>  hw/intc/loongarch_ipi.c   | 162 
>  hw/intc/loongarch_pch_msi.c   |  67 ++
>  hw/intc/loongarch_pch_pic.c   | 357 
>  hw/intc/meson.build   |   4 +
>  hw/intc/trace-events  |  21 +
>  hw/loongarch/Kconfig  |  23 +
>  hw/loongarch/acpi-build.c | 637 ++
>  hw/loongarch/fw_cfg.c |  33 +
>  hw/loongarch/fw_cfg.h |  15 +
>  hw/loongarch/loongson3.c  | 509 +++
>  hw/loongarch/meson.build  |   6 +
>  hw/meson.build|   1 +
>  hw/pci-host/Kconfig   |   4 +
>  hw/pci-host/ls7a.c| 214 +
>  hw/pci-host/meson.build   |   1 +
>  hw/rtc/Kconfig|   3 +
>  hw/rtc/ls7a_rtc.c | 323 +++
>  hw/rtc/meson.build|   1 +
>  include/exec/poison.h |   2 +
>  include/hw/acpi/ls7a.h|  53 ++
>  include/hw/intc/loongarch_extioi.h|  69 ++
>  include/hw/intc/loongarch_ipi.h   |  47 ++
>  include/hw/intc/loongarch_pch_msi.h   |  21 +
>  include/hw/intc/loongarch_pch_pic.h   |  61 ++
>  include/hw/loongarch/loongarch.h  |  68 ++
>  include/hw/pci-host/ls7a.h|  79 ++
>  include/sysemu/arch_init.h|   1 

Re: [RFC PATCH v3 15/27] hw/loongarch: Add support loongson3-ls7a machine type.

2021-12-05 Thread yangxiaojuan
Hi, Huacai

On 12/06/2021 12:36 PM, chen huacai wrote:
> Hi, Xiaojuan,
> 
> On Sat, Dec 4, 2021 at 8:11 PM Xiaojuan Yang  wrote:
>>
>> Emulate a 3A5000 board use the new loongarch instruction.
>> 3A5000 belongs to the Loongson3 series processors.
>> The board consists of a 3A5000 cpu model and the 7A1000
>> bridge. The host 3A5000 board is really complicated and
>> contains many functions.Now for the tcg softmmu mode
>> only part functions are emulated.
>>
>> More detailed info you can see
>> https://github.com/loongson/LoongArch-Documentation
>>
>> Signed-off-by: Xiaojuan Yang 
>> Signed-off-by: Song Gao 
>> ---
>>  .../devices/loongarch64-softmmu/default.mak   |   3 +
>>  configs/targets/loongarch64-softmmu.mak   |   3 +
>>  hw/Kconfig|   1 +
>>  hw/loongarch/Kconfig  |   3 +
>>  hw/loongarch/loongson3.c  | 160 ++
>>  hw/loongarch/meson.build  |   4 +
>>  hw/meson.build|   1 +
>>  include/exec/poison.h |   2 +
>>  include/hw/loongarch/loongarch.h  |  48 ++
>>  include/sysemu/arch_init.h|   1 +
>>  qapi/machine.json |   2 +-
>>  target/Kconfig|   1 +
>>  target/loongarch/Kconfig  |   2 +
>>  target/loongarch/cpu.c|   8 +
>>  target/loongarch/cpu.h|   4 +
>>  15 files changed, 242 insertions(+), 1 deletion(-)
>>  create mode 100644 configs/devices/loongarch64-softmmu/default.mak
>>  create mode 100644 hw/loongarch/Kconfig
>>  create mode 100644 hw/loongarch/loongson3.c
>>  create mode 100644 hw/loongarch/meson.build
>>  create mode 100644 include/hw/loongarch/loongarch.h
>>  create mode 100644 target/loongarch/Kconfig
>>
>> diff --git a/configs/devices/loongarch64-softmmu/default.mak 
>> b/configs/devices/loongarch64-softmmu/default.mak
>> new file mode 100644
>> index 00..973ce4c30a
>> --- /dev/null
>> +++ b/configs/devices/loongarch64-softmmu/default.mak
>> @@ -0,0 +1,3 @@
>> +# Default configuration for loongarch64-softmmu
>> +
>> +CONFIG_LOONGSON3_LS7A=y
>> diff --git a/configs/targets/loongarch64-softmmu.mak 
>> b/configs/targets/loongarch64-softmmu.mak
>> index f33fa1590b..7bc06c850c 100644
>> --- a/configs/targets/loongarch64-softmmu.mak
>> +++ b/configs/targets/loongarch64-softmmu.mak
>> @@ -1 +1,4 @@
>> +TARGET_ARCH=loongarch64
>> +TARGET_BASE_ARCH=loongarch
>> +TARGET_SUPPORTS_MTTCG=y
>>  TARGET_XML_FILES= gdb-xml/loongarch-base64.xml gdb-xml/loongarch-fpu64.xml
>> diff --git a/hw/Kconfig b/hw/Kconfig
>> index ad20cce0a9..f71b2155ed 100644
>> --- a/hw/Kconfig
>> +++ b/hw/Kconfig
>> @@ -49,6 +49,7 @@ source avr/Kconfig
>>  source cris/Kconfig
>>  source hppa/Kconfig
>>  source i386/Kconfig
>> +source loongarch/Kconfig
>>  source m68k/Kconfig
>>  source microblaze/Kconfig
>>  source mips/Kconfig
>> diff --git a/hw/loongarch/Kconfig b/hw/loongarch/Kconfig
>> new file mode 100644
>> index 00..ae8498de6a
>> --- /dev/null
>> +++ b/hw/loongarch/Kconfig
>> @@ -0,0 +1,3 @@
>> +config LOONGSON3_LS7A
>> +bool
>> +select PCI_EXPRESS_7A
>> diff --git a/hw/loongarch/loongson3.c b/hw/loongarch/loongson3.c
>> new file mode 100644
>> index 00..28b623e927
>> --- /dev/null
>> +++ b/hw/loongarch/loongson3.c
>> @@ -0,0 +1,160 @@
>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>> +/*
>> + * QEMU loongson 3a5000 develop board emulation
>> + *
>> + * Copyright (c) 2021 Loongson Technology Corporation Limited
>> + */
>> +#include "qemu/osdep.h"
>> +#include "qemu-common.h"
>> +#include "qemu/units.h"
>> +#include "qemu/datadir.h"
>> +#include "qapi/error.h"
>> +#include "hw/boards.h"
>> +#include "sysemu/sysemu.h"
>> +#include "sysemu/qtest.h"
>> +#include "sysemu/runstate.h"
>> +#include "sysemu/reset.h"
>> +#include "hw/loongarch/loongarch.h"
>> +#include "hw/pci-host/ls7a.h"
>> +
>> +
>> +static void loongarch_cpu_reset(void *opaque)
>> +{
>> +LoongArchCPU *cpu = opaque;
>> +
>> +cpu_reset(CPU(cpu));
>> +}
>> +
>> +#define LOONGARCH_SIMPLE_MMIO_OPS(ADDR, NAME, SIZE) \
>> +({\
>> + MemoryRegion *iomem = g_new(MemoryRegion, 1);\
>> + memory_region_init_io(iomem, NULL, _qemu_ops,\
>> +   (void *)ADDR, NAME, SIZE);\
>> + memory_region_add_subregion(>system_iocsr, ADDR, iomem);\
>> +})
>> +
>> +static void loongarch_qemu_write(void *opaque, hwaddr addr,
>> + uint64_t val, unsigned size)
>> +{
>> +}
>> +
>> +static uint64_t loongarch_qemu_read(void *opaque, hwaddr addr, unsigned 
>> size)
>> +{
>> +uint64_t feature = 0UL;
>> +addr = ((hwaddr)(long)opaque) + addr;
>> +
>> +switch (addr) {
>> +case FEATURE_REG:
>> +feature |= 1UL << IOCSRF_MSI | 1UL << IOCSRF_EXTIOI |
>> +   1UL << IOCSRF_CSRIPI;
>> +return feature ;
>> +case 

Re: [RFC PATCH v3 22/27] hw/loongarch: Add some devices support for 3A5000.

2021-12-05 Thread yangxiaojuan
Hi,

On 12/05/2021 01:54 AM, Philippe Mathieu-Daudé wrote:
> On 12/4/21 13:07, Xiaojuan Yang wrote:
>> 1.Add uart,virtio-net,vga and usb for 3A5000.
>> 2.Add irq set and map for the pci host. Non pci device
>> use irq 0-16, pci device use 16-64.
>> 3.Add some unimplented device to emulate guest unused
>> memory space.
>>
>> Signed-off-by: Xiaojuan Yang 
>> Signed-off-by: Song Gao 
>> ---
>>  hw/loongarch/Kconfig|  8 +
>>  hw/loongarch/loongson3.c| 63 +++--
>>  hw/pci-host/ls7a.c  | 42 +-
>>  include/hw/intc/loongarch_ipi.h |  2 ++
>>  include/hw/pci-host/ls7a.h  |  4 +++
>>  softmmu/qdev-monitor.c  |  3 +-
>>  6 files changed, 117 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/loongarch/Kconfig b/hw/loongarch/Kconfig
>> index 468e3acc74..9ea3b92708 100644
>> --- a/hw/loongarch/Kconfig
>> +++ b/hw/loongarch/Kconfig
>> @@ -1,5 +1,13 @@
>>  config LOONGSON3_LS7A
>>  bool
>> +imply VGA_PCI
>> +imply VIRTIO_VGA
>> +imply PARALLEL
> 
> Is there really a parallel port? If so, maybe you forgot to
> instantiate it.
> 
There is no parallel port, I will fix it, thanks.




Re: [RFC PATCH v2 21/30] hw/intc: Add LoongArch extioi interrupt controller(EIOINTC)

2021-11-25 Thread yangxiaojuan
Hi Mark,

On 11/11/2021 10:49 PM, Mark Cave-Ayland wrote:
> On 11/11/2021 01:35, Xiaojuan Yang wrote:
> 
>> This patch realize the EIOINTC interrupt controller.
>>
>> Signed-off-by: Xiaojuan Yang 
>> Signed-off-by: Song Gao 
>> ---
>>   hw/intc/Kconfig|   3 +
>>   hw/intc/loongarch_extioi.c | 570 +
>>   hw/intc/meson.build|   1 +
>>   hw/loongarch/Kconfig   |   1 +
>>   include/hw/intc/loongarch_extioi.h |  99 +
>>   include/hw/loongarch/loongarch.h   |   1 +
>>   6 files changed, 675 insertions(+)
>>   create mode 100644 hw/intc/loongarch_extioi.c
>>   create mode 100644 include/hw/intc/loongarch_extioi.h
>>
>> diff --git a/hw/intc/Kconfig b/hw/intc/Kconfig
>> index c0dc12dfa0..a2d9efd5aa 100644
>> --- a/hw/intc/Kconfig
>> +++ b/hw/intc/Kconfig
>> @@ -82,3 +82,6 @@ config LOONGARCH_PCH_MSI
>>   select MSI_NONBROKEN
>>   bool
>>   select UNIMP
>> +
>> +config LOONGARCH_EXTIOI
>> +bool
>> diff --git a/hw/intc/loongarch_extioi.c b/hw/intc/loongarch_extioi.c
>> new file mode 100644
>> index 00..592cd8d1e2
>> --- /dev/null
>> +++ b/hw/intc/loongarch_extioi.c
>> @@ -0,0 +1,570 @@
>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>> +/*
>> + * Loongson 3A5000 ext interrupt controller emulation
>> + *
>> + * Copyright (C) 2021 Loongson Technology Corporation Limited
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "qemu/module.h"
>> +#include "qemu/log.h"
>> +#include "hw/irq.h"
>> +#include "hw/sysbus.h"
>> +#include "hw/loongarch/loongarch.h"
>> +#include "hw/qdev-properties.h"
>> +#include "exec/address-spaces.h"
>> +#include "hw/intc/loongarch_extioi.h"
>> +#include "migration/vmstate.h"
>> +
>> +#define DEBUG_APIC 0
>> +
>> +#define DPRINTF(fmt, ...) \
>> +do { \
>> +if (DEBUG_APIC) { \
>> +fprintf(stderr, "APIC: " fmt , ## __VA_ARGS__); \
>> +} \
>> +} while (0)
> 
> Again please use trace-events insead of DPRINTF().
> 
>> +static void extioi_update_irq(void *opaque, int irq_num, int level)
>> +{
>> +loongarch_extioi *s = opaque;
>> +uint8_t  ipnum, cpu;
>> +unsigned long found1, found2;
>> +
>> +ipnum = s->sw_ipmap[irq_num];
>> +cpu   = s->sw_coremap[irq_num];
>> +if (level == 1) {
>> +if (test_bit(irq_num, (void *)s->en_reg8) == false) {
>> +return;
>> +}
>> +bitmap_set((void *)s->coreisr_reg8[cpu], irq_num, 1);
>> +found1 = find_next_bit((void *)&(s->sw_ipisr[cpu][ipnum]),
>> +   EXTIOI_IRQS, 0);
>> +bitmap_set((void *)&(s->sw_ipisr[cpu][ipnum]), irq_num, 1);
>> +
>> +if (found1 >= EXTIOI_IRQS) {
>> +qemu_set_irq(s->parent_irq[cpu][ipnum], level);
>> +}
>> +} else {
>> +bitmap_clear((void *)s->coreisr_reg8[cpu], irq_num, 1);
>> +found1 = find_next_bit((void *)&(s->sw_ipisr[cpu][ipnum]),
>> +   EXTIOI_IRQS, 0);
>> +bitmap_clear((void *)&(s->sw_ipisr[cpu][ipnum]), irq_num, 1);
>> +found2 = find_next_bit((void *)&(s->sw_ipisr[cpu][ipnum]),
>> +   EXTIOI_IRQS, 0);
>> +
>> +if ((found1 < EXTIOI_IRQS) && (found2 >= EXTIOI_IRQS)) {
>> +qemu_set_irq(s->parent_irq[cpu][ipnum], level);
>> +}
>> +}
>> +}
>> +
>> +static void extioi_setirq(void *opaque, int irq, int level)
>> +{
>> +loongarch_extioi *s = opaque;
>> +extioi_update_irq(s, irq, level);
>> +}
>> +
>> +static void extioi_handler(void *opaque, int irq, int level)
>> +{
>> +loongarch_extioi *extioi = (loongarch_extioi *)opaque;
>> +
>> +qemu_set_irq(extioi->irq[irq], level);
>> +}
>> +
>> +static uint32_t extioi_readb(void *opaque, hwaddr addr)
>> +{
>> +loongarch_extioi *state = opaque;
> 
> Add a QOM cast here.
> 
>> +unsigned long offset, reg_count;
>> +uint8_t ret;
>> +int cpu;
>> +
>> +offset = addr & 0x;
>> +
>> +if ((offset >= EXTIOI_ENABLE_START) && (offset < EXTIOI_ENABLE_END)) {
>> +reg_count = (offset - EXTIOI_ENABLE_START);
>> +ret = state->en_reg8[reg_count];
>> +} else if ((offset >= EXTIOI_BOUNCE_START) &&
>> +   (offset < EXTIOI_BOUNCE_END)) {
>> +reg_count = (offset - EXTIOI_BOUNCE_START);
>> +ret = state->bounce_reg8[reg_count];
>> +} else if ((offset >= EXTIOI_COREISR_START) &&
>> +   (offset < EXTIOI_COREISR_END)) {
>> +reg_count = ((offset - EXTIOI_COREISR_START) & 0x1f);
>> +cpu = ((offset - EXTIOI_COREISR_START) >> 8) & 0x3;
>> +ret = state->coreisr_reg8[cpu][reg_count];
>> +} else if ((offset >= EXTIOI_IPMAP_START) &&
>> +   (offset < EXTIOI_IPMAP_END)) {
>> +reg_count = (offset - EXTIOI_IPMAP_START);
>> +ret = state->ipmap_reg8[reg_count];
>> +} else if ((offset >= EXTIOI_COREMAP_START) &&
>> +   (offset < EXTIOI_COREMAP_END)) {
>> +reg_count = (offset - 

Re: [RFC PATCH v2 09/30] target/loongarch: Add TLB instruction support

2021-11-17 Thread yangxiaojuan



On 11/17/2021 04:22 PM, Richard Henderson wrote:
> On 11/17/21 8:29 AM, yangxiaojuan wrote:
>> On 11/12/2021 02:14 AM, Richard Henderson wrote:
>>> On 11/11/21 2:35 AM, Xiaojuan Yang wrote:
>>>> +static bool trans_tlbwr(DisasContext *ctx, arg_tlbwr *a)
>>>> +{
>>>> +gen_helper_check_plv(cpu_env);
>>>> +gen_helper_tlbwr(cpu_env);
>>>> +tcg_gen_movi_tl(cpu_pc, ctx->base.pc_next + 4);
>>>> +ctx->base.is_jmp = DISAS_EXIT;
>>>> +return true;
>>>> +}
>>>
>>> I think you can skip the EXIT if paging is disabled, which it usually will 
>>> be in the software tlb handler.  You'd be able to tell with the mmu_idx 
>>> being the one you use for paging disabled.
>>
>> The paging disabled only enabled at the bios startup, we can get the phys 
>> address directly, tlbwr instruction will not be used when paging enabled.
> 
> Paging is also disabled during TLBRENTRY exception (see section 6.2.4 
> Hardware Exception Handling of TLB Refil Exception).  It is this routine that 
> will usually use tlbwr most often (although the kernel at PRV 0 is not 
> prevented from doing so).

Sorry, I forgot this situation.

> 
>>>> +default:
>>>> +do_raise_exception(env, EXCP_INE, GETPC());
>>>
>>> You can detect this during translation, and dispatch to the appropriate 
>>> invtlb sub-function.
>>>
>> oh, sorry, I don't quiet understand this. detect during the translation sees 
>> more complicated.
> 
> It is not more complex at all.  Less complex, I would say.
> 
> static bool trans_invtlb(DisasContext *ctx, arg_invtlb *a)
> {
> TCGv rj = gpr_src(ctx, a->rj, EXT_NONE);
> TCGv rk = gpr_src(ctx, a->rk, EXT_NONE);
> 
> if (check_plv(ctx)) {
> return false;
> }
> 
> switch (a->invop) {
> case 0:
> case 1:
> gen_helper_invtlb_all(cpu_env);
> break;
> case 2:
> gen_helper_invtlb_all_g(cpu_env, tcg_constant_i32(1));
> break;
> case 3:
> gen_helper_invtlb_all_g(cpu_env, tcg_constant_i32(0));
> break;
> case 4:
> gen_helper_invtlb_all_asid(cpu_env, rj);
> break;
> case 5:
> gen_helper_invtlb_page_asid(cpu_env, rj, rk);
> break;
> case 6:
> gen_helper_invtlb_page_asid_or_g(cpu_env, rj, rk);
> break;
> default:
> return false;
> }
> ctx->base.is_jmp = DISAS_STOP;
> return true;
> }
> 
Thank you. I get it.
> 
> r~




Re: [RFC PATCH v2 08/30] target/loongarch: Add LoongArch CSR/IOCSR instruction

2021-11-17 Thread yangxiaojuan
Hi, Richard:

On 11/12/2021 01:43 AM, Richard Henderson wrote:
> On 11/11/21 2:35 AM, Xiaojuan Yang wrote:
>> This includes:
>> - CSRRD
>> - CSRWR
>> - CSRXCHG
>> - IOCSR{RD/WR}.{B/H/W/D}
> 
> I think IOCSR should be in a separate patch.
> It's completely unrelated to the other CSRs.
> 
>> +target_ulong helper_csr_rdq(CPULoongArchState *env, uint64_t csr)
>> +{
>> +int64_t v;
>> +
>> +switch (csr) {
>> +case LOONGARCH_CSR_PGD:
>> +if (env->CSR_TLBRERA & 0x1) {
>> +v = env->CSR_TLBRBADV;
>> +} else {
>> +v = env->CSR_BADV;
>> +}
>> +
>> +if ((v >> 63) & 0x1) {
>> +v = env->CSR_PGDH;
>> +} else {
>> +v = env->CSR_PGDL;
>> +}
>> +v = v & TARGET_PHYS_MASK;
> 
> This csr is defined to be GRLEN bits; I this mask looks wrong.
> 
>> +default:
>> +assert(0);
> 
> g_assert_not_reached.
> 
>> +switch (csr) {
>> +case LOONGARCH_CSR_ASID:
>> +old_v = env->CSR_ASID;
>> +env->CSR_ASID = val;
> 
> Mask the write to the field; you don't want to corrupt ASIDBITS, or the other 
> read-only bits.

Ok, all above have been modified.

> 
>> +case LOONGARCH_CSR_TCFG:
>> +old_v = env->CSR_TCFG;
>> +cpu_loongarch_store_stable_timer_config(env, val);
>> +break;
>> +case LOONGARCH_CSR_TINTCLR:
>> +old_v = 0;
>> +qemu_irq_lower(env->irq[IRQ_TIMER]);
> 
> The interrupt is not documented to clear on any write; only writes of 1 to 
> bit 0.

I think the manual has mentioned at 7.6.5 which says when 1 is written to this 
bit, the clock interrupt 
flag is cleared.

> 
>> +default:
>> +assert(0);
> 
> g_assert_not_reached.
> 
>> +}
>> +
>> +return old_v;
>> +}
>> +
>> +target_ulong helper_csr_xchgq(CPULoongArchState *env, target_ulong val,
>> +  target_ulong mask, uint64_t csr)
>> +{
>> +target_ulong tmp;
>> +target_ulong v = val & mask;
> 
> I think it would be less confusing to name the input parameter new_val, and 
> the local temporary old_val.
> 
>> +#define CASE_CSR_XCHGQ(csr) \
>> +case LOONGARCH_CSR_ ## csr: \
>> +{   \
>> +val = env->CSR_ ## csr; \
>> +env->CSR_ ## csr = (env->CSR_ ## csr) & (~mask);\
>> +env->CSR_ ## csr = (env->CSR_ ## csr) | v;  \
> 
>   old_val = env->CSR_##csr;
>   env->CSR_##csr = (old_val & ~mask) | (new_val & mask);
> 
> 
>> +switch (csr) {
>> +CASE_CSR_XCHGQ(CRMD)
> 
> I wonder if all of this would be better with a table of offsets, which could 
> be shared with the translator.
> 
> #define CSR_OFF(X)  [LOONGARCH_CSR_##X] = offsetof(CPUArchState, CSR_##X)
> 
> static const int csr_offsets[] = {
> CSR_OFF(CRMD),
> ...
> };
> 
> int cpu_csr_offset(unsigned csr_num)
> {
> if (csr_num < ARRAY_SIZE(csr_offsets)) {
> return csr_offsets[csr_num];
> }
> return 0;
> }
> 
> Which reduces this function to
> 
> unsigned csr_offset = cpu_csr_offset(csr_num);
> if (csr_offset == 0) {
> /* CSR is undefined: read as 0, write ignored. */
> return 0;
> }
> 
> uint64_t *csr = (void *)env + csr_offset;
> uint64_t old_val = *csr;
> 
> new_val = (new_val & mask) | (old_val & ~mask);
> 
> *csr = (old_val & ~mask) | (new_val & mask);
> 
> if (csr_num == LOONGARCH_CSR_TCFG) {
> cpu_loongarch_store_stable_timer_config(env, new_val);
> } else {
> *csr = new_val;
> }
> return old_val;
> 
>> +uint64_t helper_iocsr_read(CPULoongArchState *env, target_ulong r_addr,
>> +   uint32_t size)
>> +{
>> +LoongArchMachineState *lams = LOONGARCH_MACHINE(qdev_get_machine());
>> +int cpuid = env_cpu(env)->cpu_index;
>> +
>> +if (((r_addr & 0xff00) == 0x1000) || ((r_addr & 0xff00) == 0x1800)) {
>> +r_addr = r_addr + ((target_ulong)(cpuid & 0x3) << 8);
>> +}
> 
> This looks to be something that should be controlled by the address space 
> assigned to each cpu.
> 
>   But it's hard to tell.
> 
> Where is the documentation for this?  I didn't immediately find it in 3A5000 
> Technical Reference Manual, Chapter 10.

Yes, most iocsr instructions introduced on 3A5000 Technical Reference Manual, 
Chapter 10. 
Table 10-2, 10-3, 10-4, 10-5 and 11-10 lists per core iocsr

> 
>> +void helper_iocsr_write(CPULoongArchState *env, target_ulong w_addr,
>> +target_ulong val, uint32_t size)
>> +{
>> +LoongArchMachineState *lams = LOONGARCH_MACHINE(qdev_get_machine());
>> +int cpuid = env_cpu(env)->cpu_index;
>> +int mask, i;
>> +
>> +/*
>> + * For IPI send, Mail send, ANY send adjust addr and val
>> + * according to their real meaning
>> + */
>> +if (w_addr == 0x1040) { /* IPI send */
>> +cpuid = (val >> 16) & 

Re: [RFC PATCH v2 09/30] target/loongarch: Add TLB instruction support

2021-11-16 Thread yangxiaojuan
Hi, Richard:

On 11/12/2021 02:14 AM, Richard Henderson wrote:
> On 11/11/21 2:35 AM, Xiaojuan Yang wrote:
>> +static bool trans_tlbwr(DisasContext *ctx, arg_tlbwr *a)
>> +{
>> +gen_helper_check_plv(cpu_env);
>> +gen_helper_tlbwr(cpu_env);
>> +tcg_gen_movi_tl(cpu_pc, ctx->base.pc_next + 4);
>> +ctx->base.is_jmp = DISAS_EXIT;
>> +return true;
>> +}
> 
> I think you can skip the EXIT if paging is disabled, which it usually will be 
> in the software tlb handler.  You'd be able to tell with the mmu_idx being 
> the one you use for paging disabled.

The paging disabled only enabled at the bios startup, we can get the phys 
address directly, tlbwr instruction will not be used when paging enabled.

> 
>> +static void loongarch_invalidate_tlb_entry(CPULoongArchState *env,
>> +   loongarch_tlb *tlb)
>> +{
>> +CPUState *cs = env_cpu(env);
>> +target_ulong addr, end, mask;
>> +int tlb_v0, tlb_v1;
>> +uint64_t tlb_vppn;
>> +uint8_t tlb_ps;
>> +
>> +tlb_v0 = FIELD_EX64(tlb->tlb_entry0, ENTRY0, V);
>> +tlb_v1 = FIELD_EX64(tlb->tlb_entry1, ENTRY1, V);
>> +tlb_vppn = FIELD_EX64(tlb->tlb_misc, TLB_MISC, VPPN);
>> +tlb_ps = FIELD_EX64(tlb->tlb_misc, TLB_MISC, PS);
>> +mask = (1 << (1 + tlb_ps)) - 1;
> 
> MAKE_64BIT_MASK.
> 
>> +
>> +if (tlb_v0) {
>> +addr = tlb_vppn & ~mask;/* xxx...xxx[0]000.. */
>> +end = addr | (mask >> 1);   /* xxx...xxx[0]111.. */
>> +while (addr < end) {
>> +tlb_flush_page(cs, addr);
>> +addr += TARGET_PAGE_SIZE;
> 
> tlb_flush_range_by_mmuidx.
> 
>> +tlb->tlb_misc = FIELD_DP64(tlb->tlb_misc, TLB_MISC, VPPN, csr_vppn);
>> +tlb->tlb_misc = FIELD_DP64(tlb->tlb_misc, TLB_MISC, E, 1);
>> +csr_asid = FIELD_EX64(env->CSR_ASID, CSR_ASID, ASID);
>> +tlb->tlb_misc = FIELD_DP64(tlb->tlb_misc, TLB_MISC, ASID, csr_asid);
>> +
>> +csr_g = FIELD_EX64(env->CSR_TLBELO0, CSR_TLBELO0, G) &
>> + FIELD_EX64(env->CSR_TLBELO1, CSR_TLBELO1, G);
>> +tlb->tlb_misc = FIELD_DP64(tlb->tlb_misc, TLB_MISC, G, csr_g);
>> +
>> +tlb->tlb_entry0 = FIELD_DP64(tlb->tlb_entry0, ENTRY0, V,
>> + FIELD_EX64(lo0, CSR_TLBELO0, V));/* [0] */
>> +tlb->tlb_entry0 = FIELD_DP64(tlb->tlb_entry0, ENTRY0, D,
>> + FIELD_EX64(lo0, CSR_TLBELO0, D));/* [1] */
>> +tlb->tlb_entry0 = FIELD_DP64(tlb->tlb_entry0, ENTRY0, PLV,
>> + FIELD_EX64(lo0, CSR_TLBELO0, PLV));/* 
>> [3:2] */
>> +tlb->tlb_entry0 = FIELD_DP64(tlb->tlb_entry0, ENTRY0, MAT,
>> + FIELD_EX64(lo0, CSR_TLBELO0, MAT));/* 
>> [5:4] */
>> +tlb->tlb_entry0 = FIELD_DP64(tlb->tlb_entry0, ENTRY0, PPN,
>> + FIELD_EX64(lo0, CSR_TLBELO0, PPN));/* 
>> [47:12] */
>> +tlb->tlb_entry0 = FIELD_DP64(tlb->tlb_entry0, ENTRY0, NR,
>> + FIELD_EX64(lo0, CSR_TLBELO0, NR));/* [61] 
>> */
>> +tlb->tlb_entry0 = FIELD_DP64(tlb->tlb_entry0, ENTRY0, NX,
>> + FIELD_EX64(lo0, CSR_TLBELO0, NX));/* [62] 
>> */
>> +tlb->tlb_entry0 = FIELD_DP64(tlb->tlb_entry0, ENTRY0, RPLV,
>> + FIELD_EX64(lo0, CSR_TLBELO0, RPLV));/* 
>> [63] */
>> +
>> +tlb->tlb_entry1 = FIELD_DP64(tlb->tlb_entry1, ENTRY1, V,
>> + FIELD_EX64(lo1, CSR_TLBELO1, V));/* [0] */
>> +tlb->tlb_entry1 = FIELD_DP64(tlb->tlb_entry1, ENTRY1, D,
>> + FIELD_EX64(lo1, CSR_TLBELO1, D));/* [1] */
>> +tlb->tlb_entry1 = FIELD_DP64(tlb->tlb_entry1, ENTRY1, PLV,
>> + FIELD_EX64(lo1, CSR_TLBELO1, PLV));/* 
>> [3:2] */
>> +tlb->tlb_entry1 = FIELD_DP64(tlb->tlb_entry1, ENTRY1, MAT,
>> + FIELD_EX64(lo1, CSR_TLBELO1, MAT));/* 
>> [5:4] */
>> +tlb->tlb_entry1 = FIELD_DP64(tlb->tlb_entry1, ENTRY1, PPN,
>> + FIELD_EX64(lo1, CSR_TLBELO1, PPN));/* 
>> [47:12] */
>> +tlb->tlb_entry1 = FIELD_DP64(tlb->tlb_entry1, ENTRY1, NR,
>> + FIELD_EX64(lo1, CSR_TLBELO1, NR));/* [61] 
>> */
>> +tlb->tlb_entry1 = FIELD_DP64(tlb->tlb_entry1, ENTRY1, NX,
>> + FIELD_EX64(lo1, CSR_TLBELO1, NX));/* [62] 
>> */
>> +tlb->tlb_entry1 = FIELD_DP64(tlb->tlb_entry1, ENTRY1, RPLV,
>> + FIELD_EX64(lo1, CSR_TLBELO1, RPLV));/* 
>> [63] */
> 
> The point of making the two values have the same field layout is so that you 
> can just assign the whole value across, not extract and re-deposit each field.

Yes, it is much simpler when use the same field layout.

> 
>> +void helper_tlbsrch(CPULoongArchState *env)
>> +{
>> +loongarch_tlb *tlb;
>> +uint64_t vpn, tlb_vppn;
>> +uint16_t csr_asid, tlb_asid, tlb_ps, tlb_e, tlb_g;
>> +
>> +int stlb_size = 

Re: [RFC PATCH v2 07/30] target/loongarch: Add MMU support for LoongArch CPU.

2021-11-16 Thread yangxiaojuan
Hi, Richard:

On 11/11/2021 11:53 PM, Richard Henderson wrote:
> On 11/11/21 2:35 AM, Xiaojuan Yang wrote:
>> This patch introduces basic TLB interfaces.
>>
>> Signed-off-by: Xiaojuan Yang 
>> Signed-off-by: Song Gao 
>> ---
>>   target/loongarch/cpu-param.h  |   3 +
>>   target/loongarch/cpu.c|  36 
>>   target/loongarch/cpu.h|  57 ++
>>   target/loongarch/internals.h  |   7 +
>>   target/loongarch/machine.c|  56 ++
>>   target/loongarch/meson.build  |   1 +
>>   target/loongarch/tlb_helper.c | 339 ++
>>   7 files changed, 499 insertions(+)
>>   create mode 100644 target/loongarch/tlb_helper.c
>>
>> diff --git a/target/loongarch/cpu-param.h b/target/loongarch/cpu-param.h
>> index 9a769b67e0..5a2147fb90 100644
>> --- a/target/loongarch/cpu-param.h
>> +++ b/target/loongarch/cpu-param.h
>> @@ -12,6 +12,9 @@
>>   #define TARGET_PHYS_ADDR_SPACE_BITS 48
>>   #define TARGET_VIRT_ADDR_SPACE_BITS 48
>>   +#define TARGET_PHYS_MASK ((1UL << TARGET_PHYS_ADDR_SPACE_BITS) - 1)
>> +#define TARGET_VIRT_MASK ((1UL << TARGET_VIRT_ADDR_SPACE_BITS) - 1)
> 
> As before, unsigned long is wrong; use MAKE_64BIT_MASK.
> 
> These do not belong in cpu-param.h anyway; probably only tlb_helper.c needs 
> them.
> 
>> +#ifndef CONFIG_USER_ONLY
>> +qemu_fprintf(f, "EUEN0x%lx\n", env->CSR_EUEN);
>> +qemu_fprintf(f, "ESTAT   0x%lx\n", env->CSR_ESTAT);
>> +qemu_fprintf(f, "ERA 0x%lx\n", env->CSR_ERA);
>> +qemu_fprintf(f, "CRMD0x%lx\n", env->CSR_CRMD);
>> +qemu_fprintf(f, "PRMD0x%lx\n", env->CSR_PRMD);
>> +qemu_fprintf(f, "BadVAddr0x%lx\n", env->CSR_BADV);
>> +qemu_fprintf(f, "TLB refill ERA  0x%lx\n", env->CSR_TLBRERA);
>> +qemu_fprintf(f, "TLB refill BadV 0x%lx\n", env->CSR_TLBRBADV);
>> +qemu_fprintf(f, "EENTRY  0x%lx\n", env->CSR_EENTRY);
>> +qemu_fprintf(f, "BadInstr0x%lx\n", env->CSR_BADI);
>> +qemu_fprintf(f, "PRCFG10x%lx\nPRCFG2 0x%lx\nPRCFG3 0x%lx\n",
>> + env->CSR_PRCFG1, env->CSR_PRCFG3, env->CSR_PRCFG3);
> 
> %lx is wrong; use PRIx64.
> 
>> +#define LOONGARCH_TLB_MAX  2112 /* 2048 STLB + 64 MTLB */
> 
> Better to write (2048 + 64).
> 
>> +FIELD(TLB_MISC, E, 0, 1)
>> +FIELD(TLB_MISC, ASID, 1, 10)
>> +FIELD(TLB_MISC, G, 11, 1)
>> +FIELD(TLB_MISC, PS, 12, 6)
>> +FIELD(TLB_MISC, VPPN, 18, 35)
>> +
>> +/* Corresponding to CSR_TLBELO0/1 */
>> +FIELD(ENTRY0, V, 0, 1)
>> +FIELD(ENTRY0, D, 1, 1)
>> +FIELD(ENTRY0, NR, 2, 1)
>> +FIELD(ENTRY0, NX, 3, 1)
>> +FIELD(ENTRY0, MAT, 4, 2)
>> +FIELD(ENTRY0, PLV, 6, 2)
>> +FIELD(ENTRY0, RPLV, 8, 1)
>> +FIELD(ENTRY0, PPN, 9, 36)
>> +
>> +FIELD(ENTRY1, V, 0, 1)
>> +FIELD(ENTRY1, D, 1, 1)
>> +FIELD(ENTRY1, NR, 2, 1)
>> +FIELD(ENTRY1, NX, 3, 1)
>> +FIELD(ENTRY1, MAT, 4, 2)
>> +FIELD(ENTRY1, PLV, 6, 2)
>> +FIELD(ENTRY1, RPLV, 8, 1)
>> +FIELD(ENTRY1, PPN, 9, 36)
> 
> Why are you duplicating the CSR_TLBELO* fields?
> 
>> +const VMStateInfo vmstate_info_tlb = {
>> +.name = "tlb_entry",
>> +.get  = get_tlb,
>> +.put  = put_tlb,
>> +};
> 
> Better to use .fields.
> 
>> +#define VMSTATE_TLB_ARRAY_V(_f, _s, _n, _v) \
>> +VMSTATE_ARRAY(_f, _s, _n, _v, vmstate_info_tlb, loongarch_tlb)
>> +
>> +#define VMSTATE_TLB_ARRAY(_f, _s, _n)   \
>> +VMSTATE_TLB_ARRAY_V(_f, _s, _n, 0)
> 
> Don't need these.
> 
>> +
>> +const VMStateDescription vmstate_tlb = {
>> +.name = "cpu/tlb",
>> +.version_id = 0,
>> +.minimum_version_id = 0,
>> +.fields = (VMStateField[]) {
>> +VMSTATE_TLB_ARRAY(env.tlb, LoongArchCPU, LOONGARCH_TLB_MAX),
> 
> VMSTATE_STRUCT_ARRAY.
> 
>> +VMSTATE_END_OF_LIST()
>> +}
>> +};
>> /* LoongArch CPU state */
>>   @@ -22,6 +70,10 @@ const VMStateDescription vmstate_loongarch_cpu = {
>>   VMSTATE_UINT64_ARRAY(env.fpr, LoongArchCPU, 32),
>>   VMSTATE_UINT32(env.fcsr0, LoongArchCPU),
>>   +/* TLB */
>> +VMSTATE_UINT32(env.stlb_size, LoongArchCPU),
>> +VMSTATE_UINT32(env.mtlb_size, LoongArchCPU),
> 
> Might as well keep these in vmstate_tlb.

All of the vmstate has been modified.

>> +/* TLB address map */
>> +static int loongarch_map_tlb_entry(CPULoongArchState *env, hwaddr *physical,
>> +   int *prot, target_ulong address,
>> +   int access_type, loongarch_tlb *tlb)
>> +{
>> +uint64_t plv = FIELD_EX64(env->CSR_CRMD, CSR_CRMD, PLV);
> 
> Incorrect.  PLV associated with mmu_idx, so you need to use that.
> 
>> +uint8_t tlb_ps, n, tlb_v0, tlb_v1, tlb_d0, tlb_d1;
>> +uint8_t tlb_nx0, tlb_nx1, tlb_nr0, tlb_nr1;
>> +uint64_t tlb_ppn0, tlb_ppn1;
>> +uint8_t tlb_rplv0, tlb_rplv1, tlb_plv0, tlb_plv1;
>> +
>> +tlb_ps = FIELD_EX64(tlb->tlb_misc, TLB_MISC, PS);
>> +n = (address >> tlb_ps) & 0x1;/* Odd or even */
> 
> Surely you need to pass in tlb_ps, since it's not present in 

Re: [RFC PATCH v2 01/30] target/loongarch: Update README

2021-11-14 Thread yangxiaojuan



On 11/11/2021 07:50 PM, chen huacai wrote:
> Hi, Xiaojuan,
> 
> On Thu, Nov 11, 2021 at 9:41 AM Xiaojuan Yang  
> wrote:
>>
>> Mainly introduce how to run the softmmu
>>
>> Signed-off-by: Xiaojuan Yang 
>> Signed-off-by: Song Gao 
>> ---
>>  target/loongarch/README | 20 
>>  1 file changed, 20 insertions(+)
>>
>> diff --git a/target/loongarch/README b/target/loongarch/README
>> index 09f809cf80..6f64bde22f 100644
>> --- a/target/loongarch/README
>> +++ b/target/loongarch/README
>> @@ -71,6 +71,26 @@
>>./qemu-loongarch64  /opt/clfs/usr/bin/pwd
>>...
>>
>> +- Softmmu emulation
>> +
>> +  Add support softmmu emulation support in the following series patches.
>> +  Mainly emulate a virt 3A5000 board that is not exactly the same as the 
>> host.
>> +  Kernel code is on the github and the uefi code will be opened in the near 
>> future.
>> +  All required binaries can get from github for test.
>> +
>> +  1.Download kernel and the cross-tools.(vmlinux)
>> +
>> +  wget https://github.com/loongson/linux
> This is a git repo URL, I think we cannot use wget to download.

oh, sorry, I will modify.

> 
>> +  wget 
>> https://github.com/loongson/build-tools/releases/latest/download/loongarch64-clfs-20210831-cross-tools.tar.xz
>> +
>> +  2.Download the clfs-system and made a ramdisk with busybox.(ramdisk)
>> +
>> +  3.Run with command,eg:
>> +
>> +   ./build/qemu-system-loongarch64 -m 4G -smp 16 --cpu Loongson-3A5000 
>> --machine loongson7a -kernel ./vmlinux -initrd ./ramdisk  -append 
>> "root=/dev/ram console=ttyS0,115200 rdinit=/sbin/init loglevel=8" -monitor 
>> tcp::4000,server,nowait -nographic
> It isn't recommended to use "loongson7a" as the machine name. In my
> opinion, we will have two types of machines run in qemu (One is an
> emulated LS7A and the other is pure virtual). I think we can call them
> "loongson3-ls7a" and "loongson3-virt".
> 
> Huacai

Thank you for your advice,  I didn't think comprehensively. I will modify in 
the next versison.

Thanks,
Xiaojuan
 
>> +
>> +The vmlinux, ramdisk and uefi binary loongarch_bios.bin can get from :
>> +git clone https://github.com/yangxiaojuan-loongson/qemu-binary
>>
>>  - Note.
>>We can get the latest LoongArch documents or LoongArch tools at 
>> https://github.com/loongson/
>> --
>> 2.27.0
>>
>>
> 
> 




Re: [RFC PATCH v2 04/30] target/loongarch: Define exceptions for LoongArch.

2021-11-11 Thread yangxiaojuan
Hi Richard,

On 11/11/2021 09:36 PM, Richard Henderson wrote:
> On 11/11/21 2:35 AM, Xiaojuan Yang wrote:
>> +++ b/target/loongarch/cpu.h
>> @@ -369,8 +369,21 @@ enum {
>>   EXCP_BREAK,
>>   EXCP_INE,
>>   EXCP_FPE,
>> -
>> -EXCP_LAST = EXCP_FPE,
>> +EXCP_IPE,
>> +EXCP_TLBL,
>> +EXCP_TLBS,
>> +EXCP_INST_NOTAVAIL,
>> +EXCP_TLBM,
>> +EXCP_TLBPE,
>> +EXCP_TLBNX,
>> +EXCP_TLBNR,
>> +EXCP_EXT_INTERRUPT,
>> +EXCP_DBP,
>> +EXCP_IBE,
>> +EXCP_DBE,
>> +EXCP_DINT,
>> +
>> +EXCP_LAST = EXCP_DINT,
> 
> Surely this is (essentially) a duplicate of EXCCODE_*.
> I think we should have only one copy of this.
> 

OK, I will use EXCCODE_* directly. Thanks

Xiaojuan

> 
> r~




Re: [RFC PATCH v2 02/30] target/loongarch: Add CSR registers definition

2021-11-11 Thread yangxiaojuan
Hi, Richard,

On 11/11/2021 09:29 PM, Richard Henderson wrote:
> On 11/11/21 2:35 AM, Xiaojuan Yang wrote:
>> +#define LOONGARCH_CSR_MISC   0x3 /* Misc config */
>> +
> 
> Missing bitfield definitions for misc.
> 

 At present, there is no read/write to each field of misc register so it is not 
defined.

>> +#define  EXCODE_IP   64
> 
> What's this?
> 

The manual 6.1.3 says the exception number of an interrupt is the exception 
number plus an offset of 64.
This defines the offset, sorry, the name is bad, maybe change a name or just 
use 64 directly in the
exception handle is better.

>> +#define  EXCCODE_INT 0
>> +#define  EXCCODE_PIL 1
>> +#define  EXCCODE_PIS 2
>> +#define  EXCCODE_PIF 3
>> +#define  EXCCODE_PME 4
>> +#define  EXCCODE_PNR 5
>> +#define  EXCCODE_PNX 6
>> +#define  EXCCODE_PPI 7
>> +#define  EXCCODE_ADE 8
> 
> ADEF vs ADEM?
> 
Yes, ADEF and ADEM have different esubcode, maybe I should add a description.

>> +#define  EXCCODE_ALE 9
>> +#define  EXCCODE_BCE 10
>> +#define  EXCCODE_SYS 11
>> +#define  EXCCODE_BRK 12
>> +#define  EXCCODE_INE 13
>> +#define  EXCCODE_IPE 14
>> +#define  EXCCODE_FPD 15
>> +#define  EXCCODE_SXD 16
>> +#define  EXCCODE_ASXD17
>> +#define  EXCCODE_FPE 18 /* Have different expsubcode */
>> +#define  EXCCODE_VFPE18
>> +#define  EXCCODE_WPEF19 /* Have different expsubcode */
>> +#define  EXCCODE_WPEM19
>> +#define  EXCCODE_BTD 20
>> +#define  EXCCODE_BTE 21
> 
> Missing BSPR, HVC, GCSC, GCHC.
> 

 At present, these exceptions are not triggered, so they are not defined.
 They are used by kvm. 

>> +#define LOONGARCH_CSR_ERA0x6 /* ERA */
> 
> Not really helpful to name the acronym with the acronym.
> "Exception return address".
> 

OK, I will use a more detailed description for all the registers.

>> +#define LOONGARCH_CSR_TLBELO00x12 /* TLB EntryLo0 */
>> +FIELD(CSR_TLBELO0, V, 0, 1)
>> +FIELD(CSR_TLBELO0, D, 1, 1)
>> +FIELD(CSR_TLBELO0, PLV, 2, 2)
>> +FIELD(CSR_TLBELO0, MAT, 4, 2)
>> +FIELD(CSR_TLBELO0, G, 6, 1)
>> +FIELD(CSR_TLBELO0, PPN, 12, 36)
>> +FIELD(CSR_TLBELO0, NR, 61, 1)
>> +FIELD(CSR_TLBELO0, NX, 62, 1)
>> +FIELD(CSR_TLBELO0, RPLV, 63, 1)
>> +
>> +#define LOONGARCH_CSR_TLBELO10x13 /* 64 TLB EntryLo1 */
>> +FIELD(CSR_TLBELO1, V, 0, 1)
>> +FIELD(CSR_TLBELO1, D, 1, 1)
>> +FIELD(CSR_TLBELO1, PLV, 2, 2)
>> +FIELD(CSR_TLBELO1, MAT, 4, 2)
>> +FIELD(CSR_TLBELO1, G, 6, 1)
>> +FIELD(CSR_TLBELO1, PPN, 12, 36)
>> +FIELD(CSR_TLBELO1, NR, 61, 1)
>> +FIELD(CSR_TLBELO1, NX, 62, 1)
>> +FIELD(CSR_TLBELO1, RPLV, 63, 1)
> 
> Better to define the fields once, dropping the 0/1 from the name.
OK, I will modify.
> 
>> +#define LOONGARCH_CSR_PWCL   0x1c /* PWCl */
> 
> "Page walk controller, low addr"
> 
>> +#define LOONGARCH_CSR_PWCH   0x1d /* PWCh */
> 
> "Page walk controller, high addr"
> 
>> +#define LOONGARCH_CSR_STLBPS 0x1e /* 64 */
> 
> 64?  "STLB Page size".
> 
>> +#define LOONGARCH_CSR_RVACFG 0x1f
> 
> "Reduced virtual address config"
> 
>> +/* Save registers */
>> +#define LOONGARCH_CSR_SAVE00x30
>> +#define LOONGARCH_CSR_SAVE10x31
>> +#define LOONGARCH_CSR_SAVE20x32
>> +#define LOONGARCH_CSR_SAVE30x33
>> +#define LOONGARCH_CSR_SAVE40x34
>> +#define LOONGARCH_CSR_SAVE50x35
>> +#define LOONGARCH_CSR_SAVE60x36
>> +#define LOONGARCH_CSR_SAVE70x37
> 
> Might as well must define SAVE0, and comment that the count is in 
> PRCFG1.SAVE_NUM.
> 
>> +#define  CSR_DMW_BASE_SH 48
> 
> What's this?  It looks like you should be using TARGET_VIRT_ADDR_SPACE_BITS 
> anyway.
Yes, TARGET_VIRT_ADDR_SPACE_BITS is better.
> 
>> +#define dmwin_va2pa(va) \
>> +(va & (((unsigned long)1 << CSR_DMW_BASE_SH) - 1))
> 
> Using unsigned long is wrong, breaking 32-bit hosts.
> You want
> 
> ((va) & MAKE_64BIT_MASK(0, TARGET_VIRT_ADDR_SPACE_BITS))
> 
>> +/* Performance Counter registers */
>> +#define LOONGARCH_CSR_PERFCTRL0  0x200 /* 32 perf event 0 config */
>> +#define LOONGARCH_CSR_PERFCNTR0  0x201 /* 64 perf event 0 count value */
>> +#define LOONGARCH_CSR_PERFCTRL1  0x202 /* 32 perf event 1 config */
>> +#define LOONGARCH_CSR_PERFCNTR1  0x203 /* 64 perf event 1 count value */
>> +#define LOONGARCH_CSR_PERFCTRL2  0x204 /* 32 perf event 2 config */
>> +#define LOONGARCH_CSR_PERFCNTR2  0x205 /* 64 perf event 2 count value */
>> +#define LOONGARCH_CSR_PERFCTRL3  0x206 /* 32 perf event 3 config */
>> +#define LOONGARCH_CSR_PERFCNTR3  0x207 /* 64 perf event 3 count value */
> 
> Perhaps better to 

Re: [RFC PATCH v2 00/30] Add Loongarch softmmu support.

2021-11-11 Thread yangxiaojuan
Hi, Mark,

On 11/11/2021 10:58 PM, Mark Cave-Ayland wrote:
> On 11/11/2021 01:34, Xiaojuan Yang wrote:
> 
>> Sorry only part of the v2 patch succeed. I consulted GNU sysadmin,
>> He said our mail server was getting temporarily banned by fail2ban. Now the 
>> ban
>> was removed. I resend the v2 series patch. For uefi is preparing to submit to
>> the community only uefi binary can be provided now. All of the series patch
>> add RFC title.
>>
>> This series patch add softmmu support for LoongArch.
>> Base on the linux-user emulation support V9 patch.
>>* 
>> https://patchew.org/QEMU/1630586467-22463-1-git-send-email-gaos...@loongson.cn/diff/1636340895-5255-1-git-send-email-gaos...@loongson.cn/
>>
>> The latest kernel:
>>* https://github.com/loongson/linux/tree/loongarch-next
>> The manual:
>>* 
>> https://github.com/loongson/LoongArch-Documentation/releases/tag/2021.10.11
>>
>> Changes for v2:
>> 1.Combine patch 2 and 3 into one.
>> 2.Adjust the order of the patch.
>> 3.Put all the binaries on the github.
>> 4.Modify some emulate errors when use the kernel from the github.
>> 5.Adjust some format problem and the Naming problem
>> 6.Others mainly follow Richard's code review comments.
>>
>> Please help review!
>>
>> Thanks
>>
>> Xiaojuan Yang (30):
>>target/loongarch: Update README
>>target/loongarch: Add CSR registers definition
>>target/loongarch: Add basic vmstate description of CPU.
>>target/loongarch: Define exceptions for LoongArch.
>>target/loongarch: Implement qmp_query_cpu_definitions()
>>target/loongarch: Add stabletimer support
>>target/loongarch: Add MMU support for LoongArch CPU.
>>target/loongarch: Add LoongArch CSR/IOCSR instruction
>>target/loongarch: Add TLB instruction support
>>target/loongarch: Add other core instructions support
>>target/loongarch: Add LoongArch interrupt and exception handle
>>target/loongarch: Add timer related instructions support.
>>target/loongarch: Add gdb support.
>>target/loongarch: Implement privilege instructions disassembly
>>hw/pci-host: Add ls7a1000 PCIe Host bridge support for Loongson
>>  Platform
>>hw/loongarch: Add a virt LoongArch 3A5000 board support
>>hw/loongarch: Add LoongArch cpu interrupt support(CPUINTC)
>>hw/loongarch: Add LoongArch ipi interrupt support(IPI)
>>hw/intc: Add LoongArch ls7a interrupt controller support(PCH-PIC)
>>hw/intc: Add LoongArch ls7a msi interrupt controller support(PCH-MSI)
>>hw/intc: Add LoongArch extioi interrupt controller(EIOINTC)
>>hw/loongarch: Add irq hierarchy for the system
>>hw/loongarch: Add some devices support for 3A5000.
>>hw/loongarch: Add LoongArch ls7a rtc device support
>>hw/loongarch: Add default bios startup support.
>>hw/loongarch: Add -kernel and -initrd options support
>>hw/loongarch: Add LoongArch smbios support
>>hw/loongarch: Add LoongArch acpi support
>>hw/loongarch: Add machine->possible_cpus
>>hw/loongarch: Add Numa support.
>>
>>   .../devices/loongarch64-softmmu/default.mak   |   3 +
>>   configs/targets/loongarch64-softmmu.mak   |   4 +
>>   gdb-xml/loongarch-base64.xml  |  43 +
>>   gdb-xml/loongarch-fpu64.xml   |  57 ++
>>   hw/Kconfig|   1 +
>>   hw/acpi/Kconfig   |   4 +
>>   hw/acpi/ls7a.c| 349 +++
>>   hw/acpi/meson.build   |   1 +
>>   hw/intc/Kconfig   |  12 +
>>   hw/intc/loongarch_extioi.c| 588 
>>   hw/intc/loongarch_pch_msi.c   |  73 ++
>>   hw/intc/loongarch_pch_pic.c   | 283 ++
>>   hw/intc/meson.build   |   3 +
>>   hw/loongarch/Kconfig  |  22 +
>>   hw/loongarch/acpi-build.c | 653 +
>>   hw/loongarch/fw_cfg.c |  33 +
>>   hw/loongarch/fw_cfg.h |  15 +
>>   hw/loongarch/ipi.c| 146 +++
>>   hw/loongarch/loongarch_int.c  |  59 ++
>>   hw/loongarch/ls3a5000_virt.c  | 647 +
>>   hw/loongarch/meson.build  |   7 +
>>   hw/meson.build|   1 +
>>   hw/pci-host/Kconfig   |   4 +
>>   hw/pci-host/ls7a.c| 223 +
>>   hw/pci-host/meson.build   |   1 +
>>   hw/rtc/Kconfig|   3 +
>>   hw/rtc/ls7a_rtc.c | 323 +++
>>   hw/rtc/meson.build|   1 +
>>   include/exec/poison.h |   2 +
>>   include/hw/acpi/ls7a.h|  53 ++
>>   include/hw/intc/loongarch_extioi.h| 101 ++
>>   include/hw/intc/loongarch_pch_msi.h   |  16 +
>>   

Re: [PATCH 08/31] target/loongarch: Add tlb instruction support

2021-10-29 Thread yangxiaojuan
Hi, Richard:

On 10/20/2021 12:19 PM, Richard Henderson wrote:
> On 10/19/21 12:34 AM, Xiaojuan Yang wrote:
>> This includes:
>> - TLBSRCH
>> - TLBRD
>> - TLBWR
>> - TLBFILL
>> - TLBCLR
>> - TLBFLUSH
>> - INVTLB
>>
>> Signed-off-by: Xiaojuan Yang 
>> Signed-off-by: Song Gao 
>> ---
>>   target/loongarch/cpu.c   |  19 +
>>   target/loongarch/helper.h|   8 +
>>   target/loongarch/insn_trans/trans_core.c |  54 +++
>>   target/loongarch/insns.decode|  14 +
>>   target/loongarch/internals.h |  18 +
>>   target/loongarch/tlb_helper.c| 468 +++
>>   6 files changed, 581 insertions(+)
>>
>> diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c
>> index f145afb603..afd186abac 100644
>> --- a/target/loongarch/cpu.c
>> +++ b/target/loongarch/cpu.c
>> @@ -118,6 +118,7 @@ static void set_loongarch_cpucfg(CPULoongArchState *env)
>>   static void set_loongarch_csr(CPULoongArchState *env)
>>   {
>>   uint64_t t;
>> +CPUState *cs = env_cpu(env);
>> t = FIELD_DP64(0, CSR_PRCFG1, SAVE_NUM, 8);
>>   t = FIELD_DP64(t, CSR_PRCFG1, TIMER_BITS, 0x2f);
>> @@ -145,6 +146,9 @@ static void set_loongarch_csr(CPULoongArchState *env)
>>   env->CSR_RVACFG = 0x0;
>>   env->CSR_ASID = 0xa;
>>   env->CSR_ERA = env->pc;
>> +env->CSR_CPUID = (cs->cpu_index & 0x1ff);
> 
> Any reason to have a copy of cpu_index, as opposed to just using that field?  
> CSR_CPUID is read-only after all.
> 
Yes, we need this value, the uefi code read this CPUID when Start slave cores.

>> +env->CSR_EENTRY |= (uint64_t)0x8000;
>> +env->CSR_TLBRENTRY |= (uint64_t)0x8000;
> 
> Are there really a defined reset values?  The documentation doesn't say.  It 
> would appear that the kernel must set these before enabling interrupts or 
> turning on paging.
> 
OK, it can be removed.

>> +#ifndef CONFIG_USER_ONLY
>> +qemu_fprintf(f, "EUEN0x%lx\n", env->CSR_EUEN);
>> +qemu_fprintf(f, "ESTAT   0x%lx\n", env->CSR_ESTAT);
>> +qemu_fprintf(f, "ERA 0x%lx\n", env->CSR_ERA);
>> +qemu_fprintf(f, "CRMD0x%lx\n", env->CSR_CRMD);
>> +qemu_fprintf(f, "PRMD0x%lx\n", env->CSR_PRMD);
>> +qemu_fprintf(f, "BadVAddr0x%lx\n", env->CSR_BADV);
>> +qemu_fprintf(f, "TLB refill ERA  0x%lx\n", env->CSR_TLBRERA);
>> +qemu_fprintf(f, "TLB refill BadV 0x%lx\n", env->CSR_TLBRBADV);
>> +qemu_fprintf(f, "EENTRY0x%lx\n", env->CSR_EENTRY);
>> +qemu_fprintf(f, "BadInstr0x%lx\n", env->CSR_BADI);
>> +qemu_fprintf(f, "PRCFG10x%lx\nPRCFG2 0x%lx\nPRCFG3 0x%lx\n",
>> + env->CSR_PRCFG1, env->CSR_PRCFG3, env->CSR_PRCFG3);
>> +#endif
> 
> This probably belongs to a different patch?
> 
>> @@ -165,4 +172,51 @@ static bool trans_iocsrwr_d(DisasContext *ctx, 
>> arg_iocsrwr_d *a)
>>   gen_helper_iocsr_write(cpu_env, addr, val, tcg_constant_i32(oi));
>>   return true;
>>   }
>> +
>> +static bool trans_tlbsrch(DisasContext *ctx, arg_tlbsrch *a)
>> +{
>> +gen_helper_tlbsrch(cpu_env);
>> +return true;
>> +}
> 
> Missing priv check, all functions.
> 
>> +static bool trans_invtlb(DisasContext *ctx, arg_invtlb *a)
>> +{
>> +TCGv addr = gpr_src(ctx, a->addr, EXT_NONE);
>> +TCGv info = gpr_src(ctx, a->info, EXT_NONE);
>> +TCGv op = tcg_constant_tl(a->invop);
>> +
>> +gen_helper_invtlb(cpu_env, addr, info, op);
>> +return true;
>> +}
> 
> Decode op here -- there are only 7 defined opcodes.
> 
> Note that you'll need to end the TB after most TLB instructions, since the 
> translation of PC could change between one insn and the next.
> 
> 
>> +_invtlb addr info invop
>> +@fmt_invtlb  .. .. . . . ._invtlb   
>>   %addr %info %invop
> 
> Why are you using the names addr and info instead of rk and rj?
> 
>> diff --git a/target/loongarch/internals.h b/target/loongarch/internals.h
>> index 1251e7f21c..916c675680 100644
>> --- a/target/loongarch/internals.h
>> +++ b/target/loongarch/internals.h
>> @@ -76,6 +76,14 @@ struct CPULoongArchTLBContext {
>>   int (*map_address)(struct CPULoongArchState *env, hwaddr *physical,
>>  int *prot, target_ulong address,
>>  MMUAccessType access_type);
>> +void (*helper_tlbwr)(struct CPULoongArchState *env);
>> +void (*helper_tlbfill)(struct CPULoongArchState *env);
>> +void (*helper_tlbsrch)(struct CPULoongArchState *env);
>> +void (*helper_tlbrd)(struct CPULoongArchState *env);
>> +void (*helper_tlbclr)(struct CPULoongArchState *env);
>> +void (*helper_tlbflush)(struct CPULoongArchState *env);
>> +void (*helper_invtlb)(struct CPULoongArchState *env, target_ulong addr,
>> +  target_ulong info, int op);
> 
> Again, function pointers are premature.
> 
>> +static uint64_t ls3a5k_pagesize_to_mask(int pagesize)
>> +{
>> +

Re: [PATCH 07/31] target/loongarch: Add loongarch csr/iocsr instruction support

2021-10-29 Thread yangxiaojuan
Hi, Richard:

On 10/20/2021 09:36 AM, Richard Henderson wrote:
> On 10/19/21 12:34 AM, Xiaojuan Yang wrote:
>> +target_ulong helper_csr_rdq(CPULoongArchState *env, uint64_t csr)
>> +{
>> +int64_t v;
>> +
>> +#define CASE_CSR_RDQ(csr)\
>> +case LOONGARCH_CSR_ ## csr:  \
>> +{\
>> +v = env->CSR_ ## csr;\
>> +break;   \
>> +};   \
> 
> There's absolutely no reason to call a helper function for a simple load.
> 
>> +case LOONGARCH_CSR_PGD:
>> +
>> +if (env->CSR_TLBRERA & 0x1) {
>> +v = env->CSR_TLBRBADV;
>> +} else {
>> +v = env->CSR_BADV;
>> +}
>> +
>> +if ((v >> 63) & 0x1) {
>> +v = env->CSR_PGDH;
>> +} else {
>> +v = env->CSR_PGDL;
>> +}
>> +break;
> 
> This is the only one that requires a helper on read.
> 
>> +if (csr == LOONGARCH_CSR_ASID) {
>> +if (old_v != val) {
>> +tlb_flush(env_cpu(env));
>> +}
>> +}
> 
> And this is the only one that requires a helper on write.
> 
>> +case LOONGARCH_CSR_ESTAT:
>> +qatomic_and(>CSR_ESTAT, ~mask);
> 
> Why do you believe this requires an atomic update?
> What is going to race with the update to a cpu private value?
> 
>> +static bool trans_csrrd(DisasContext *ctx, unsigned rd, unsigned csr)
>> +{
>> +TCGv dest = gpr_dst(ctx, rd, EXT_NONE);
>> +gen_helper_csr_rdq(dest, cpu_env, tcg_constant_i64(csr));
>> +return true;
>> +}
>> +
>> +static bool trans_csrwr(DisasContext *ctx, unsigned rd, unsigned csr)
>> +{
>> +TCGv dest = gpr_dst(ctx, rd, EXT_NONE);
>> +TCGv src1 = gpr_src(ctx, rd, EXT_NONE);
>> +
>> +switch (csr) {
>> +case LOONGARCH_CSR_CRMD:
>> +tcg_gen_movi_tl(cpu_pc, ctx->base.pc_next);
>> +gen_helper_csr_wrq(dest, cpu_env, src1, 
>> tcg_constant_i64(LOONGARCH_CSR_CRMD));
>> +tcg_gen_movi_tl(cpu_pc, ctx->base.pc_next + 4);
>> +ctx->base.is_jmp = DISAS_EXIT;
>> +break;
>> +case LOONGARCH_CSR_EUEN:
>> +gen_helper_csr_wrq(dest, cpu_env, src1, 
>> tcg_constant_i64(LOONGARCH_CSR_EUEN));
>> +/* Stop translation */
>> +tcg_gen_movi_tl(cpu_pc, ctx->base.pc_next + 4);
>> +ctx->base.is_jmp = DISAS_EXIT;
>> +break;
>> +default:
>> +gen_helper_csr_wrq(dest, cpu_env, src1, tcg_constant_i64(csr));
>> +break;
>> +}
>> +return true;
>> +}
>> +
>> +static bool trans_csrxchg(DisasContext *ctx, arg_csrxchg *a)
>> +{
>> +TCGv dest = gpr_dst(ctx, a->rd, EXT_NONE);
>> +TCGv src1 = gpr_src(ctx, a->rd, EXT_NONE);
>> +TCGv src2 = gpr_src(ctx, a->rj, EXT_NONE);
>> +
>> +if (a->rj == 0) {
>> +return trans_csrrd(ctx, a->rd, a->csr);
>> +} else if (a->rj == 1) {
>> +return trans_csrwr(ctx, a->rd, a->csr);
>> +}
> 
> These should have been decoded separately; see below.
> 
> You're missing the check for priv 0 here and in all other functions.
> 
>> +
>> +if (a->rd == 0) {
>> +gen_helper_csr_xchgq_r0(cpu_env, src2, tcg_constant_i64(a->csr));
>> +} else {
>> +gen_helper_csr_xchgq(dest, cpu_env, src1, src2, 
>> tcg_constant_i64(a->csr));
>> +}
> 
> Why do you believe r0 to require a special case?
>
 
OK, I have modified  all above.

>> +static bool trans_iocsrrd_b(DisasContext *ctx, arg_iocsrrd_b *a)
>> +{
>> +TCGv tmp = tcg_temp_new();
>> +TCGv dest = gpr_dst(ctx, a->rd, EXT_NONE);
>> +TCGv src1 = gpr_src(ctx, a->rj, EXT_NONE);
>> +
>> +gen_helper_iocsr_read(tmp, cpu_env, src1);
>> +tcg_gen_qemu_ld_tl(dest, tmp, ctx->mem_idx, MO_SB);
> 
> This is wrong.  From the manual:
> 
>   All IOCSR registers use independent addressing space
> 
> therefore you cannot simply read from a modified address, you must use a 
> completely different address space.
> 
> There are a couple of different solutions that are possible.
> 
> (1) Use helper functions calling address_space_ld/st*.
> 
> (2) Use a separate mmu_idx, which uses its own address space.
> This requires bouncing through MemTxAttrs, since
> cpu_asidx_from_attrs only take attrs and not mmu_idx.
> 
> The second one is may be overkill, unless there will be any cachable memory 
> in iospace.  I would expect most of it to be device memory.
> 
(1) For the iocsr registers, most of them act on the interrupt controller, the 
read and write will go to interrupt's mmio read/write.
So I modified the addr to their mmio range. The ext interrupt controller use 
the sysbus's function to handle the interrupt cascade and
sysbus_mmio_map to map the address which use the system memory region. So if I 
use a different address space, I realize a different 
sysbus_mmio_map function use a different address space, is it feasible ?

(2)Can you help me review the remaining patches? Thanks.

>> +csrxchg   0100 .. . . 

Re: [PATCH 01/31] target/loongarch: Upate the README for the softmmu.

2021-10-21 Thread yangxiaojuan



在 2021年10月20日 02:56, Richard Henderson 写道:
> On 10/19/21 12:34 AM, Xiaojuan Yang wrote:
>> ---
>>   target/loongarch/README  | 134 +++
>>   target/loongarch/ramdisk | Bin 0 -> 3077952 bytes
>>   target/loongarch/vmlinux | Bin 0 -> 24565536 bytes
>>   3 files changed, 134 insertions(+)
>>   create mode 100644 target/loongarch/ramdisk
>>   create mode 100755 target/loongarch/vmlinux
> 
> Wang Xuerui has already mentioned this, but let's make sure not to include 
> such large test binaries in future.
Yes, I will put the test binaries on the github.
> 
> The proper place for this is in a stable url, which could then either be 
> referenced in the documentation.  But even better than that would be an 
> acceptance test entry -- see tests/acceptance/boot_linux_console.py.
> 
>> +  2.Modify the kernel code for the tcg. Modifications are listed later. I 
>> will try to
>> +commit to the kernel host.
> 
> This sounds like a bug in the qemu emulation of the device or boot 
> environment.
>
Yes, qemu needs some adjustments, in the next version we can use the kernel 
directly. Thanks
> 
> r~




Re: [PATCH v7 02/21] target/loongarch: Add core definition

2021-10-21 Thread yangxiaojuan
Hi,Bob 

 Thank you for your advice,  I'll try.

Thanks 
Xiaojuan, Yang

在 2021年10月22日 08:12, Bob Proulx 写道:
> WANG Xuerui wrote:
>> Song Gao wrote:
>>> Account yangxiaoj...@loongson.cn It seems that she has been
>>> blacklisted. Xiaojuan sent 31 e-mails, which were not displayed
>>> since the 21st one, people who don't have a CC can't read all the
>>> emails,  and xiaojuan reply can't be in qemu-le...@nongnu.org.
> 
> I see 18 mails of a patchset accepted and delivered on 10/19 just two
> days ago from yangxiaoj...@loongson.cn.  Therefore it does not seem
> like there is a problem on the mailing list side of things.
> 
>>> Richard and Karl,  How can we solve this problem?
>> ...
>> As for the supposed "ban" on Xiaojuan's account, we cannot diagnose this
>> without mailing list owners' help; maybe it was just some kind of automatic
>> temporary ban, or even connectivity problem on Loongson's side.
> 
> The list owners were CC'd on the previous message and this one.  That
> includes a team of people which includes both Karl and myself and
> others.  It appears I have read and responded to this message first.
> 
> The rejection message that was included said:
> 
> 2021-10-20 09:33:59 (in an unknown timezone)
> Can not connect to recipient's server because of unstable network or 
> firewall filter.
> rcpt handle timeout,last handle info: Can not connect to 
> nongnu.org:2001:470:142:3::10:25
> 
> I suggest opening a help ticket on this issue by sending a new message
> to sysad...@gnu.org where the FSF admins can look into the problem.
> They are the only ones that can look at the incoming mail logs.
> Perhaps they can tell if there was anything logged.  It would be good
> to know the exact time that the connection problem occurred.
> 
> Bob
> 




Re: [PATCH] fw_cfg: Set the max fw_cfg mem read size to 8 bytes

2021-05-18 Thread yangxiaojuan
Hi,Laszlo

Thanks for your kindly reply.This is my first time to submit
code to the community. I spent some time learning how to configure
the email and reply.

Yes you are right, fw_cfg_init_mem_wide can solve my problem.
I just want to access 8 bytes width.I did not look at the code
carefully.I am so sorry. My patch can be abandoned.

I am from loongson. We want to add a new board support to
the community.Later we will submit more code.

thanks
Xiaojuan Yang


On 5/18/21 11:49 PM, Laszlo Ersek wrote:
> On 05/18/21 14:43, Xiaojuan Yang wrote:
>> From: yangxiaojuan 
>>
>> fw_cfg_data_read() func supports access widths from
>> 1 to 8 bytes while the ops set the wrong read size.
>>
>> Most arch use 8 bytes to send ram_size to bios.
>>
>> Signed-off-by: yangxiaojuan 
>> ---
>>  hw/nvram/fw_cfg.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
>> index 9b8dcca4ea..242614152c 100644
>> --- a/hw/nvram/fw_cfg.c
>> +++ b/hw/nvram/fw_cfg.c
>> @@ -540,7 +540,7 @@ static const MemoryRegionOps fw_cfg_data_mem_ops = {
>>  .endianness = DEVICE_BIG_ENDIAN,
>>  .valid = {
>>  .min_access_size = 1,
>> -.max_access_size = 1,
>> +.max_access_size = 8,
>>  .accepts = fw_cfg_data_mem_valid,
>>  },
>>  };
>>
> 
> This patch conflicts with (adjacent) commits
> 
> - cfaadf0e89e7 ("fw_cfg_mem: introduce the "data_width" property",
> 2014-12-22) and
> 
> - 6c87e3d5967a ("fw_cfg_mem: expose the "data_width" property with
> fw_cfg_init_mem_wide()", 2014-12-22)
> 
> Your board code is supposed to call the fw_cfg_init_mem_wide() function,
> for setting the maximum access width.
> 
> In fact, I see a call to fw_cfg_init_mem_wide() in
> "hw/mips/loongson3_virt.c" already, from commit c76b409fef19 ("hw/mips:
> Add Loongson-3 machine support", 2021-01-04). (I'm only highlighting
> this board because your email address is from domain "loongson.cn".)
> 
> What is the actual problem you're trying to solve?
> 
> Thanks
> Laszlo
>