Re: [PATCH v8 1/1] audio/jack: fix use after free segfault
On Samstag, 22. August 2020 02:16:23 CEST Geoffrey McRae wrote: > On 2020-08-22 03:47, Paolo Bonzini wrote: > > On 21/08/20 19:34, Christian Schoenebeck wrote: > >>> static void qjack_fini_out(HWVoiceOut *hw) > >>> { > >>> > >>> QJackOut *jo = (QJackOut *)hw; > >>> qjack_client_fini(&jo->c); > >>> > >>> + > >>> +qemu_bh_delete(jo->c.shutdown_bh); > >> > >> Paolo wrapped that qemu_bh_delete() call inside the lock as well. So I > >> guess > >> it makes a difference for the BH API? > > > > It is not a problem as long as qjack_client_fini is idempotent. > > `qjack_client_fini` is indeed idempotent Right. > >>> +qemu_mutex_destroy(&jo->c.shutdown_lock); > >>> > >>> } > >> > >> Hmmm, is this qemu_mutex_destroy() safe at this point? > > > > Perhaps make the mutex global and not destroy it at all. > > It's safe at this point as `qjack_fini_out` is only called at device > destruction, and `qjack_client_fini` ensures that JACK is shut down > which prevents jack from trying to call the shutdown event handler. You mean because jack_client_close() is synchronized. That prevents JACK from firing the callback after jack_client_close() returns, that's correct. But as qemu_bh_delete() is async, you do not have a guarantee that a previously scheduled BH shutdown handler is no longer running. So it might still hold the lock when you attempt to destroy the mutex. On doubt I would do like Paolo suggested by making the mutex global and not destroying it at all. Best regards, Christian Schoenebeck
Re: [PATCH v3 05/10] migration/dirtyrate: Record hash results for each sampled page
On 2020/8/21 20:39, Dr. David Alan Gilbert wrote: > * Daniel P. Berrangé (berra...@redhat.com) wrote: >> On Fri, Aug 21, 2020 at 08:22:06PM +0800, Zheng Chuan wrote: >>> >>> >>> On 2020/8/21 1:55, Dr. David Alan Gilbert wrote: * Daniel P. Berrangé (berra...@redhat.com) wrote: > On Thu, Aug 20, 2020 at 06:30:09PM +0100, Dr. David Alan Gilbert wrote: >> * Chuan Zheng (zhengch...@huawei.com) wrote: >>> Record hash results for each sampled page. >>> >>> Signed-off-by: Chuan Zheng >>> Signed-off-by: YanYing Zhuang >>> --- >>> migration/dirtyrate.c | 144 >>> ++ >>> migration/dirtyrate.h | 7 +++ >>> 2 files changed, 151 insertions(+) >>> >>> diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c >>> index c4304ef..62b6f69 100644 >>> --- a/migration/dirtyrate.c >>> +++ b/migration/dirtyrate.c >>> @@ -25,6 +25,7 @@ >>> #include "dirtyrate.h" >>> >>> CalculatingDirtyRateState CalculatingState = CAL_DIRTY_RATE_INIT; >>> +static unsigned long int qcrypto_hash_len = QCRYPTO_HASH_LEN; >> >> Why do we need this static rather than just using the QCRYPTO_HASH_LEN ? >> It's never going to change is it? >> (and anyway it's just a MD5 len?) > > I wouldn't want to bet on that given that this is use of MD5. We might > claim this isn't security critical, but surprises happen, and we will > certainly be dinged on security audits for introducing new use of MD5 > no matter what. > > If a cryptographic hash is required, then sha256 should be the choice > for any new code that doesn't have back compat requirements. > > If a cryptographic hash is not required then how about crc32 It doesn't need to be cryptographic; is crc32 the fastest reasonable hash for use in large areas? Dave > IOW, it doesn't make a whole lot of sense to say we need a cryptographic > hash, but then pick the most insecure one. > > sha256 is slower than md5, but it is conceivable that in future we might > gain support for something like Blake2b which is similar security level > to SHA3, while being faster than MD5. > > Overall I'm pretty unethusiastic about use of MD5 being introduced and > worse, being hardcoded as the only option. > > Regards, > Daniel > -- > |: https://berrange.com -o- > https://www.flickr.com/photos/dberrange :| > |: https://libvirt.org -o- > https://fstop138.berrange.com :| > |: https://entangle-photo.org-o- > https://www.instagram.com/dberrange :| >>> >>> Hi, Daniel, Dave. >>> >>> I do compare MD5 and SHA256 with vm memory of 128G under mempress of 100G. >>> >>> 1. Calculation speed >>> 1) MD5 takes about 500ms to sample and hash all pages by >>> record_ramblock_hash_info(). >>> 2) SHA256 takes about 750ms to sample all pages by >>> record_ramblock_hash_info(). >>> >>> 2. CPU Consumption >>> 1) MD5 may have instant rise up to 48% for dirtyrate thread >>> 2) SHA256 may have instant rise up to 75% for dirtyrate thread >>> >>> 3. Memory Consumption >>> SHA256 may need twice memory than MD5 due to its HASH_LEN. >>> >>> I am trying to consider if crc32 is more faster and takes less memory and >>> is more safer than MD5? >> >> No, crc32 is absolutely *weaker* than MD5. It is NOT a cryptographic >> hash so does not try to guarantee collision resistance. It only has >> 2^32 possible outputs. >> >> MD5 does try to guarantee collision resistance, but MD5 is considered >> broken these days, so a malicious attacker can cause collisions if they >> are motivated enough. >> >> IOW if you need collision resistance that SHA256 should be used. > > There's no need to guard against malicious behaviour here - this is just > a stat to guide migration. > If CRC32 is likely to be faster than md5 I suspect it's enough. > > Dave > Hi,Dave, Daniel. I did test by crc32,it is much faster than MD5 and SHA256:) As for 128G vm it takes only about 50ms to sample and hash all pages by record_ramblock_hash_info(). And the dirtyrate calculation is still good enough:) ++ | |dirtyrate| ++ | no mempress | 4MB/s | -- | mempress 4096 1024 |1248MB/s | ++ | mempress 4096 4096 |4060MB/s | ++ I will take crc32 in PatchV4, is that OK from the perspective of safety? In my opinion, it should be safe. The crc32 is only for compare and the recorder will be free after calculation is over. The output is just dirtyrate for user to guide migration. What's more, i consider increase DIRTYRATE_DEFAULT_SAMPLE_PAGES from 256 to 512 which may takes about 75ms to sa
[PULL v3 20/20] hw/intc: ibex_plic: Honour source priorities
This patch follows what commit aa4d30f6618dc "riscv: plic: Honour source priorities" does and ensures that the highest priority interrupt will be serviced first. Signed-off-by: Alistair Francis Cc: Jessica Clarke Reviewed-by: Philippe Mathieu-Daudé Message-Id: --- hw/intc/ibex_plic.c | 15 ++- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/hw/intc/ibex_plic.c b/hw/intc/ibex_plic.c index 669247ef08..f49fa67c91 100644 --- a/hw/intc/ibex_plic.c +++ b/hw/intc/ibex_plic.c @@ -57,6 +57,8 @@ static void ibex_plic_irqs_set_pending(IbexPlicState *s, int irq, bool level) static bool ibex_plic_irqs_pending(IbexPlicState *s, uint32_t context) { int i; +uint32_t max_irq = 0; +uint32_t max_prio = s->threshold; for (i = 0; i < s->pending_num; i++) { uint32_t irq_num = ctz64(s->pending[i]) + (i * 32); @@ -66,14 +68,17 @@ static bool ibex_plic_irqs_pending(IbexPlicState *s, uint32_t context) continue; } -if (s->priority[irq_num] > s->threshold) { -if (!s->claim) { -s->claim = irq_num; -} -return true; +if (s->priority[irq_num] > max_prio) { +max_irq = irq_num; +max_prio = s->priority[irq_num]; } } +if (max_irq) { +s->claim = max_irq; +return true; +} + return false; } -- 2.28.0
[PULL v3 19/20] hw/intc: ibex_plic: Don't allow repeat interrupts on claimed lines
Once an interrupt has been claimed, but before it has been compelted we shouldn't receive any more pending interrupts. This patche keeps track of this to ensure that we don't see any more interrupts until it is completed. Signed-off-by: Alistair Francis Message-Id: <394c3f070615ff2b4fab61a1cf9cb48c122913b7.1595655188.git.alistair.fran...@wdc.com> --- include/hw/intc/ibex_plic.h | 1 + hw/intc/ibex_plic.c | 17 + 2 files changed, 18 insertions(+) diff --git a/include/hw/intc/ibex_plic.h b/include/hw/intc/ibex_plic.h index ddc7909903..d8eb09b258 100644 --- a/include/hw/intc/ibex_plic.h +++ b/include/hw/intc/ibex_plic.h @@ -33,6 +33,7 @@ typedef struct IbexPlicState { MemoryRegion mmio; uint32_t *pending; +uint32_t *claimed; uint32_t *source; uint32_t *priority; uint32_t *enable; diff --git a/hw/intc/ibex_plic.c b/hw/intc/ibex_plic.c index 578edd2ce0..669247ef08 100644 --- a/hw/intc/ibex_plic.c +++ b/hw/intc/ibex_plic.c @@ -43,6 +43,14 @@ static void ibex_plic_irqs_set_pending(IbexPlicState *s, int irq, bool level) { int pending_num = irq / 32; +if (s->claimed[pending_num] & 1 << (irq % 32)) { +/* + * The interrupt has been claimed, but not compelted. + * The pending bit can't be set. + */ +return; +} + s->pending[pending_num] |= level << (irq % 32); } @@ -120,6 +128,10 @@ static uint64_t ibex_plic_read(void *opaque, hwaddr addr, int pending_num = s->claim / 32; s->pending[pending_num] &= ~(1 << (s->claim % 32)); +/* Set the interrupt as claimed, but not compelted */ +s->claimed[pending_num] |= 1 << (s->claim % 32); + +/* Return the current claimed interrupt */ ret = s->claim; /* Update the interrupt status after the claim */ @@ -155,6 +167,10 @@ static void ibex_plic_write(void *opaque, hwaddr addr, /* Interrupt was completed */ s->claim = 0; } +if (s->claimed[value / 32] & 1 << (value % 32)) { +/* This value was already claimed, clear it. */ +s->claimed[value / 32] &= ~(1 << (value % 32)); +} } ibex_plic_update(s); @@ -215,6 +231,7 @@ static void ibex_plic_realize(DeviceState *dev, Error **errp) int i; s->pending = g_new0(uint32_t, s->pending_num); +s->claimed = g_new0(uint32_t, s->pending_num); s->source = g_new0(uint32_t, s->source_num); s->priority = g_new0(uint32_t, s->priority_num); s->enable = g_new0(uint32_t, s->enable_num); -- 2.28.0
[PULL v3 18/20] hw/intc: ibex_plic: Update the pending irqs
After a claim or a priority change we need to update the pending interrupts. This is based on the same patch for the SiFive PLIC: 55765822804f5a58594e "riscv: plic: Add a couple of mising sifive_plic_update calls" Signed-off-by: Alistair Francis Cc: Jessica Clarke Reviewed-by: Philippe Mathieu-Daudé Message-Id: <0693aa700a4c67c49b3f1c973a82b257fdb7198d.1595655188.git.alistair.fran...@wdc.com> --- hw/intc/ibex_plic.c | 4 1 file changed, 4 insertions(+) diff --git a/hw/intc/ibex_plic.c b/hw/intc/ibex_plic.c index 41079518c6..578edd2ce0 100644 --- a/hw/intc/ibex_plic.c +++ b/hw/intc/ibex_plic.c @@ -121,6 +121,9 @@ static uint64_t ibex_plic_read(void *opaque, hwaddr addr, s->pending[pending_num] &= ~(1 << (s->claim % 32)); ret = s->claim; + +/* Update the interrupt status after the claim */ +ibex_plic_update(s); } return ret; @@ -140,6 +143,7 @@ static void ibex_plic_write(void *opaque, hwaddr addr, } else if (addr_between(addr, s->priority_base, s->priority_num)) { uint32_t irq = ((addr - s->priority_base) >> 2) + 1; s->priority[irq] = value & 7; +ibex_plic_update(s); } else if (addr_between(addr, s->enable_base, s->enable_num)) { uint32_t enable_reg = (addr - s->enable_base) / 4; -- 2.28.0
[PULL v3 16/20] target/riscv: Fix the translation of physical address
From: Zong Li The real physical address should add the 12 bits page offset. It also causes the PMP wrong checking due to the minimum granularity of PMP is 4 byte, but we always get the physical address which is 4KB alignment, that means, we always use the start address of the page to check PMP for all addresses which in the same page. Signed-off-by: Zong Li Reviewed-by: Alistair Francis Message-Id: <370a983d0f9e8a9a927b9bb8af5e7bc84b1bf9b1.1595924470.git.zong...@sifive.com> Signed-off-by: Alistair Francis --- target/riscv/cpu_helper.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c index 75d2ae3434..2f337e418c 100644 --- a/target/riscv/cpu_helper.c +++ b/target/riscv/cpu_helper.c @@ -543,7 +543,8 @@ restart: /* for superpage mappings, make a fake leaf PTE for the TLB's benefit. */ target_ulong vpn = addr >> PGSHIFT; -*physical = (ppn | (vpn & ((1L << ptshift) - 1))) << PGSHIFT; +*physical = ((ppn | (vpn & ((1L << ptshift) - 1))) << PGSHIFT) | +(addr & ~TARGET_PAGE_MASK); /* set permissions on the TLB entry */ if ((pte & PTE_R) || ((pte & PTE_X) && mxr)) { @@ -630,7 +631,7 @@ hwaddr riscv_cpu_get_phys_page_debug(CPUState *cs, vaddr addr) } } -return phys_addr; +return phys_addr & TARGET_PAGE_MASK; } void riscv_cpu_do_transaction_failed(CPUState *cs, hwaddr physaddr, -- 2.28.0
[PULL v3 17/20] target/riscv: Change the TLB page size depends on PMP entries.
From: Zong Li The minimum granularity of PMP is 4 bytes, it is small than 4KB page size, therefore, the pmp checking would be ignored if its range doesn't start from the alignment of one page. This patch detects the pmp entries and sets the small page size to TLB if there is a PMP entry which cover the page size. Signed-off-by: Zong Li Reviewed-by: Alistair Francis Message-Id: <6b0bf48662ef26ab4c15381a08e78a74ebd7ca79.1595924470.git.zong...@sifive.com> Signed-off-by: Alistair Francis --- target/riscv/pmp.h| 2 ++ target/riscv/cpu_helper.c | 10 ++-- target/riscv/pmp.c| 52 +++ 3 files changed, 62 insertions(+), 2 deletions(-) diff --git a/target/riscv/pmp.h b/target/riscv/pmp.h index 8e19793132..6a8f072871 100644 --- a/target/riscv/pmp.h +++ b/target/riscv/pmp.h @@ -60,5 +60,7 @@ void pmpaddr_csr_write(CPURISCVState *env, uint32_t addr_index, target_ulong pmpaddr_csr_read(CPURISCVState *env, uint32_t addr_index); bool pmp_hart_has_privs(CPURISCVState *env, target_ulong addr, target_ulong size, pmp_priv_t priv, target_ulong mode); +bool pmp_is_range_in_tlb(CPURISCVState *env, hwaddr tlb_sa, + target_ulong *tlb_size); #endif diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c index 2f337e418c..fd1d373b6f 100644 --- a/target/riscv/cpu_helper.c +++ b/target/riscv/cpu_helper.c @@ -693,6 +693,7 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size, bool first_stage_error = true; int ret = TRANSLATE_FAIL; int mode = mmu_idx; +target_ulong tlb_size = 0; env->guest_phys_fault_addr = 0; @@ -784,8 +785,13 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size, } if (ret == TRANSLATE_SUCCESS) { -tlb_set_page(cs, address & TARGET_PAGE_MASK, pa & TARGET_PAGE_MASK, - prot, mmu_idx, TARGET_PAGE_SIZE); +if (pmp_is_range_in_tlb(env, pa & TARGET_PAGE_MASK, &tlb_size)) { +tlb_set_page(cs, address & ~(tlb_size - 1), pa & ~(tlb_size - 1), + prot, mmu_idx, tlb_size); +} else { +tlb_set_page(cs, address & TARGET_PAGE_MASK, pa & TARGET_PAGE_MASK, + prot, mmu_idx, TARGET_PAGE_SIZE); +} return true; } else if (probe) { return false; diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c index b14feeb7da..c394e867f8 100644 --- a/target/riscv/pmp.c +++ b/target/riscv/pmp.c @@ -383,3 +383,55 @@ target_ulong pmpaddr_csr_read(CPURISCVState *env, uint32_t addr_index) return val; } + +/* + * Calculate the TLB size if the start address or the end address of + * PMP entry is presented in thie TLB page. + */ +static target_ulong pmp_get_tlb_size(CPURISCVState *env, int pmp_index, + target_ulong tlb_sa, target_ulong tlb_ea) +{ +target_ulong pmp_sa = env->pmp_state.addr[pmp_index].sa; +target_ulong pmp_ea = env->pmp_state.addr[pmp_index].ea; + +if (pmp_sa >= tlb_sa && pmp_ea <= tlb_ea) { +return pmp_ea - pmp_sa + 1; +} + +if (pmp_sa >= tlb_sa && pmp_sa <= tlb_ea && pmp_ea >= tlb_ea) { +return tlb_ea - pmp_sa + 1; +} + +if (pmp_ea <= tlb_ea && pmp_ea >= tlb_sa && pmp_sa <= tlb_sa) { +return pmp_ea - tlb_sa + 1; +} + +return 0; +} + +/* + * Check is there a PMP entry which range covers this page. If so, + * try to find the minimum granularity for the TLB size. + */ +bool pmp_is_range_in_tlb(CPURISCVState *env, hwaddr tlb_sa, + target_ulong *tlb_size) +{ +int i; +target_ulong val; +target_ulong tlb_ea = (tlb_sa + TARGET_PAGE_SIZE - 1); + +for (i = 0; i < MAX_RISCV_PMPS; i++) { +val = pmp_get_tlb_size(env, i, tlb_sa, tlb_ea); +if (val) { +if (*tlb_size == 0 || *tlb_size > val) { +*tlb_size = val; +} +} +} + +if (*tlb_size != 0) { +return true; +} + +return false; +} -- 2.28.0
[PULL v3 09/20] riscv: Fix bug in setting pmpcfg CSR for RISCV64
From: Hou Weiying First, sizeof(target_ulong) equals to 4 on riscv32, so this change does not change the function on riscv32. Second, sizeof(target_ulong) equals to 8 on riscv64, and 'reg_index * 8 + i' is not a legal pmp_index (we will explain later), which should be 'reg_index * 4 + i'. If the parameter reg_index equals to 2 (means that we will change the value of pmpcfg2, or the second pmpcfg on riscv64), then pmpcfg_csr_write(env, 2, val) will map write tasks to pmp_write_cfg(env, 2 * 8 + [0...7], val). However, no cfg csr is indexed by value 16 or 23 on riscv64, so we consider it as a bug. We are looking for constant (e.g., define a new constant named RISCV_WORD_SIZE) in QEMU to help others understand code better, but none was found. A possible good explanation of this literal is it is the minimum word length on riscv is 4 bytes (32 bit). Signed-off-by: Hongzheng-Li Signed-off-by: Hou Weiying Signed-off-by: Myriad-Dreamin Reviewed-by: Alistair Francis Message-Id: Signed-off-by: Alistair Francis --- target/riscv/pmp.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c index 2a2b9f5363..b14feeb7da 100644 --- a/target/riscv/pmp.c +++ b/target/riscv/pmp.c @@ -320,8 +320,7 @@ void pmpcfg_csr_write(CPURISCVState *env, uint32_t reg_index, for (i = 0; i < sizeof(target_ulong); i++) { cfg_val = (val >> 8 * i) & 0xff; -pmp_write_cfg(env, (reg_index * sizeof(target_ulong)) + i, -cfg_val); +pmp_write_cfg(env, (reg_index * 4) + i, cfg_val); } } @@ -336,7 +335,7 @@ target_ulong pmpcfg_csr_read(CPURISCVState *env, uint32_t reg_index) target_ulong val = 0; for (i = 0; i < sizeof(target_ulong); i++) { -val = pmp_read_cfg(env, (reg_index * sizeof(target_ulong)) + i); +val = pmp_read_cfg(env, (reg_index * 4) + i); cfg_val |= (val << (i * 8)); } trace_pmpcfg_csr_read(env->mhartid, reg_index, cfg_val); -- 2.28.0
[PULL v3 10/20] configure: Create symbolic links for pc-bios/*.elf files
From: Bin Meng Now we need to ship the OpenSBI fw_dynamic.elf image for the RISC-V Spike machine, it requires us to create symbolic links for pc-bios/*.elf files. Signed-off-by: Bin Meng Reviewed-by: Alistair Francis Message-Id: <1596439832-29238-2-git-send-email-bmeng...@gmail.com> Signed-off-by: Alistair Francis --- configure | 1 + 1 file changed, 1 insertion(+) diff --git a/configure b/configure index 4e5fe33211..0a01802637 100755 --- a/configure +++ b/configure @@ -8129,6 +8129,7 @@ LINKS="$LINKS tests/qemu-iotests/check" LINKS="$LINKS python" for bios_file in \ $source_path/pc-bios/*.bin \ +$source_path/pc-bios/*.elf \ $source_path/pc-bios/*.lid \ $source_path/pc-bios/*.rom \ $source_path/pc-bios/*.dtb \ -- 2.28.0
[PULL v3 08/20] hw/riscv: sifive_u: Add a dummy L2 cache controller device
From: Bin Meng It is enough to simply map the SiFive FU540 L2 cache controller into the MMIO space using create_unimplemented_device(), with an FDT fragment generated, to make the latest upstream U-Boot happy. Signed-off-by: Bin Meng Reviewed-by: Alistair Francis Message-Id: <1595227748-24720-1-git-send-email-bmeng...@gmail.com> Signed-off-by: Alistair Francis --- include/hw/riscv/sifive_u.h | 4 hw/riscv/sifive_u.c | 22 ++ 2 files changed, 26 insertions(+) diff --git a/include/hw/riscv/sifive_u.h b/include/hw/riscv/sifive_u.h index aba4d0181f..d3c0c00d10 100644 --- a/include/hw/riscv/sifive_u.h +++ b/include/hw/riscv/sifive_u.h @@ -71,6 +71,7 @@ enum { SIFIVE_U_DEBUG, SIFIVE_U_MROM, SIFIVE_U_CLINT, +SIFIVE_U_L2CC, SIFIVE_U_L2LIM, SIFIVE_U_PLIC, SIFIVE_U_PRCI, @@ -86,6 +87,9 @@ enum { }; enum { +SIFIVE_U_L2CC_IRQ0 = 1, +SIFIVE_U_L2CC_IRQ1 = 2, +SIFIVE_U_L2CC_IRQ2 = 3, SIFIVE_U_UART0_IRQ = 4, SIFIVE_U_UART1_IRQ = 5, SIFIVE_U_GPIO_IRQ0 = 7, diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c index e5682c38a9..55b3383c31 100644 --- a/hw/riscv/sifive_u.c +++ b/hw/riscv/sifive_u.c @@ -72,6 +72,7 @@ static const struct MemmapEntry { [SIFIVE_U_DEBUG] ={0x0, 0x100 }, [SIFIVE_U_MROM] = { 0x1000, 0xf000 }, [SIFIVE_U_CLINT] ={ 0x200,0x1 }, +[SIFIVE_U_L2CC] = { 0x201, 0x1000 }, [SIFIVE_U_L2LIM] ={ 0x800, 0x200 }, [SIFIVE_U_PLIC] = { 0xc00, 0x400 }, [SIFIVE_U_PRCI] = { 0x1000, 0x1000 }, @@ -302,6 +303,24 @@ static void create_fdt(SiFiveUState *s, const struct MemmapEntry *memmap, qemu_fdt_setprop_string(fdt, nodename, "compatible", "gpio-restart"); g_free(nodename); +nodename = g_strdup_printf("/soc/cache-controller@%lx", +(long)memmap[SIFIVE_U_L2CC].base); +qemu_fdt_add_subnode(fdt, nodename); +qemu_fdt_setprop_cells(fdt, nodename, "reg", +0x0, memmap[SIFIVE_U_L2CC].base, +0x0, memmap[SIFIVE_U_L2CC].size); +qemu_fdt_setprop_cells(fdt, nodename, "interrupts", +SIFIVE_U_L2CC_IRQ0, SIFIVE_U_L2CC_IRQ1, SIFIVE_U_L2CC_IRQ2); +qemu_fdt_setprop_cell(fdt, nodename, "interrupt-parent", plic_phandle); +qemu_fdt_setprop(fdt, nodename, "cache-unified", NULL, 0); +qemu_fdt_setprop_cell(fdt, nodename, "cache-size", 2097152); +qemu_fdt_setprop_cell(fdt, nodename, "cache-sets", 1024); +qemu_fdt_setprop_cell(fdt, nodename, "cache-level", 2); +qemu_fdt_setprop_cell(fdt, nodename, "cache-block-size", 64); +qemu_fdt_setprop_string(fdt, nodename, "compatible", +"sifive,fu540-c000-ccache"); +g_free(nodename); + phy_phandle = phandle++; nodename = g_strdup_printf("/soc/ethernet@%lx", (long)memmap[SIFIVE_U_GEM].base); @@ -733,6 +752,9 @@ static void sifive_u_soc_realize(DeviceState *dev, Error **errp) create_unimplemented_device("riscv.sifive.u.dmc", memmap[SIFIVE_U_DMC].base, memmap[SIFIVE_U_DMC].size); + +create_unimplemented_device("riscv.sifive.u.l2cc", +memmap[SIFIVE_U_L2CC].base, memmap[SIFIVE_U_L2CC].size); } static Property sifive_u_soc_props[] = { -- 2.28.0
[PULL v3 04/20] target/riscv: Check nanboxed inputs to fp helpers
From: Richard Henderson If a 32-bit input is not properly nanboxed, then the input is replaced with the default qnan. Signed-off-by: Richard Henderson Reviewed-by: LIU Zhiwei Message-Id: <20200724002807.441147-5-richard.hender...@linaro.org> Signed-off-by: Alistair Francis --- target/riscv/internals.h | 11 +++ target/riscv/fpu_helper.c | 64 --- 2 files changed, 57 insertions(+), 18 deletions(-) diff --git a/target/riscv/internals.h b/target/riscv/internals.h index 9f4ba7d617..f1a546dba6 100644 --- a/target/riscv/internals.h +++ b/target/riscv/internals.h @@ -43,4 +43,15 @@ static inline uint64_t nanbox_s(float32 f) return f | MAKE_64BIT_MASK(32, 32); } +static inline float32 check_nanbox_s(uint64_t f) +{ +uint64_t mask = MAKE_64BIT_MASK(32, 32); + +if (likely((f & mask) == mask)) { +return (uint32_t)f; +} else { +return 0x7fc0u; /* default qnan */ +} +} + #endif diff --git a/target/riscv/fpu_helper.c b/target/riscv/fpu_helper.c index 72541958a7..bb346a8249 100644 --- a/target/riscv/fpu_helper.c +++ b/target/riscv/fpu_helper.c @@ -81,9 +81,12 @@ void helper_set_rounding_mode(CPURISCVState *env, uint32_t rm) set_float_rounding_mode(softrm, &env->fp_status); } -static uint64_t do_fmadd_s(CPURISCVState *env, uint64_t frs1, uint64_t frs2, - uint64_t frs3, int flags) +static uint64_t do_fmadd_s(CPURISCVState *env, uint64_t rs1, uint64_t rs2, + uint64_t rs3, int flags) { +float32 frs1 = check_nanbox_s(rs1); +float32 frs2 = check_nanbox_s(rs2); +float32 frs3 = check_nanbox_s(rs3); return nanbox_s(float32_muladd(frs1, frs2, frs3, flags, &env->fp_status)); } @@ -139,74 +142,97 @@ uint64_t helper_fnmadd_d(CPURISCVState *env, uint64_t frs1, uint64_t frs2, float_muladd_negate_product, &env->fp_status); } -uint64_t helper_fadd_s(CPURISCVState *env, uint64_t frs1, uint64_t frs2) +uint64_t helper_fadd_s(CPURISCVState *env, uint64_t rs1, uint64_t rs2) { +float32 frs1 = check_nanbox_s(rs1); +float32 frs2 = check_nanbox_s(rs2); return nanbox_s(float32_add(frs1, frs2, &env->fp_status)); } -uint64_t helper_fsub_s(CPURISCVState *env, uint64_t frs1, uint64_t frs2) +uint64_t helper_fsub_s(CPURISCVState *env, uint64_t rs1, uint64_t rs2) { +float32 frs1 = check_nanbox_s(rs1); +float32 frs2 = check_nanbox_s(rs2); return nanbox_s(float32_sub(frs1, frs2, &env->fp_status)); } -uint64_t helper_fmul_s(CPURISCVState *env, uint64_t frs1, uint64_t frs2) +uint64_t helper_fmul_s(CPURISCVState *env, uint64_t rs1, uint64_t rs2) { +float32 frs1 = check_nanbox_s(rs1); +float32 frs2 = check_nanbox_s(rs2); return nanbox_s(float32_mul(frs1, frs2, &env->fp_status)); } -uint64_t helper_fdiv_s(CPURISCVState *env, uint64_t frs1, uint64_t frs2) +uint64_t helper_fdiv_s(CPURISCVState *env, uint64_t rs1, uint64_t rs2) { +float32 frs1 = check_nanbox_s(rs1); +float32 frs2 = check_nanbox_s(rs2); return nanbox_s(float32_div(frs1, frs2, &env->fp_status)); } -uint64_t helper_fmin_s(CPURISCVState *env, uint64_t frs1, uint64_t frs2) +uint64_t helper_fmin_s(CPURISCVState *env, uint64_t rs1, uint64_t rs2) { +float32 frs1 = check_nanbox_s(rs1); +float32 frs2 = check_nanbox_s(rs2); return nanbox_s(float32_minnum(frs1, frs2, &env->fp_status)); } -uint64_t helper_fmax_s(CPURISCVState *env, uint64_t frs1, uint64_t frs2) +uint64_t helper_fmax_s(CPURISCVState *env, uint64_t rs1, uint64_t rs2) { +float32 frs1 = check_nanbox_s(rs1); +float32 frs2 = check_nanbox_s(rs2); return nanbox_s(float32_maxnum(frs1, frs2, &env->fp_status)); } -uint64_t helper_fsqrt_s(CPURISCVState *env, uint64_t frs1) +uint64_t helper_fsqrt_s(CPURISCVState *env, uint64_t rs1) { +float32 frs1 = check_nanbox_s(rs1); return nanbox_s(float32_sqrt(frs1, &env->fp_status)); } -target_ulong helper_fle_s(CPURISCVState *env, uint64_t frs1, uint64_t frs2) +target_ulong helper_fle_s(CPURISCVState *env, uint64_t rs1, uint64_t rs2) { +float32 frs1 = check_nanbox_s(rs1); +float32 frs2 = check_nanbox_s(rs2); return float32_le(frs1, frs2, &env->fp_status); } -target_ulong helper_flt_s(CPURISCVState *env, uint64_t frs1, uint64_t frs2) +target_ulong helper_flt_s(CPURISCVState *env, uint64_t rs1, uint64_t rs2) { +float32 frs1 = check_nanbox_s(rs1); +float32 frs2 = check_nanbox_s(rs2); return float32_lt(frs1, frs2, &env->fp_status); } -target_ulong helper_feq_s(CPURISCVState *env, uint64_t frs1, uint64_t frs2) +target_ulong helper_feq_s(CPURISCVState *env, uint64_t rs1, uint64_t rs2) { +float32 frs1 = check_nanbox_s(rs1); +float32 frs2 = check_nanbox_s(rs2); return float32_eq_quiet(frs1, frs2, &env->fp_status); } -target_ulong helper_fcvt_w_s(CPURISCVState *env, uint64_t frs1) +target_ulong helper_fcvt_w_s(CPURISCVState *env, uint64_t rs1) { +float32
[PULL v3 15/20] gitlab-ci/opensbi: Update GitLab CI to build generic platform
From: Bin Meng This updates the GitLab CI opensbi job to build opensbi bios images for the generic platform. Signed-off-by: Bin Meng Reviewed-by: Anup Patel Reviewed-by: Alistair Francis Message-Id: <1596439832-29238-7-git-send-email-bmeng...@gmail.com> Signed-off-by: Alistair Francis --- .gitlab-ci.d/opensbi.yml | 28 ++-- 1 file changed, 10 insertions(+), 18 deletions(-) diff --git a/.gitlab-ci.d/opensbi.yml b/.gitlab-ci.d/opensbi.yml index 62088ec5ec..5b13047e2a 100644 --- a/.gitlab-ci.d/opensbi.yml +++ b/.gitlab-ci.d/opensbi.yml @@ -35,18 +35,14 @@ build-opensbi: when: always artifacts: paths: # 'artifacts.zip' will contains the following files: - - pc-bios/opensbi-riscv32-sifive_u-fw_jump.bin - - pc-bios/opensbi-riscv32-virt-fw_jump.bin - - pc-bios/opensbi-riscv64-sifive_u-fw_jump.bin - - pc-bios/opensbi-riscv64-virt-fw_jump.bin - - opensbi32-virt-stdout.log - - opensbi32-virt-stderr.log - - opensbi64-virt-stdout.log - - opensbi64-virt-stderr.log - - opensbi32-sifive_u-stdout.log - - opensbi32-sifive_u-stderr.log - - opensbi64-sifive_u-stdout.log - - opensbi64-sifive_u-stderr.log + - pc-bios/opensbi-riscv32-generic-fw_dynamic.bin + - pc-bios/opensbi-riscv32-generic-fw_dynamic.elf + - pc-bios/opensbi-riscv64-generic-fw_dynamic.bin + - pc-bios/opensbi-riscv64-generic-fw_dynamic.elf + - opensbi32-generic-stdout.log + - opensbi32-generic-stderr.log + - opensbi64-generic-stdout.log + - opensbi64-generic-stderr.log image: $CI_REGISTRY_IMAGE:opensbi-cross-build variables: GIT_DEPTH: 3 @@ -55,10 +51,6 @@ build-opensbi: - export JOBS=$(($(getconf _NPROCESSORS_ONLN) + 1)) - echo "=== Using ${JOBS} simultaneous jobs ===" - make -j${JOBS} -C roms/opensbi clean - - make -j${JOBS} -C roms opensbi32-virt 2>&1 1>opensbi32-virt-stdout.log | tee -a opensbi32-virt-stderr.log >&2 + - make -j${JOBS} -C roms opensbi32-generic 2>&1 1>opensbi32-generic-stdout.log | tee -a opensbi32-generic-stderr.log >&2 - make -j${JOBS} -C roms/opensbi clean - - make -j${JOBS} -C roms opensbi64-virt 2>&1 1>opensbi64-virt-stdout.log | tee -a opensbi64-virt-stderr.log >&2 - - make -j${JOBS} -C roms/opensbi clean - - make -j${JOBS} -C roms opensbi32-sifive_u 2>&1 1>opensbi32-sifive_u-stdout.log | tee -a opensbi32-sifive_u-stderr.log >&2 - - make -j${JOBS} -C roms/opensbi clean - - make -j${JOBS} -C roms opensbi64-sifive_u 2>&1 1>opensbi64-sifive_u-stdout.log | tee -a opensbi64-sifive_u-stderr.log >&2 + - make -j${JOBS} -C roms opensbi64-generic 2>&1 1>opensbi64-generic-stdout.log | tee -a opensbi64-generic-stderr.log >&2 -- 2.28.0
[PULL v3 01/20] target/riscv: Generate nanboxed results from fp helpers
From: Richard Henderson Make sure that all results from single-precision scalar helpers are properly nan-boxed to 64-bits. Signed-off-by: Richard Henderson Reviewed-by: LIU Zhiwei Message-Id: <20200724002807.441147-2-richard.hender...@linaro.org> Signed-off-by: Alistair Francis --- target/riscv/internals.h | 5 + target/riscv/fpu_helper.c | 42 +-- 2 files changed, 28 insertions(+), 19 deletions(-) diff --git a/target/riscv/internals.h b/target/riscv/internals.h index 37d33820ad..9f4ba7d617 100644 --- a/target/riscv/internals.h +++ b/target/riscv/internals.h @@ -38,4 +38,9 @@ target_ulong fclass_d(uint64_t frs1); #define SEW32 2 #define SEW64 3 +static inline uint64_t nanbox_s(float32 f) +{ +return f | MAKE_64BIT_MASK(32, 32); +} + #endif diff --git a/target/riscv/fpu_helper.c b/target/riscv/fpu_helper.c index 4379756dc4..72541958a7 100644 --- a/target/riscv/fpu_helper.c +++ b/target/riscv/fpu_helper.c @@ -81,10 +81,16 @@ void helper_set_rounding_mode(CPURISCVState *env, uint32_t rm) set_float_rounding_mode(softrm, &env->fp_status); } +static uint64_t do_fmadd_s(CPURISCVState *env, uint64_t frs1, uint64_t frs2, + uint64_t frs3, int flags) +{ +return nanbox_s(float32_muladd(frs1, frs2, frs3, flags, &env->fp_status)); +} + uint64_t helper_fmadd_s(CPURISCVState *env, uint64_t frs1, uint64_t frs2, uint64_t frs3) { -return float32_muladd(frs1, frs2, frs3, 0, &env->fp_status); +return do_fmadd_s(env, frs1, frs2, frs3, 0); } uint64_t helper_fmadd_d(CPURISCVState *env, uint64_t frs1, uint64_t frs2, @@ -96,8 +102,7 @@ uint64_t helper_fmadd_d(CPURISCVState *env, uint64_t frs1, uint64_t frs2, uint64_t helper_fmsub_s(CPURISCVState *env, uint64_t frs1, uint64_t frs2, uint64_t frs3) { -return float32_muladd(frs1, frs2, frs3, float_muladd_negate_c, - &env->fp_status); +return do_fmadd_s(env, frs1, frs2, frs3, float_muladd_negate_c); } uint64_t helper_fmsub_d(CPURISCVState *env, uint64_t frs1, uint64_t frs2, @@ -110,8 +115,7 @@ uint64_t helper_fmsub_d(CPURISCVState *env, uint64_t frs1, uint64_t frs2, uint64_t helper_fnmsub_s(CPURISCVState *env, uint64_t frs1, uint64_t frs2, uint64_t frs3) { -return float32_muladd(frs1, frs2, frs3, float_muladd_negate_product, - &env->fp_status); +return do_fmadd_s(env, frs1, frs2, frs3, float_muladd_negate_product); } uint64_t helper_fnmsub_d(CPURISCVState *env, uint64_t frs1, uint64_t frs2, @@ -124,8 +128,8 @@ uint64_t helper_fnmsub_d(CPURISCVState *env, uint64_t frs1, uint64_t frs2, uint64_t helper_fnmadd_s(CPURISCVState *env, uint64_t frs1, uint64_t frs2, uint64_t frs3) { -return float32_muladd(frs1, frs2, frs3, float_muladd_negate_c | - float_muladd_negate_product, &env->fp_status); +return do_fmadd_s(env, frs1, frs2, frs3, + float_muladd_negate_c | float_muladd_negate_product); } uint64_t helper_fnmadd_d(CPURISCVState *env, uint64_t frs1, uint64_t frs2, @@ -137,37 +141,37 @@ uint64_t helper_fnmadd_d(CPURISCVState *env, uint64_t frs1, uint64_t frs2, uint64_t helper_fadd_s(CPURISCVState *env, uint64_t frs1, uint64_t frs2) { -return float32_add(frs1, frs2, &env->fp_status); +return nanbox_s(float32_add(frs1, frs2, &env->fp_status)); } uint64_t helper_fsub_s(CPURISCVState *env, uint64_t frs1, uint64_t frs2) { -return float32_sub(frs1, frs2, &env->fp_status); +return nanbox_s(float32_sub(frs1, frs2, &env->fp_status)); } uint64_t helper_fmul_s(CPURISCVState *env, uint64_t frs1, uint64_t frs2) { -return float32_mul(frs1, frs2, &env->fp_status); +return nanbox_s(float32_mul(frs1, frs2, &env->fp_status)); } uint64_t helper_fdiv_s(CPURISCVState *env, uint64_t frs1, uint64_t frs2) { -return float32_div(frs1, frs2, &env->fp_status); +return nanbox_s(float32_div(frs1, frs2, &env->fp_status)); } uint64_t helper_fmin_s(CPURISCVState *env, uint64_t frs1, uint64_t frs2) { -return float32_minnum(frs1, frs2, &env->fp_status); +return nanbox_s(float32_minnum(frs1, frs2, &env->fp_status)); } uint64_t helper_fmax_s(CPURISCVState *env, uint64_t frs1, uint64_t frs2) { -return float32_maxnum(frs1, frs2, &env->fp_status); +return nanbox_s(float32_maxnum(frs1, frs2, &env->fp_status)); } uint64_t helper_fsqrt_s(CPURISCVState *env, uint64_t frs1) { -return float32_sqrt(frs1, &env->fp_status); +return nanbox_s(float32_sqrt(frs1, &env->fp_status)); } target_ulong helper_fle_s(CPURISCVState *env, uint64_t frs1, uint64_t frs2) @@ -209,23 +213,23 @@ uint64_t helper_fcvt_lu_s(CPURISCVState *env, uint64_t frs1) uint64_t helper_fcvt_s_w(CPURISCVState *env, target_ulong rs1) { -return int32_to_float32((int32_t)rs1, &env->fp_status); +return nanbox_s(int32_to_float32(
[PULL v3 11/20] roms/opensbi: Upgrade from v0.7 to v0.8
From: Bin Meng Upgrade OpenSBI from v0.7 to v0.8. The v0.8 release includes the following commits: 1bb00ab lib: No need to provide default PMP region using platform callbacks a9eac67 include: sbi_platform: Combine reboot and shutdown into one callback 6585fab lib: utils: Add SiFive test device 4781545 platform: Add Nuclei UX600 platform 3a326af scripts: adapt binary archive script for Nuclei UX600 5bdf022 firmware: fw_base: Remove CSR_MTVEC update check e6c1345 lib: utils/serial: Skip baudrate config if input frequency is zero 01a8c8e lib: utils: Improve fdt_parse_uart8250() API 0a0093b lib: utils: Add fdt_parse_uart8250_node() function 243b0d0 lib: utils: Remove redundant clint_ipi_sync() declaration e3ad7c1 lib: utils: Rename fdt_parse_clint() to fdt_parse_compat_addr() a39cd6f lib: utils: Add FDT match table based node lookup dd33b9e lib: utils: Make fdt_get_node_addr_size() public function 66185b3 lib: utils: Add fdt_parse_sifive_uart_node() function 19e966b lib: utils: Add fdt_parse_hart_id() function 44dd7be lib: utils: Add fdt_parse_max_hart_id() API f0eb503 lib: utils: Add fdt_parse_plic_node() function 1ac794c include: Add array_size() macro 8ff2b94 lib: utils: Add simple FDT timer framework 76f0f81 lib: utils: Add simple FDT ipi framework 75322a6 lib: utils: Add simple FDT irqchip framework 76a8940 lib: utils: Add simple FDT serial framework 7cc6fa4 lib: utils: Add simple FDT reset framework 4d06353 firmware: fw_base: Introduce optional fw_platform_init() f1aa9e5 platform: Add generic FDT based platform support 1f21b99 lib: sbi: Print platform hart count at boot time 2ba7087 scripts: Add generic platform to create-binary-archive.sh 4f18c6e platform: generic: Add Sifive FU540 TLB flush range limit override 13717a8 platform: Remove qemu/virt directory 65c06b0 platform: Remove spike directory d626037 docs: Add missing links in platform.md 7993ca2 include: sbi: Remove redundant page table related defines 5338679 lib: sbi_tlb: Fix remote TLB HFENCE VVMA implementation dc38929 lib: sbi: Improve misa_string() implementation 433bac7 docs: platform/generic: Add details about stdout-path DT property b4efa70 docs: platform/generic: Add details about IPI and timer expectations dfd9dd6 docs: Add platform requirements document c2286b6 docs: Fix ordering of pages in table of contents 7be75f5 docs: Don't use italic text in page title 63a513e lib: Rename unprivileged trap handler aef9a60 lib: Add csr detect support 13ca20d lib: Create a separate math helper function file 79d0fad lib: utils: Update reserved memory fdt node even if PMP is not present 6a053f6 lib: Add support for hart specific features b2df751 platform: Move platform features to hart 4938024 platform: fpga: Remove redundant platform specific features ec0d2a7 lib: timer: Provide a hart based timer feature 1f235ec lib: Add platform features in boot time print 22c4334 lib: Add hart features in boot time print 36833ab lib: Optimize inline assembly for unprivilege access functions 38a4b54 firmware: Correct spelling mistakes 28b4052 lib: sbi: detect features before everything else in sbi_hart_init() 4984183 lib: sbi: Improve get_feature_str() implementation and usage 3aa1036 lib: sbi: Remove extra spaces from boot time prints 3a8fc81 lib: sbi: Print platform HART count just before boot HART id 63b0f5f include: sbi: Use scratch pointer as parmeter in HART feature APIs 2966510 lib: sbi: Few cosmetic improvements to HART feature detection a38bea9 lib: sbi_hart: Detect number of supported PMP regions 89ba634 include: sbi: Add firmware extension constants 73d6ef3 lib: utils: Remove redundant parameters from PLIC init functions 446a9c6 lib: utils: Allow PLIC functions to be used for multiple PLICs 2c685c2 lib: utils: Extend fdt_find_match() Implementation d30bb68 lib: utils/irqchip: Initialize all matching irqchip DT nodes a9a9751 lib: utils: Allow CLINT functions to be used for multiple CLINTs 569dd64 lib: utils: Add fdt_parse_clint_node() function 6956e83 lib: utils/ipi: Initialize all matching ipi DT nodes a63f05f lib: utils/timer: Initialize all matching timer DT nodes 30b6040 Makefile: Fix builtin DTB compilation for out-of-tree platforms 64f1408 firmware: fw_base: Make builtin DTB available to fw_platform_init() 4ce6b7a firmware: fw_base: Don't OR forced FW_OPTIONS 86ec534 firmware: Allow fw_platform_init() to return updated FDT location c6c65ee Makefile: Preprocess builtin DTS 4e3876d Makefile: Add mechanism for platforms to have multiple builtin DTBs 72019ee platform: kendryte/k210: Use new mechanism of builtin DTB 51f0e4a firmware: Remove FW_PAYLOAD_FDT and related documentation 1b8c012 lib: Add RISC-V hypervisor v0.6.1 support 79bfd67 docs: Use doxygen config to mark the main page 106b888 docs: Remove redundant documentation about combined payload use case 9802906 platform: Add AE350 platform specific SBI handler 32f87e5 platform: Add AE350 cache control SBIs e2c3f01 lib: Fix __sbi_hfence_gvma_vmid_gpa() and __sbi_hfence_vvma_asid_va() 6966ad0 pla
[PULL v3 07/20] target/riscv: check before allocating TCG temps
From: LIU Zhiwei Signed-off-by: LIU Zhiwei Signed-off-by: Richard Henderson Message-Id: <20200626205917.4545-5-zhiwei_...@c-sky.com> Signed-off-by: Richard Henderson Message-Id: <20200724002807.441147-8-richard.hender...@linaro.org> Signed-off-by: Alistair Francis --- target/riscv/insn_trans/trans_rvd.c.inc | 8 target/riscv/insn_trans/trans_rvf.c.inc | 8 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/target/riscv/insn_trans/trans_rvd.c.inc b/target/riscv/insn_trans/trans_rvd.c.inc index ea1044f13b..4f832637fa 100644 --- a/target/riscv/insn_trans/trans_rvd.c.inc +++ b/target/riscv/insn_trans/trans_rvd.c.inc @@ -20,10 +20,10 @@ static bool trans_fld(DisasContext *ctx, arg_fld *a) { -TCGv t0 = tcg_temp_new(); -gen_get_gpr(t0, a->rs1); REQUIRE_FPU; REQUIRE_EXT(ctx, RVD); +TCGv t0 = tcg_temp_new(); +gen_get_gpr(t0, a->rs1); tcg_gen_addi_tl(t0, t0, a->imm); tcg_gen_qemu_ld_i64(cpu_fpr[a->rd], t0, ctx->mem_idx, MO_TEQ); @@ -35,10 +35,10 @@ static bool trans_fld(DisasContext *ctx, arg_fld *a) static bool trans_fsd(DisasContext *ctx, arg_fsd *a) { -TCGv t0 = tcg_temp_new(); -gen_get_gpr(t0, a->rs1); REQUIRE_FPU; REQUIRE_EXT(ctx, RVD); +TCGv t0 = tcg_temp_new(); +gen_get_gpr(t0, a->rs1); tcg_gen_addi_tl(t0, t0, a->imm); tcg_gen_qemu_st_i64(cpu_fpr[a->rs2], t0, ctx->mem_idx, MO_TEQ); diff --git a/target/riscv/insn_trans/trans_rvf.c.inc b/target/riscv/insn_trans/trans_rvf.c.inc index 138e317723..3dfec8211d 100644 --- a/target/riscv/insn_trans/trans_rvf.c.inc +++ b/target/riscv/insn_trans/trans_rvf.c.inc @@ -25,10 +25,10 @@ static bool trans_flw(DisasContext *ctx, arg_flw *a) { -TCGv t0 = tcg_temp_new(); -gen_get_gpr(t0, a->rs1); REQUIRE_FPU; REQUIRE_EXT(ctx, RVF); +TCGv t0 = tcg_temp_new(); +gen_get_gpr(t0, a->rs1); tcg_gen_addi_tl(t0, t0, a->imm); tcg_gen_qemu_ld_i64(cpu_fpr[a->rd], t0, ctx->mem_idx, MO_TEUL); @@ -41,11 +41,11 @@ static bool trans_flw(DisasContext *ctx, arg_flw *a) static bool trans_fsw(DisasContext *ctx, arg_fsw *a) { +REQUIRE_FPU; +REQUIRE_EXT(ctx, RVF); TCGv t0 = tcg_temp_new(); gen_get_gpr(t0, a->rs1); -REQUIRE_FPU; -REQUIRE_EXT(ctx, RVF); tcg_gen_addi_tl(t0, t0, a->imm); tcg_gen_qemu_st_i64(cpu_fpr[a->rs2], t0, ctx->mem_idx, MO_TEUL); -- 2.28.0
[PULL v3 12/20] roms/Makefile: Build the generic platform for RISC-V OpenSBI firmware
From: Bin Meng The RISC-V generic platform is a flattened device tree (FDT) based platform where all platform specific functionality is provided based on FDT passed by previous booting stage. The support was added in the upstream OpenSBI v0.8 release recently. Update our Makefile to build the generic platform instead of building virt and sifive_u separately for RISC-V OpenSBI firmware, and change to use fw_dynamic type images as well. Signed-off-by: Bin Meng Reviewed-by: Anup Patel Reviewed-by: Alistair Francis Message-Id: <1596439832-29238-4-git-send-email-bmeng...@gmail.com> Signed-off-by: Alistair Francis --- roms/Makefile | 32 ++-- 1 file changed, 10 insertions(+), 22 deletions(-) diff --git a/roms/Makefile b/roms/Makefile index f9acf39954..5d9f15b677 100644 --- a/roms/Makefile +++ b/roms/Makefile @@ -64,10 +64,8 @@ default help: @echo " u-boot.e500-- update u-boot.e500" @echo " u-boot.sam460 -- update u-boot.sam460" @echo " efi-- update UEFI (edk2) platform firmware" - @echo " opensbi32-virt -- update OpenSBI for 32-bit virt machine" - @echo " opensbi64-virt -- update OpenSBI for 64-bit virt machine" - @echo " opensbi32-sifive_u -- update OpenSBI for 32-bit sifive_u machine" - @echo " opensbi64-sifive_u -- update OpenSBI for 64-bit sifive_u machine" + @echo " opensbi32-generic -- update OpenSBI for 32-bit generic machine" + @echo " opensbi64-generic -- update OpenSBI for 64-bit generic machine" @echo " bios-microvm -- update bios-microvm.bin (qboot)" @echo " clean -- delete the files generated by the previous" \ "build targets" @@ -170,29 +168,19 @@ skiboot: efi: edk2-basetools $(MAKE) -f Makefile.edk2 -opensbi32-virt: +opensbi32-generic: $(MAKE) -C opensbi \ CROSS_COMPILE=$(riscv32_cross_prefix) \ - PLATFORM="qemu/virt" - cp opensbi/build/platform/qemu/virt/firmware/fw_jump.bin ../pc-bios/opensbi-riscv32-virt-fw_jump.bin + PLATFORM="generic" + cp opensbi/build/platform/generic/firmware/fw_dynamic.bin ../pc-bios/opensbi-riscv32-generic-fw_dynamic.bin + cp opensbi/build/platform/generic/firmware/fw_dynamic.elf ../pc-bios/opensbi-riscv32-generic-fw_dynamic.elf -opensbi64-virt: +opensbi64-generic: $(MAKE) -C opensbi \ CROSS_COMPILE=$(riscv64_cross_prefix) \ - PLATFORM="qemu/virt" - cp opensbi/build/platform/qemu/virt/firmware/fw_jump.bin ../pc-bios/opensbi-riscv64-virt-fw_jump.bin - -opensbi32-sifive_u: - $(MAKE) -C opensbi \ - CROSS_COMPILE=$(riscv32_cross_prefix) \ - PLATFORM="sifive/fu540" - cp opensbi/build/platform/sifive/fu540/firmware/fw_jump.bin ../pc-bios/opensbi-riscv32-sifive_u-fw_jump.bin - -opensbi64-sifive_u: - $(MAKE) -C opensbi \ - CROSS_COMPILE=$(riscv64_cross_prefix) \ - PLATFORM="sifive/fu540" - cp opensbi/build/platform/sifive/fu540/firmware/fw_jump.bin ../pc-bios/opensbi-riscv64-sifive_u-fw_jump.bin + PLATFORM="generic" + cp opensbi/build/platform/generic/firmware/fw_dynamic.bin ../pc-bios/opensbi-riscv64-generic-fw_dynamic.bin + cp opensbi/build/platform/generic/firmware/fw_dynamic.elf ../pc-bios/opensbi-riscv64-generic-fw_dynamic.elf bios-microvm: $(MAKE) -C qboot -- 2.28.0
[PULL v3 03/20] target/riscv: Generate nanboxed results from trans_rvf.inc.c
From: Richard Henderson Make sure that all results from inline single-precision scalar operations are properly nan-boxed to 64-bits. Signed-off-by: Richard Henderson Reviewed-by: LIU Zhiwei Message-Id: <20200724002807.441147-4-richard.hender...@linaro.org> Signed-off-by: Alistair Francis --- target/riscv/insn_trans/trans_rvf.c.inc | 4 1 file changed, 4 insertions(+) diff --git a/target/riscv/insn_trans/trans_rvf.c.inc b/target/riscv/insn_trans/trans_rvf.c.inc index c7057482e8..264d3139f1 100644 --- a/target/riscv/insn_trans/trans_rvf.c.inc +++ b/target/riscv/insn_trans/trans_rvf.c.inc @@ -167,6 +167,7 @@ static bool trans_fsgnj_s(DisasContext *ctx, arg_fsgnj_s *a) tcg_gen_deposit_i64(cpu_fpr[a->rd], cpu_fpr[a->rs2], cpu_fpr[a->rs1], 0, 31); } +gen_nanbox_s(cpu_fpr[a->rd], cpu_fpr[a->rd]); mark_fs_dirty(ctx); return true; } @@ -183,6 +184,7 @@ static bool trans_fsgnjn_s(DisasContext *ctx, arg_fsgnjn_s *a) tcg_gen_deposit_i64(cpu_fpr[a->rd], t0, cpu_fpr[a->rs1], 0, 31); tcg_temp_free_i64(t0); } +gen_nanbox_s(cpu_fpr[a->rd], cpu_fpr[a->rd]); mark_fs_dirty(ctx); return true; } @@ -199,6 +201,7 @@ static bool trans_fsgnjx_s(DisasContext *ctx, arg_fsgnjx_s *a) tcg_gen_xor_i64(cpu_fpr[a->rd], cpu_fpr[a->rs1], t0); tcg_temp_free_i64(t0); } +gen_nanbox_s(cpu_fpr[a->rd], cpu_fpr[a->rd]); mark_fs_dirty(ctx); return true; } @@ -369,6 +372,7 @@ static bool trans_fmv_w_x(DisasContext *ctx, arg_fmv_w_x *a) #else tcg_gen_extu_i32_i64(cpu_fpr[a->rd], t0); #endif +gen_nanbox_s(cpu_fpr[a->rd], cpu_fpr[a->rd]); mark_fs_dirty(ctx); tcg_temp_free(t0); -- 2.28.0
[PULL v3 02/20] target/riscv: Generalize gen_nanbox_fpr to gen_nanbox_s
From: Richard Henderson Do not depend on the RVD extension, take input and output via TCGv_i64 instead of fpu regno. Move the function to translate.c so that it can be used in multiple trans_*.inc.c files. Signed-off-by: Richard Henderson Reviewed-by: LIU Zhiwei Message-Id: <20200724002807.441147-3-richard.hender...@linaro.org> Signed-off-by: Alistair Francis --- target/riscv/translate.c| 11 +++ target/riscv/insn_trans/trans_rvf.c.inc | 16 +--- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/target/riscv/translate.c b/target/riscv/translate.c index d0485c0750..1290faddda 100644 --- a/target/riscv/translate.c +++ b/target/riscv/translate.c @@ -90,6 +90,17 @@ static inline bool has_ext(DisasContext *ctx, uint32_t ext) return ctx->misa & ext; } +/* + * RISC-V requires NaN-boxing of narrower width floating point values. + * This applies when a 32-bit value is assigned to a 64-bit FP register. + * For consistency and simplicity, we nanbox results even when the RVD + * extension is not present. + */ +static void gen_nanbox_s(TCGv_i64 out, TCGv_i64 in) +{ +tcg_gen_ori_i64(out, in, MAKE_64BIT_MASK(32, 32)); +} + static void generate_exception(DisasContext *ctx, int excp) { tcg_gen_movi_tl(cpu_pc, ctx->base.pc_next); diff --git a/target/riscv/insn_trans/trans_rvf.c.inc b/target/riscv/insn_trans/trans_rvf.c.inc index 3bfd8881e7..c7057482e8 100644 --- a/target/riscv/insn_trans/trans_rvf.c.inc +++ b/target/riscv/insn_trans/trans_rvf.c.inc @@ -23,20 +23,6 @@ return false; \ } while (0) -/* - * RISC-V requires NaN-boxing of narrower width floating - * point values. This applies when a 32-bit value is - * assigned to a 64-bit FP register. Thus this does not - * apply when the RVD extension is not present. - */ -static void gen_nanbox_fpr(DisasContext *ctx, int regno) -{ -if (has_ext(ctx, RVD)) { -tcg_gen_ori_i64(cpu_fpr[regno], cpu_fpr[regno], -MAKE_64BIT_MASK(32, 32)); -} -} - static bool trans_flw(DisasContext *ctx, arg_flw *a) { TCGv t0 = tcg_temp_new(); @@ -46,7 +32,7 @@ static bool trans_flw(DisasContext *ctx, arg_flw *a) tcg_gen_addi_tl(t0, t0, a->imm); tcg_gen_qemu_ld_i64(cpu_fpr[a->rd], t0, ctx->mem_idx, MO_TEUL); -gen_nanbox_fpr(ctx, a->rd); +gen_nanbox_s(cpu_fpr[a->rd], cpu_fpr[a->rd]); tcg_temp_free(t0); mark_fs_dirty(ctx); -- 2.28.0
[PULL v3 06/20] target/riscv: Clean up fmv.w.x
From: LIU Zhiwei Use tcg_gen_extu_tl_i64 to avoid the ifdef. Signed-off-by: LIU Zhiwei Signed-off-by: Richard Henderson Message-Id: <20200626205917.4545-7-zhiwei_...@c-sky.com> Signed-off-by: Richard Henderson Message-Id: <20200724002807.441147-7-richard.hender...@linaro.org> Signed-off-by: Alistair Francis --- target/riscv/insn_trans/trans_rvf.c.inc | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/target/riscv/insn_trans/trans_rvf.c.inc b/target/riscv/insn_trans/trans_rvf.c.inc index 832f01db6f..138e317723 100644 --- a/target/riscv/insn_trans/trans_rvf.c.inc +++ b/target/riscv/insn_trans/trans_rvf.c.inc @@ -406,11 +406,7 @@ static bool trans_fmv_w_x(DisasContext *ctx, arg_fmv_w_x *a) TCGv t0 = tcg_temp_new(); gen_get_gpr(t0, a->rs1); -#if defined(TARGET_RISCV64) -tcg_gen_mov_i64(cpu_fpr[a->rd], t0); -#else -tcg_gen_extu_i32_i64(cpu_fpr[a->rd], t0); -#endif +tcg_gen_extu_tl_i64(cpu_fpr[a->rd], t0); gen_nanbox_s(cpu_fpr[a->rd], cpu_fpr[a->rd]); mark_fs_dirty(ctx); -- 2.28.0
[PULL v3 00/20] riscv-to-apply queue
The following changes since commit f86d9a093dada59bde5582c7ec320487c4b8: Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging (2020-08-21 17:26:52 +0100) are available in the Git repository at: g...@github.com:alistair23/qemu.git tags/pull-riscv-to-apply-20200821-1 for you to fetch changes up to 01c41d15de13104774d08e951db24815c8cffc79: hw/intc: ibex_plic: Honour source priorities (2020-08-21 22:37:55 -0700) The first RISC-V PR for the 5.2 window. This includes: - NaNBox fixes - Vector extension improvements - a L2 cache controller - PMP fixes - Upgrade to OpenSBI v0.8 and the generic platform - Fixes for the Ibex PLIC Alistair Francis (3): hw/intc: ibex_plic: Update the pending irqs hw/intc: ibex_plic: Don't allow repeat interrupts on claimed lines hw/intc: ibex_plic: Honour source priorities Bin Meng (7): hw/riscv: sifive_u: Add a dummy L2 cache controller device configure: Create symbolic links for pc-bios/*.elf files roms/opensbi: Upgrade from v0.7 to v0.8 roms/Makefile: Build the generic platform for RISC-V OpenSBI firmware hw/riscv: Use pre-built bios image of generic platform for virt & sifive_u hw/riscv: spike: Change the default bios to use generic platform image gitlab-ci/opensbi: Update GitLab CI to build generic platform Hou Weiying (1): riscv: Fix bug in setting pmpcfg CSR for RISCV64 LIU Zhiwei (2): target/riscv: Clean up fmv.w.x target/riscv: check before allocating TCG temps Richard Henderson (5): target/riscv: Generate nanboxed results from fp helpers target/riscv: Generalize gen_nanbox_fpr to gen_nanbox_s target/riscv: Generate nanboxed results from trans_rvf.inc.c target/riscv: Check nanboxed inputs to fp helpers target/riscv: Check nanboxed inputs in trans_rvf.inc.c Zong Li (2): target/riscv: Fix the translation of physical address target/riscv: Change the TLB page size depends on PMP entries. configure | 1 + Makefile | 4 +- include/hw/intc/ibex_plic.h| 1 + include/hw/riscv/sifive_u.h| 4 + target/riscv/internals.h | 16 target/riscv/pmp.h | 2 + hw/intc/ibex_plic.c| 36 +++-- hw/riscv/sifive_u.c| 26 ++- hw/riscv/spike.c | 9 ++- hw/riscv/virt.c| 4 +- target/riscv/cpu_helper.c | 15 +++- target/riscv/fpu_helper.c | 102 - target/riscv/pmp.c | 57 +- target/riscv/translate.c | 29 +++ .gitlab-ci.d/opensbi.yml | 28 +++ pc-bios/opensbi-riscv32-generic-fw_dynamic.bin | Bin 0 -> 62144 bytes pc-bios/opensbi-riscv32-generic-fw_dynamic.elf | Bin 0 -> 558668 bytes pc-bios/opensbi-riscv32-sifive_u-fw_jump.bin | Bin 49520 -> 0 bytes pc-bios/opensbi-riscv32-virt-fw_jump.bin | Bin 49504 -> 0 bytes pc-bios/opensbi-riscv64-generic-fw_dynamic.bin | Bin 0 -> 70792 bytes pc-bios/opensbi-riscv64-generic-fw_dynamic.elf | Bin 0 -> 620424 bytes pc-bios/opensbi-riscv64-sifive_u-fw_jump.bin | Bin 57936 -> 0 bytes pc-bios/opensbi-riscv64-virt-fw_jump.bin | Bin 57920 -> 0 bytes roms/Makefile | 32 +++- roms/opensbi | 2 +- target/riscv/insn_trans/trans_rvd.c.inc| 8 +- target/riscv/insn_trans/trans_rvf.c.inc| 99 +++- 27 files changed, 338 insertions(+), 137 deletions(-) create mode 100644 pc-bios/opensbi-riscv32-generic-fw_dynamic.bin create mode 100644 pc-bios/opensbi-riscv32-generic-fw_dynamic.elf delete mode 100644 pc-bios/opensbi-riscv32-sifive_u-fw_jump.bin delete mode 100644 pc-bios/opensbi-riscv32-virt-fw_jump.bin create mode 100644 pc-bios/opensbi-riscv64-generic-fw_dynamic.bin create mode 100644 pc-bios/opensbi-riscv64-generic-fw_dynamic.elf delete mode 100644 pc-bios/opensbi-riscv64-sifive_u-fw_jump.bin delete mode 100644 pc-bios/opensbi-riscv64-virt-fw_jump.bin
[PULL v3 05/20] target/riscv: Check nanboxed inputs in trans_rvf.inc.c
From: Richard Henderson If a 32-bit input is not properly nanboxed, then the input is replaced with the default qnan. The only inline expansion is for the sign-changing set of instructions: FSGNJ.S, FSGNJX.S, FSGNJN.S. Signed-off-by: Richard Henderson Reviewed-by: LIU Zhiwei Message-Id: <20200724002807.441147-6-richard.hender...@linaro.org> Signed-off-by: Alistair Francis --- target/riscv/translate.c| 18 +++ target/riscv/insn_trans/trans_rvf.c.inc | 71 +++-- 2 files changed, 73 insertions(+), 16 deletions(-) diff --git a/target/riscv/translate.c b/target/riscv/translate.c index 1290faddda..3919f570f7 100644 --- a/target/riscv/translate.c +++ b/target/riscv/translate.c @@ -101,6 +101,24 @@ static void gen_nanbox_s(TCGv_i64 out, TCGv_i64 in) tcg_gen_ori_i64(out, in, MAKE_64BIT_MASK(32, 32)); } +/* + * A narrow n-bit operation, where n < FLEN, checks that input operands + * are correctly Nan-boxed, i.e., all upper FLEN - n bits are 1. + * If so, the least-significant bits of the input are used, otherwise the + * input value is treated as an n-bit canonical NaN (v2.2 section 9.2). + * + * Here, the result is always nan-boxed, even the canonical nan. + */ +static void gen_check_nanbox_s(TCGv_i64 out, TCGv_i64 in) +{ +TCGv_i64 t_max = tcg_const_i64(0xull); +TCGv_i64 t_nan = tcg_const_i64(0x7fc0ull); + +tcg_gen_movcond_i64(TCG_COND_GEU, out, in, t_max, in, t_nan); +tcg_temp_free_i64(t_max); +tcg_temp_free_i64(t_nan); +} + static void generate_exception(DisasContext *ctx, int excp) { tcg_gen_movi_tl(cpu_pc, ctx->base.pc_next); diff --git a/target/riscv/insn_trans/trans_rvf.c.inc b/target/riscv/insn_trans/trans_rvf.c.inc index 264d3139f1..832f01db6f 100644 --- a/target/riscv/insn_trans/trans_rvf.c.inc +++ b/target/riscv/insn_trans/trans_rvf.c.inc @@ -161,47 +161,86 @@ static bool trans_fsgnj_s(DisasContext *ctx, arg_fsgnj_s *a) { REQUIRE_FPU; REQUIRE_EXT(ctx, RVF); + if (a->rs1 == a->rs2) { /* FMOV */ -tcg_gen_mov_i64(cpu_fpr[a->rd], cpu_fpr[a->rs1]); +gen_check_nanbox_s(cpu_fpr[a->rd], cpu_fpr[a->rs1]); } else { /* FSGNJ */ -tcg_gen_deposit_i64(cpu_fpr[a->rd], cpu_fpr[a->rs2], cpu_fpr[a->rs1], -0, 31); +TCGv_i64 rs1 = tcg_temp_new_i64(); +TCGv_i64 rs2 = tcg_temp_new_i64(); + +gen_check_nanbox_s(rs1, cpu_fpr[a->rs1]); +gen_check_nanbox_s(rs2, cpu_fpr[a->rs2]); + +/* This formulation retains the nanboxing of rs2. */ +tcg_gen_deposit_i64(cpu_fpr[a->rd], rs2, rs1, 0, 31); +tcg_temp_free_i64(rs1); +tcg_temp_free_i64(rs2); } -gen_nanbox_s(cpu_fpr[a->rd], cpu_fpr[a->rd]); mark_fs_dirty(ctx); return true; } static bool trans_fsgnjn_s(DisasContext *ctx, arg_fsgnjn_s *a) { +TCGv_i64 rs1, rs2, mask; + REQUIRE_FPU; REQUIRE_EXT(ctx, RVF); + +rs1 = tcg_temp_new_i64(); +gen_check_nanbox_s(rs1, cpu_fpr[a->rs1]); + if (a->rs1 == a->rs2) { /* FNEG */ -tcg_gen_xori_i64(cpu_fpr[a->rd], cpu_fpr[a->rs1], INT32_MIN); +tcg_gen_xori_i64(cpu_fpr[a->rd], rs1, MAKE_64BIT_MASK(31, 1)); } else { -TCGv_i64 t0 = tcg_temp_new_i64(); -tcg_gen_not_i64(t0, cpu_fpr[a->rs2]); -tcg_gen_deposit_i64(cpu_fpr[a->rd], t0, cpu_fpr[a->rs1], 0, 31); -tcg_temp_free_i64(t0); +rs2 = tcg_temp_new_i64(); +gen_check_nanbox_s(rs2, cpu_fpr[a->rs2]); + +/* + * Replace bit 31 in rs1 with inverse in rs2. + * This formulation retains the nanboxing of rs1. + */ +mask = tcg_const_i64(~MAKE_64BIT_MASK(31, 1)); +tcg_gen_nor_i64(rs2, rs2, mask); +tcg_gen_and_i64(rs1, mask, rs1); +tcg_gen_or_i64(cpu_fpr[a->rd], rs1, rs2); + +tcg_temp_free_i64(mask); +tcg_temp_free_i64(rs2); } -gen_nanbox_s(cpu_fpr[a->rd], cpu_fpr[a->rd]); +tcg_temp_free_i64(rs1); + mark_fs_dirty(ctx); return true; } static bool trans_fsgnjx_s(DisasContext *ctx, arg_fsgnjx_s *a) { +TCGv_i64 rs1, rs2; + REQUIRE_FPU; REQUIRE_EXT(ctx, RVF); + +rs1 = tcg_temp_new_i64(); +gen_check_nanbox_s(rs1, cpu_fpr[a->rs1]); + if (a->rs1 == a->rs2) { /* FABS */ -tcg_gen_andi_i64(cpu_fpr[a->rd], cpu_fpr[a->rs1], ~INT32_MIN); +tcg_gen_andi_i64(cpu_fpr[a->rd], rs1, ~MAKE_64BIT_MASK(31, 1)); } else { -TCGv_i64 t0 = tcg_temp_new_i64(); -tcg_gen_andi_i64(t0, cpu_fpr[a->rs2], INT32_MIN); -tcg_gen_xor_i64(cpu_fpr[a->rd], cpu_fpr[a->rs1], t0); -tcg_temp_free_i64(t0); +rs2 = tcg_temp_new_i64(); +gen_check_nanbox_s(rs2, cpu_fpr[a->rs2]); + +/* + * Xor bit 31 in rs1 with that in rs2. + * This formulation retains the nanboxing of rs1. + */ +tcg_gen_andi_i64(rs2, rs2, MAKE_64BIT_MASK(31, 1)); +tcg_gen_xor_i64(cpu_fpr[a->rd], rs1,
[PATCH v1 1/1] core/register: Specify instance_size in the TypeInfo
Reported-by: Eduardo Habkost Signed-off-by: Alistair Francis --- hw/core/register.c | 14 +- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/hw/core/register.c b/hw/core/register.c index ddf91eb445..5e8e8199d0 100644 --- a/hw/core/register.c +++ b/hw/core/register.c @@ -180,11 +180,7 @@ void register_init(RegisterInfo *reg) { assert(reg); -if (!reg->data || !reg->access) { -return; -} -object_initialize((void *)reg, sizeof(*reg), TYPE_REGISTER); } void register_write_memory(void *opaque, hwaddr addr, @@ -269,13 +265,20 @@ static RegisterInfoArray *register_init_block(DeviceState *owner, int index = rae[i].addr / data_size; RegisterInfo *r = &ri[index]; +if (data + data_size * index == 0 || !&rae[i]) { +continue; +} + +/* Init the register, this will zero it. */ +object_initialize((void *)r, sizeof(*r), TYPE_REGISTER); + +/* Set the properties of the register */ *r = (RegisterInfo) { .data = data + data_size * index, .data_size = data_size, .access = &rae[i], .opaque = owner, }; -register_init(r); r_array->r[i] = r; } @@ -329,6 +332,7 @@ static const TypeInfo register_info = { .name = TYPE_REGISTER, .parent = TYPE_DEVICE, .class_init = register_class_init, +.instance_size = sizeof(RegisterInfo), }; static void register_register_types(void) -- 2.28.0
Re: [PATCH v8 1/1] audio/jack: fix use after free segfault
On 2020-08-22 03:47, Paolo Bonzini wrote: On 21/08/20 19:34, Christian Schoenebeck wrote: static void qjack_fini_out(HWVoiceOut *hw) { QJackOut *jo = (QJackOut *)hw; qjack_client_fini(&jo->c); + +qemu_bh_delete(jo->c.shutdown_bh); Paolo wrapped that qemu_bh_delete() call inside the lock as well. So I guess it makes a difference for the BH API? It is not a problem as long as qjack_client_fini is idempotent. `qjack_client_fini` is indeed idempotent +qemu_mutex_destroy(&jo->c.shutdown_lock); } Hmmm, is this qemu_mutex_destroy() safe at this point? Perhaps make the mutex global and not destroy it at all. It's safe at this point as `qjack_fini_out` is only called at device destruction, and `qjack_client_fini` ensures that JACK is shut down which prevents jack from trying to call the shutdown event handler. Paolo
[Bug 1883984] Autopkgtest regression report (qemu/1:4.2-3ubuntu6.5)
All autopkgtests for the newly accepted qemu (1:4.2-3ubuntu6.5) for focal have finished running. The following regressions have been reported in tests triggered by the package: ubuntu-image/1.9+20.04ubuntu1 (amd64) systemd/245.4-4ubuntu3.2 (amd64, armhf, s390x, ppc64el) livecd-rootfs/2.664.4 (amd64, arm64, s390x, ppc64el) Please visit the excuses page listed below and investigate the failures, proceeding afterwards as per the StableReleaseUpdates policy regarding autopkgtest regressions [1]. https://people.canonical.com/~ubuntu-archive/proposed- migration/focal/update_excuses.html#qemu [1] https://wiki.ubuntu.com/StableReleaseUpdates#Autopkgtest_Regressions Thank you! -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1883984 Title: QEMU S/390x sqxbr (128-bit IEEE 754 square root) crashes qemu-system- s390x Status in QEMU: Fix Released Status in qemu package in Ubuntu: Fix Released Status in qemu source package in Focal: Fix Committed Bug description: [Impact] * An instruction was described wrong so that on usage the program would crash. [Test Case] * Run s390x in emulation and there use this program: For simplicity and speed you can use KVM guest as usual on s390x, that after prep&install&compile of the test you run in qemu-tcg like: $ sudo qemu-system-s390x -machine s390-ccw-virtio,accel=tcg -cpu max,zpci=on -serial mon:stdio -display none -m 4096 -nic user,model=virtio,hostfwd=tcp::-:22 -drive file=/var/lib/uvtool/libvirt/images/focal-sqxbr.qcow,if=none,id=drive-virtio-disk0,format=qcow2,cache=none -device virtio-blk-ccw,devno=fe.0.0001,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1,scsi=off Obviously is you have no s390x access you need to use emulation right away. * Build and run failing program $ sudo apt install clang $ cat > bug-sqrtl-one-line.c << EOF int main(void) { volatile long double x, r; x = 4.0L; __asm__ __volatile__("sqxbr %0, %1" : "=f" (r) : "f" (x)); return (0);} EOF $ cc bug-sqrtl-one-line.c $ ./a.out Segmentation fault (core dumped) qemu is dead by now as long as the bug is present [Regression Potential] * The change only modifies 128 bit square root on s390x so regressions should be limited to exactly that - which formerly before this fix was a broken instruction. [Other Info] * n/a --- In porting software to guest Ubuntu 18.04 and 20.04 VMs for S/390x, I discovered that some of my own numerical programs, and also a GNU configure script for at least one package with CC=clang, would cause an instant crash of the VM, sometimes also destroying recently opened files, and producing long strings of NUL characters in /var/log/syslog in the S/390 guest O/S. Further detective work narrowed the cause of the crash down to a single IBM S/390 instruction: sqxbr (128-bit IEEE 754 square root). Here is a one-line program that when compiled and run on a VM hosted on QEMUcc emulator version 4.2.0 (Debian 1:4.2-3ubuntu6.1) [hosted on Ubuntu 20.04 on a Dell Precision 7920 workstation with an Intel Xeon Platinum 8253 CPU], and also on QEMU emulator version 5.0.0, reproducibly produces a VM crash under qemu-system-s390x. % cat bug-sqrtl-one-line.c int main(void) { volatile long double x, r; x = 4.0L; __asm__ __volatile__("sqxbr %0, %1" : "=f" (r) : "f" (x)); return (0);} % cc bug-sqrtl-one-line.c && ./a.out Segmentation fault (core dumped) The problem code may be the function float128_sqrt() defined in qemu-5.0.0/fpu/softfloat.c starting at line 7619. I have NOT attempted to run the qemu-system-s390x executable under a debugger. However, I observe that S/390 is the only CPU family that I know of, except possibly for a Fujitsu SPARC-64, that has a 128-bit square root in hardware. Thus, this instruction bug may not have been seen before. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1883984/+subscriptions
[PATCH] linux-user: warn if trying to use qemu-mipsn32[el] with non n32 ELF
While technically compatible will (depending on the CPU) hopefully fail later with a cryptic error: qemu: Unexpected FPU mode Provide an earlier hint of what the problem might be by detecting if the binary might not be using the n32 ABI and print a warning. Signed-off-by: Carlo Marcelo Arenas Belón --- linux-user/elfload.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/linux-user/elfload.c b/linux-user/elfload.c index fe9dfe795d..64c3921cd9 100644 --- a/linux-user/elfload.c +++ b/linux-user/elfload.c @@ -2390,6 +2390,13 @@ static void load_elf_image(const char *image_name, int image_fd, if (!elf_check_ehdr(ehdr)) { goto exit_errmsg; } +#ifdef TARGET_ABI_MIPSN32 +/* from arch/mips/include/asm/elf.h */ +#define EF_MIPS_ABI2 0x0020 +if (!(ehdr->e_flags & EF_MIPS_ABI2)) { +fprintf(stderr, "warning: ELF binary missing n32 flag\n"); +} +#endif i = ehdr->e_phnum * sizeof(struct elf_phdr); if (ehdr->e_phoff + i <= BPRM_BUF_SIZE) { -- 2.28.0.213.gdd9653da77
[PATCH v5 0/8] Remove EPYC mode apicid decode and use generic decode
To support some of the complex topology, we introduced EPYC mode apicid decode. But, EPYC mode decode is running into problems. Also it can become quite a maintenance problem in the future. So, it was decided to remove that code and use the generic decode which works for majority of the topology. Most of the SPECed configuration would work just fine. With some non-SPECed user inputs, it will create some sub-optimal configuration. Here is the discussion thread. https://lore.kernel.org/qemu-devel/c0bcc1a6-1d84-a6e7-e468-d5b437c1b...@amd.com/ This series removes all the EPYC mode specific apicid changes and use the generic apicid decode. --- v5: Revert EPYC specific decode. Simplify CPUID_8000_001E v4: https://lore.kernel.org/qemu-devel/159744083536.39197.13827776633866601278.st...@naples-babu.amd.com/ Not much of a change. Just added few text changes. Error out configuration instead of warning if dies are not configured in EPYC. Few other text changes to clarify the removal of node_id, nr_nodes and nodes_per_pkg. v3: https://lore.kernel.org/qemu-devel/159681772267.9679.1334429994189974662.st...@naples-babu.amd.com/#r Added a new check to pass the dies for EPYC numa configuration. Added Simplify CPUID_8000_001E patch with some changes suggested by Igor. Dropped the patch to build the topology from CpuInstanceProperties. TODO: Not sure if we still need the Autonuma changes Igor mentioned. Needs more clarity on that. v2: https://lore.kernel.org/qemu-devel/159362436285.36204.986406297373871949.st...@naples-babu.amd.com/ Used the numa information from CpuInstanceProperties for building the apic_id suggested by Igor. Also did some minor code re-aarangement to take care of changes. Dropped the patch "Simplify CPUID_8000_001E" from v1. Will send it later. v1: https://lore.kernel.org/qemu-devel/159164739269.20543.3074052993891532749.st...@naples-babu.amd.com Babu Moger (8): hw/i386: Remove node_id, nr_nodes and nodes_per_pkg from topology Revert "i386: Fix pkg_id offset for EPYC cpu models" Revert "target/i386: Enable new apic id encoding for EPYC based cpus models" Revert "hw/i386: Move arch_id decode inside x86_cpus_init" Revert "i386: Introduce use_epyc_apic_id_encoding in X86CPUDefinition" Revert "hw/i386: Introduce apicid functions inside X86MachineState" Revert "hw/386: Add EPYC mode topology decoding functions" i386: Simplify CPUID_8000_001E for AMD hw/i386/pc.c |8 +-- hw/i386/x86.c | 43 +++- include/hw/i386/topology.h | 101 --- include/hw/i386/x86.h |9 --- target/i386/cpu.c | 115 target/i386/cpu.h |3 - tests/test-x86-cpuid.c | 40 --- 7 files changed, 73 insertions(+), 246 deletions(-) -- Signature
[PATCH v5 3/8] Revert "target/i386: Enable new apic id encoding for EPYC based cpus models"
Remove the EPYC specific apicid decoding and use the generic default decoding. This reverts commit 247b18c593ec298446645af8d5d28911daf653b1. Signed-off-by: Babu Moger --- target/i386/cpu.c |2 -- 1 file changed, 2 deletions(-) diff --git a/target/i386/cpu.c b/target/i386/cpu.c index 83acbce3e9..567d864051 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -3993,7 +3993,6 @@ static X86CPUDefinition builtin_x86_defs[] = { .xlevel = 0x801E, .model_id = "AMD EPYC Processor", .cache_info = &epyc_cache_info, -.use_epyc_apic_id_encoding = 1, .versions = (X86CPUVersionDefinition[]) { { .version = 1 }, { @@ -4121,7 +4120,6 @@ static X86CPUDefinition builtin_x86_defs[] = { .xlevel = 0x801E, .model_id = "AMD EPYC-Rome Processor", .cache_info = &epyc_rome_cache_info, -.use_epyc_apic_id_encoding = 1, }, };
[PATCH v5 8/8] i386: Simplify CPUID_8000_001E for AMD
apic_id contains all the information required to build CPUID_8000_001E. core_id and node_id is already part of apic_id generated by x86_topo_ids_from_apicid_epyc. Also remove the restriction on number bits on core_id and node_id. Remove all the hardcoded values and replace with generalized fields. Refer the Processor Programming Reference (PPR) documentation available from the bugzilla Link below. Link: https://bugzilla.kernel.org/show_bug.cgi?id=206537 Signed-off-by: Babu Moger --- target/i386/cpu.c | 81 - 1 file changed, 37 insertions(+), 44 deletions(-) diff --git a/target/i386/cpu.c b/target/i386/cpu.c index b29686220e..bea2822923 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -385,58 +385,51 @@ static void encode_topo_cpuid801e(X86CPUTopoInfo *topo_info, X86CPU *cpu, uint32_t *ecx, uint32_t *edx) { X86CPUTopoIDs topo_ids = {0}; -unsigned long dies = topo_info->dies_per_pkg; -int shift; x86_topo_ids_from_apicid(cpu->apic_id, topo_info, &topo_ids); *eax = cpu->apic_id; + /* - * CPUID_Fn801E_EBX - * 31:16 Reserved - * 15:8 Threads per core (The number of threads per core is - * Threads per core + 1) - * 7:0 Core id (see bit decoding below) - * SMT: - * 4:3 node id - * 2 Core complex id - * 1:0 Core id - * Non SMT: - * 5:4 node id - * 3 Core complex id - * 1:0 Core id + * CPUID_Fn801E_EBX [Core Identifiers] (CoreId) + * Read-only. Reset: _h. + * See Core::X86::Cpuid::ExtApicId. + * Core::X86::Cpuid::CoreId_lthree[1:0]_core[3:0]_thread[1:0]; + * Bits Description + * 31:16 Reserved. + * 15:8 ThreadsPerCore: threads per core. Read-only. Reset: XXh. + * The number of threads per core is ThreadsPerCore+1. + * 7:0 CoreId: core ID. Read-only. Reset: XXh. + * + * NOTE: CoreId is already part of apic_id. Just use it. We can + * use all the 8 bits to represent the core_id here. */ -*ebx = ((topo_info->threads_per_core - 1) << 8) | (topo_ids.die_id << 3) | -(topo_ids.core_id); +*ebx = ((topo_info->threads_per_core - 1) << 8) | (topo_ids.core_id & 0xFF); + /* - * CPUID_Fn801E_ECX - * 31:11 Reserved - * 10:8 Nodes per processor (Nodes per processor is number of nodes + 1) - * 7:0 Node id (see bit decoding below) - * 2 Socket id - * 1:0 Node id + * CPUID_Fn801E_ECX [Node Identifiers] (NodeId) + * Read-only. Reset: _0XXXh. + * Core::X86::Cpuid::NodeId_lthree[1:0]_core[3:0]_thread[1:0]; + * Bits Description + * 31:11 Reserved. + * 10:8 NodesPerProcessor: Node per processor. Read-only. Reset: XXXb. + * ValidValues: + * Value Description + * 000b 1 node per processor. + * 001b 2 nodes per processor. + * 010b Reserved. + * 011b 4 nodes per processor. + * 111b-100b Reserved. + * 7:0 NodeId: Node ID. Read-only. Reset: XXh. + * + * NOTE: Hardware reserves 3 bits for number of nodes per processor. + * But users can create more nodes than the actual hardware can + * support. To genaralize we can use all the upper 8 bits for nodes. + * NodeId is combination of node and socket_id which is already decoded + * in apic_id. Just use it by shifting. */ -if (dies <= 4) { -*ecx = ((dies - 1) << 8) | (topo_ids.pkg_id << 2) | topo_ids.die_id; -} else { -/* - * Node id fix up. Actual hardware supports up to 4 nodes. But with - * more than 32 cores, we may end up with more than 4 nodes. - * Node id is a combination of socket id and node id. Only requirement - * here is that this number should be unique accross the system. - * Shift the socket id to accommodate more nodes. We dont expect both - * socket id and node id to be big number at the same time. This is not - * an ideal config but we need to to support it. Max nodes we can have - * is 32 (255/8) with 8 cores per node and 255 max cores. We only need - * 5 bits for nodes. Find the left most set bit to represent the total - * number of nodes. find_last_bit returns last set bit(0 based). Left - * shift(+1) the socket id to represent all the nodes. - */ -dies -= 1; -shift = find_last_bit(&dies, 8); -*ecx = (dies << 8) | (topo_ids.pkg_id << (shift + 1)) | - topo_ids.die_id; -} +*ecx = ((topo_info->dies_per_pkg - 1) << 8) | + ((cpu->apic_id >> apicid_die_offset(topo_info)) & 0xFF); *edx = 0; }
[PATCH v5 7/8] Revert "hw/386: Add EPYC mode topology decoding functions"
Remove the EPYC specific apicid decoding and use the generic default decoding. This reverts commit 7568b20a6405042f62c64af3268f4330aed5. Signed-off-by: Babu Moger --- include/hw/i386/topology.h | 79 target/i386/cpu.c |2 + 2 files changed, 1 insertion(+), 80 deletions(-) diff --git a/include/hw/i386/topology.h b/include/hw/i386/topology.h index 05ddde7ba0..81573f6cfd 100644 --- a/include/hw/i386/topology.h +++ b/include/hw/i386/topology.h @@ -107,85 +107,6 @@ static inline unsigned apicid_pkg_offset(X86CPUTopoInfo *topo_info) return apicid_die_offset(topo_info) + apicid_die_width(topo_info); } -#define EPYC_DIE_OFFSET 3 /* Minimum die_id offset if numa configured */ - -/* - * Bit offset of the die_id field - */ -static inline unsigned apicid_die_offset_epyc(X86CPUTopoInfo *topo_info) -{ -unsigned offset = apicid_core_offset(topo_info) + - apicid_core_width(topo_info); - -return MAX(EPYC_DIE_OFFSET, offset); -} - -/* Bit offset of the Pkg_ID (socket ID) field */ -static inline unsigned apicid_pkg_offset_epyc(X86CPUTopoInfo *topo_info) -{ -return apicid_die_offset_epyc(topo_info) + apicid_die_width(topo_info); -} - -/* - * Make APIC ID for the CPU based on Pkg_ID, Core_ID, SMT_ID - * - * The caller must make sure core_id < nr_cores and smt_id < nr_threads. - */ -static inline apic_id_t -x86_apicid_from_topo_ids_epyc(X86CPUTopoInfo *topo_info, - const X86CPUTopoIDs *topo_ids) -{ -return (topo_ids->pkg_id << apicid_pkg_offset_epyc(topo_info)) | - (topo_ids->die_id << apicid_die_offset_epyc(topo_info)) | - (topo_ids->core_id << apicid_core_offset(topo_info)) | - topo_ids->smt_id; -} - -static inline void x86_topo_ids_from_idx_epyc(X86CPUTopoInfo *topo_info, - unsigned cpu_index, - X86CPUTopoIDs *topo_ids) -{ -unsigned nr_dies = topo_info->dies_per_pkg; -unsigned nr_cores = topo_info->cores_per_die; -unsigned nr_threads = topo_info->threads_per_core; - -topo_ids->pkg_id = cpu_index / (nr_dies * nr_cores * nr_threads); -topo_ids->die_id = cpu_index / (nr_cores * nr_threads) % nr_dies; -topo_ids->core_id = cpu_index / nr_threads % nr_cores; -topo_ids->smt_id = cpu_index % nr_threads; -} - -/* - * Calculate thread/core/package IDs for a specific topology, - * based on APIC ID - */ -static inline void x86_topo_ids_from_apicid_epyc(apic_id_t apicid, -X86CPUTopoInfo *topo_info, -X86CPUTopoIDs *topo_ids) -{ -topo_ids->smt_id = apicid & -~(0xUL << apicid_smt_width(topo_info)); -topo_ids->core_id = -(apicid >> apicid_core_offset(topo_info)) & -~(0xUL << apicid_core_width(topo_info)); -topo_ids->die_id = -(apicid >> apicid_die_offset_epyc(topo_info)) & -~(0xUL << apicid_die_width(topo_info)); -topo_ids->pkg_id = apicid >> apicid_pkg_offset_epyc(topo_info); -} - -/* - * Make APIC ID for the CPU 'cpu_index' - * - * 'cpu_index' is a sequential, contiguous ID for the CPU. - */ -static inline apic_id_t x86_apicid_from_cpu_idx_epyc(X86CPUTopoInfo *topo_info, - unsigned cpu_index) -{ -X86CPUTopoIDs topo_ids; -x86_topo_ids_from_idx_epyc(topo_info, cpu_index, &topo_ids); -return x86_apicid_from_topo_ids_epyc(topo_info, &topo_ids); -} /* Make APIC ID for the CPU based on Pkg_ID, Core_ID, SMT_ID * * The caller must make sure core_id < nr_cores and smt_id < nr_threads. diff --git a/target/i386/cpu.c b/target/i386/cpu.c index 19198e3e7f..b29686220e 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -388,7 +388,7 @@ static void encode_topo_cpuid801e(X86CPUTopoInfo *topo_info, X86CPU *cpu, unsigned long dies = topo_info->dies_per_pkg; int shift; -x86_topo_ids_from_apicid_epyc(cpu->apic_id, topo_info, &topo_ids); +x86_topo_ids_from_apicid(cpu->apic_id, topo_info, &topo_ids); *eax = cpu->apic_id; /*
[PATCH v5 2/8] Revert "i386: Fix pkg_id offset for EPYC cpu models"
Remove the EPYC specific apicid decoding and use the generic default decoding. This reverts commit 7b225762c8c05fd31d4c2be116aedfbc00383f8b. Signed-off-by: Babu Moger --- hw/i386/pc.c |1 - target/i386/cpu.c |6 +++--- target/i386/cpu.h |1 - 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 0ae5cb3af4..e74b3cb1d8 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1498,7 +1498,6 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev, init_topo_info(&topo_info, x86ms); env->nr_dies = x86ms->smp_dies; -env->pkg_offset = x86ms->apicid_pkg_offset(&topo_info); /* * If APIC ID is not set, diff --git a/target/i386/cpu.c b/target/i386/cpu.c index 3c58af1f43..83acbce3e9 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -5675,7 +5675,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, *ecx |= CPUID_TOPOLOGY_LEVEL_SMT; break; case 1: -*eax = env->pkg_offset; +*eax = apicid_pkg_offset(&topo_info); *ebx = cs->nr_cores * cs->nr_threads; *ecx |= CPUID_TOPOLOGY_LEVEL_CORE; break; @@ -5709,7 +5709,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, *ecx |= CPUID_TOPOLOGY_LEVEL_CORE; break; case 2: -*eax = env->pkg_offset; +*eax = apicid_pkg_offset(&topo_info); *ebx = env->nr_dies * cs->nr_cores * cs->nr_threads; *ecx |= CPUID_TOPOLOGY_LEVEL_DIE; break; @@ -5890,7 +5890,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, * CPUX86State::pkg_offset. * Bits 7:0 is "The number of threads in the package is NC+1" */ -*ecx = (env->pkg_offset << 12) | +*ecx = (apicid_pkg_offset(&topo_info) << 12) | ((cs->nr_cores * cs->nr_threads) - 1); } else { *ecx = 0; diff --git a/target/i386/cpu.h b/target/i386/cpu.h index 4c89bee8d1..a345172afd 100644 --- a/target/i386/cpu.h +++ b/target/i386/cpu.h @@ -1629,7 +1629,6 @@ typedef struct CPUX86State { TPRAccess tpr_access_type; unsigned nr_dies; -unsigned pkg_offset; } CPUX86State; struct kvm_msrs;
[PATCH v5 5/8] Revert "i386: Introduce use_epyc_apic_id_encoding in X86CPUDefinition"
Remove the EPYC specific apicid decoding and use the generic default decoding. This reverts commit 0c1538cb1a26287c072645f4759b9872b1596d79. Signed-off-by: Babu Moger --- target/i386/cpu.c | 16 target/i386/cpu.h |1 - 2 files changed, 17 deletions(-) diff --git a/target/i386/cpu.c b/target/i386/cpu.c index 567d864051..19198e3e7f 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -1636,10 +1636,6 @@ typedef struct X86CPUDefinition { FeatureWordArray features; const char *model_id; CPUCaches *cache_info; - -/* Use AMD EPYC encoding for apic id */ -bool use_epyc_apic_id_encoding; - /* * Definitions for alternative versions of CPU model. * List is terminated by item with version == 0. @@ -1681,18 +1677,6 @@ static const X86CPUVersionDefinition *x86_cpu_def_get_versions(X86CPUDefinition return def->versions ?: default_version_list; } -bool cpu_x86_use_epyc_apic_id_encoding(const char *cpu_type) -{ -X86CPUClass *xcc = X86_CPU_CLASS(object_class_by_name(cpu_type)); - -assert(xcc); -if (xcc->model && xcc->model->cpudef) { -return xcc->model->cpudef->use_epyc_apic_id_encoding; -} else { -return false; -} -} - static CPUCaches epyc_cache_info = { .l1d_cache = &(CPUCacheInfo) { .type = DATA_CACHE, diff --git a/target/i386/cpu.h b/target/i386/cpu.h index a345172afd..d3097be6a5 100644 --- a/target/i386/cpu.h +++ b/target/i386/cpu.h @@ -1917,7 +1917,6 @@ void cpu_clear_apic_feature(CPUX86State *env); void host_cpuid(uint32_t function, uint32_t count, uint32_t *eax, uint32_t *ebx, uint32_t *ecx, uint32_t *edx); void host_vendor_fms(char *vendor, int *family, int *model, int *stepping); -bool cpu_x86_use_epyc_apic_id_encoding(const char *cpu_type); /* helper.c */ bool x86_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
[PATCH v5 4/8] Revert "hw/i386: Move arch_id decode inside x86_cpus_init"
Remove the EPYC specific apicid decoding and use the generic default decoding. This reverts commit 2e26f4ab3bf8390a2677d3afd9b1a04f015d7721. Signed-off-by: Babu Moger --- hw/i386/pc.c |6 +++--- hw/i386/x86.c | 37 +++-- 2 files changed, 10 insertions(+), 33 deletions(-) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index e74b3cb1d8..6abe7723e0 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1552,14 +1552,14 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev, topo_ids.die_id = cpu->die_id; topo_ids.core_id = cpu->core_id; topo_ids.smt_id = cpu->thread_id; -cpu->apic_id = x86ms->apicid_from_topo_ids(&topo_info, &topo_ids); +cpu->apic_id = x86_apicid_from_topo_ids(&topo_info, &topo_ids); } cpu_slot = pc_find_cpu_slot(MACHINE(pcms), cpu->apic_id, &idx); if (!cpu_slot) { MachineState *ms = MACHINE(pcms); -x86ms->topo_ids_from_apicid(cpu->apic_id, &topo_info, &topo_ids); +x86_topo_ids_from_apicid(cpu->apic_id, &topo_info, &topo_ids); error_setg(errp, "Invalid CPU [socket: %u, die: %u, core: %u, thread: %u] with" " APIC ID %" PRIu32 ", valid index range 0:%d", @@ -1580,7 +1580,7 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev, /* TODO: move socket_id/core_id/thread_id checks into x86_cpu_realizefn() * once -smp refactoring is complete and there will be CPU private * CPUState::nr_cores and CPUState::nr_threads fields instead of globals */ -x86ms->topo_ids_from_apicid(cpu->apic_id, &topo_info, &topo_ids); +x86_topo_ids_from_apicid(cpu->apic_id, &topo_info, &topo_ids); if (cpu->socket_id != -1 && cpu->socket_id != topo_ids.pkg_id) { error_setg(errp, "property socket-id: %u doesn't match set apic-id:" " 0x%x (socket-id: %u)", cpu->socket_id, cpu->apic_id, diff --git a/hw/i386/x86.c b/hw/i386/x86.c index f6eab947df..0a7cf8336c 100644 --- a/hw/i386/x86.c +++ b/hw/i386/x86.c @@ -67,22 +67,6 @@ inline void init_topo_info(X86CPUTopoInfo *topo_info, topo_info->threads_per_core = ms->smp.threads; } -/* - * Set up with the new EPYC topology handlers - * - * AMD uses different apic id encoding for EPYC based cpus. Override - * the default topo handlers with EPYC encoding handlers. - */ -static void x86_set_epyc_topo_handlers(MachineState *machine) -{ -X86MachineState *x86ms = X86_MACHINE(machine); - -x86ms->apicid_from_cpu_idx = x86_apicid_from_cpu_idx_epyc; -x86ms->topo_ids_from_apicid = x86_topo_ids_from_apicid_epyc; -x86ms->apicid_from_topo_ids = x86_apicid_from_topo_ids_epyc; -x86ms->apicid_pkg_offset = apicid_pkg_offset_epyc; -} - /* * Calculates initial APIC ID for a specific CPU index * @@ -101,7 +85,7 @@ uint32_t x86_cpu_apic_id_from_index(X86MachineState *x86ms, init_topo_info(&topo_info, x86ms); -correct_id = x86ms->apicid_from_cpu_idx(&topo_info, cpu_index); +correct_id = x86_apicid_from_cpu_idx(&topo_info, cpu_index); if (x86mc->compat_apic_id_mode) { if (cpu_index != correct_id && !warned && !qtest_enabled()) { error_report("APIC IDs set in compatibility mode, " @@ -135,11 +119,6 @@ void x86_cpus_init(X86MachineState *x86ms, int default_cpu_version) MachineState *ms = MACHINE(x86ms); MachineClass *mc = MACHINE_GET_CLASS(x86ms); -/* Check for apicid encoding */ -if (cpu_x86_use_epyc_apic_id_encoding(ms->cpu_type)) { -x86_set_epyc_topo_handlers(ms); -} - x86_cpu_set_default_version(default_cpu_version); /* @@ -153,12 +132,6 @@ void x86_cpus_init(X86MachineState *x86ms, int default_cpu_version) x86ms->apic_id_limit = x86_cpu_apic_id_from_index(x86ms, ms->smp.max_cpus - 1) + 1; possible_cpus = mc->possible_cpu_arch_ids(ms); - -for (i = 0; i < ms->possible_cpus->len; i++) { -ms->possible_cpus->cpus[i].arch_id = -x86_cpu_apic_id_from_index(x86ms, i); -} - for (i = 0; i < ms->smp.cpus; i++) { x86_cpu_new(x86ms, possible_cpus->cpus[i].arch_id, &error_fatal); } @@ -183,7 +156,8 @@ int64_t x86_get_default_cpu_node_id(const MachineState *ms, int idx) init_topo_info(&topo_info, x86ms); assert(idx < ms->possible_cpus->len); - x86_topo_ids_from_idx(&topo_info, idx, &topo_ids); + x86_topo_ids_from_apicid(ms->possible_cpus->cpus[idx].arch_id, +&topo_info, &topo_ids); return topo_ids.pkg_id % ms->numa_state->num_nodes; } @@ -214,7 +188,10 @@ const CPUArchIdList *x86_possible_cpu_arch_ids(MachineState *ms) ms->possible_cpus->cpus[i].type = ms->cpu_type; ms->possible_cpus->cpus[i].vcpus_count = 1; -x86_topo_ids_from_idx(&topo_info, i, &topo_ids); +ms->possible_cpus->cpus[i].arch_id = +x86_cpu_apic_id_from_index(x86ms, i); +x86_topo_ids_from_apicid(ms->possible_cpus-
[PATCH v5 6/8] Revert "hw/i386: Introduce apicid functions inside X86MachineState"
Remove the EPYC specific apicid decoding and use the generic default decoding. This reverts commit 6121c7fbfd98dbc3af1b00b56ff2eef66df87828. Signed-off-by: Babu Moger --- hw/i386/x86.c |5 - include/hw/i386/x86.h |9 - 2 files changed, 14 deletions(-) diff --git a/hw/i386/x86.c b/hw/i386/x86.c index 0a7cf8336c..c9dba0811a 100644 --- a/hw/i386/x86.c +++ b/hw/i386/x86.c @@ -895,11 +895,6 @@ static void x86_machine_initfn(Object *obj) x86ms->smm = ON_OFF_AUTO_AUTO; x86ms->acpi = ON_OFF_AUTO_AUTO; x86ms->smp_dies = 1; - -x86ms->apicid_from_cpu_idx = x86_apicid_from_cpu_idx; -x86ms->topo_ids_from_apicid = x86_topo_ids_from_apicid; -x86ms->apicid_from_topo_ids = x86_apicid_from_topo_ids; -x86ms->apicid_pkg_offset = apicid_pkg_offset; } static void x86_machine_class_init(ObjectClass *oc, void *data) diff --git a/include/hw/i386/x86.h b/include/hw/i386/x86.h index b79f24e285..4d9a26326d 100644 --- a/include/hw/i386/x86.h +++ b/include/hw/i386/x86.h @@ -63,15 +63,6 @@ typedef struct { OnOffAuto smm; OnOffAuto acpi; -/* Apic id specific handlers */ -uint32_t (*apicid_from_cpu_idx)(X86CPUTopoInfo *topo_info, -unsigned cpu_index); -void (*topo_ids_from_apicid)(apic_id_t apicid, X86CPUTopoInfo *topo_info, - X86CPUTopoIDs *topo_ids); -apic_id_t (*apicid_from_topo_ids)(X86CPUTopoInfo *topo_info, - const X86CPUTopoIDs *topo_ids); -uint32_t (*apicid_pkg_offset)(X86CPUTopoInfo *topo_info); - /* * Address space used by IOAPIC device. All IOAPIC interrupts * will be translated to MSI messages in the address space.
[PATCH v5 1/8] hw/i386: Remove node_id, nr_nodes and nodes_per_pkg from topology
Remove node_id, nr_nodes and nodes_per_pkg from topology. Use die_id, nr_dies and dies_per_pkg which is already available. Removes the confusion over two variables. With node_id removed in topology the uninitialized memory issue with -device and CPU hotplug will be fixed. Link: https://bugzilla.redhat.com/show_bug.cgi?id=1828750 Signed-off-by: Babu Moger --- hw/i386/pc.c |1 - hw/i386/x86.c |1 - include/hw/i386/topology.h | 40 +--- target/i386/cpu.c | 24 ++-- target/i386/cpu.h |1 - tests/test-x86-cpuid.c | 40 6 files changed, 39 insertions(+), 68 deletions(-) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 47c5ca3e34..0ae5cb3af4 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1498,7 +1498,6 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev, init_topo_info(&topo_info, x86ms); env->nr_dies = x86ms->smp_dies; -env->nr_nodes = topo_info.nodes_per_pkg; env->pkg_offset = x86ms->apicid_pkg_offset(&topo_info); /* diff --git a/hw/i386/x86.c b/hw/i386/x86.c index 67bee1bcb8..f6eab947df 100644 --- a/hw/i386/x86.c +++ b/hw/i386/x86.c @@ -62,7 +62,6 @@ inline void init_topo_info(X86CPUTopoInfo *topo_info, { MachineState *ms = MACHINE(x86ms); -topo_info->nodes_per_pkg = ms->numa_state->num_nodes / ms->smp.sockets; topo_info->dies_per_pkg = x86ms->smp_dies; topo_info->cores_per_die = ms->smp.cores; topo_info->threads_per_core = ms->smp.threads; diff --git a/include/hw/i386/topology.h b/include/hw/i386/topology.h index 07239f95f4..05ddde7ba0 100644 --- a/include/hw/i386/topology.h +++ b/include/hw/i386/topology.h @@ -47,14 +47,12 @@ typedef uint32_t apic_id_t; typedef struct X86CPUTopoIDs { unsigned pkg_id; -unsigned node_id; unsigned die_id; unsigned core_id; unsigned smt_id; } X86CPUTopoIDs; typedef struct X86CPUTopoInfo { -unsigned nodes_per_pkg; unsigned dies_per_pkg; unsigned cores_per_die; unsigned threads_per_core; @@ -89,11 +87,6 @@ static inline unsigned apicid_die_width(X86CPUTopoInfo *topo_info) return apicid_bitwidth_for_count(topo_info->dies_per_pkg); } -/* Bit width of the node_id field per socket */ -static inline unsigned apicid_node_width_epyc(X86CPUTopoInfo *topo_info) -{ -return apicid_bitwidth_for_count(MAX(topo_info->nodes_per_pkg, 1)); -} /* Bit offset of the Core_ID field */ static inline unsigned apicid_core_offset(X86CPUTopoInfo *topo_info) @@ -114,30 +107,23 @@ static inline unsigned apicid_pkg_offset(X86CPUTopoInfo *topo_info) return apicid_die_offset(topo_info) + apicid_die_width(topo_info); } -#define NODE_ID_OFFSET 3 /* Minimum node_id offset if numa configured */ +#define EPYC_DIE_OFFSET 3 /* Minimum die_id offset if numa configured */ /* - * Bit offset of the node_id field - * - * Make sure nodes_per_pkg > 0 if numa configured else zero. + * Bit offset of the die_id field */ -static inline unsigned apicid_node_offset_epyc(X86CPUTopoInfo *topo_info) +static inline unsigned apicid_die_offset_epyc(X86CPUTopoInfo *topo_info) { -unsigned offset = apicid_die_offset(topo_info) + - apicid_die_width(topo_info); +unsigned offset = apicid_core_offset(topo_info) + + apicid_core_width(topo_info); -if (topo_info->nodes_per_pkg) { -return MAX(NODE_ID_OFFSET, offset); -} else { -return offset; -} +return MAX(EPYC_DIE_OFFSET, offset); } /* Bit offset of the Pkg_ID (socket ID) field */ static inline unsigned apicid_pkg_offset_epyc(X86CPUTopoInfo *topo_info) { -return apicid_node_offset_epyc(topo_info) + - apicid_node_width_epyc(topo_info); +return apicid_die_offset_epyc(topo_info) + apicid_die_width(topo_info); } /* @@ -150,8 +136,7 @@ x86_apicid_from_topo_ids_epyc(X86CPUTopoInfo *topo_info, const X86CPUTopoIDs *topo_ids) { return (topo_ids->pkg_id << apicid_pkg_offset_epyc(topo_info)) | - (topo_ids->node_id << apicid_node_offset_epyc(topo_info)) | - (topo_ids->die_id << apicid_die_offset(topo_info)) | + (topo_ids->die_id << apicid_die_offset_epyc(topo_info)) | (topo_ids->core_id << apicid_core_offset(topo_info)) | topo_ids->smt_id; } @@ -160,15 +145,11 @@ static inline void x86_topo_ids_from_idx_epyc(X86CPUTopoInfo *topo_info, unsigned cpu_index, X86CPUTopoIDs *topo_ids) { -unsigned nr_nodes = MAX(topo_info->nodes_per_pkg, 1); unsigned nr_dies = topo_info->dies_per_pkg; unsigned nr_cores = topo_info->cores_per_die; unsigned nr_threads = topo_info->threads_per_core; -unsigned cores_per_node = DIV_ROUND_UP((nr_dies * nr_cores * nr_threads), -
Re: [PATCH] hw/isa/isa-superio: Set abstract TYPE_ISA_SUPERIO instance size
On Fri, Aug 21, 2020 at 07:38:31PM +0200, Philippe Mathieu-Daudé wrote: > Instead of setting the instance size on each implementations, > set it on the abstract parent, so we are sure no implementation > will forget to set it. > > Reported-by: Eduardo Habkost > Signed-off-by: Philippe Mathieu-Daudé Thanks! The changes look good, but while we are at it we could also clear class_size at smc37c669_type_info and via_superio_info. Signed-off-by: Eduardo Habkost --- diff --git a/hw/isa/smc37c669-superio.c b/hw/isa/smc37c669-superio.c index a988c55330..2dab9d90bf 100644 --- a/hw/isa/smc37c669-superio.c +++ b/hw/isa/smc37c669-superio.c @@ -103,7 +103,6 @@ static void smc37c669_class_init(ObjectClass *klass, void *data) static const TypeInfo smc37c669_type_info = { .name = TYPE_SMC37C669_SUPERIO, .parent= TYPE_ISA_SUPERIO, -.class_size= sizeof(ISASuperIOClass), .class_init= smc37c669_class_init, }; TYPE_INFO(smc37c669_type_info) diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c index 4a9f8954a9..4fd3550cbb 100644 --- a/hw/isa/vt82c686.c +++ b/hw/isa/vt82c686.c @@ -542,7 +542,6 @@ static void vt82c686b_superio_class_init(ObjectClass *klass, void *data) static const TypeInfo via_superio_info = { .name = TYPE_VT82C686B_SUPERIO, .parent= TYPE_ISA_SUPERIO, -.class_size= sizeof(ISASuperIOClass), .class_init= vt82c686b_superio_class_init, }; TYPE_INFO(via_superio_info) > --- > See: > https://www.mail-archive.com/qemu-devel@nongnu.org/msg731954.html > --- > hw/isa/isa-superio.c | 2 +- > hw/isa/smc37c669-superio.c | 1 - > hw/isa/vt82c686.c | 1 - > 3 files changed, 1 insertion(+), 3 deletions(-) > > diff --git a/hw/isa/isa-superio.c b/hw/isa/isa-superio.c > index e2e47d8fd99..83eb21ebb2a 100644 > --- a/hw/isa/isa-superio.c > +++ b/hw/isa/isa-superio.c > @@ -182,6 +182,7 @@ static const TypeInfo isa_superio_type_info = { > .name = TYPE_ISA_SUPERIO, > .parent = TYPE_ISA_DEVICE, > .abstract = true, > +.instance_size = sizeof(ISASuperIODevice), > .class_size = sizeof(ISASuperIOClass), > .class_init = isa_superio_class_init, > }; > @@ -200,7 +201,6 @@ static void fdc37m81x_class_init(ObjectClass *klass, void > *data) > static const TypeInfo fdc37m81x_type_info = { > .name = TYPE_FDC37M81X_SUPERIO, > .parent= TYPE_ISA_SUPERIO, > -.instance_size = sizeof(ISASuperIODevice), > .class_init= fdc37m81x_class_init, > }; > > diff --git a/hw/isa/smc37c669-superio.c b/hw/isa/smc37c669-superio.c > index 18287741cb4..9e59dc16039 100644 > --- a/hw/isa/smc37c669-superio.c > +++ b/hw/isa/smc37c669-superio.c > @@ -103,7 +103,6 @@ static void smc37c669_class_init(ObjectClass *klass, void > *data) > static const TypeInfo smc37c669_type_info = { > .name = TYPE_SMC37C669_SUPERIO, > .parent= TYPE_ISA_SUPERIO, > -.instance_size = sizeof(ISASuperIODevice), > .class_size= sizeof(ISASuperIOClass), > .class_init= smc37c669_class_init, > }; > diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c > index 18160ca445d..a4b84d405d0 100644 > --- a/hw/isa/vt82c686.c > +++ b/hw/isa/vt82c686.c > @@ -533,7 +533,6 @@ static void vt82c686b_superio_class_init(ObjectClass > *klass, void *data) > static const TypeInfo via_superio_info = { > .name = TYPE_VT82C686B_SUPERIO, > .parent= TYPE_ISA_SUPERIO, > -.instance_size = sizeof(ISASuperIODevice), > .class_size= sizeof(ISASuperIOClass), > .class_init= vt82c686b_superio_class_init, > }; > -- > 2.26.2 > -- Eduardo
Re: [PULL 00/17] Usb 20200819 patches
On Wed, 19 Aug 2020 at 06:54, Gerd Hoffmann wrote: > > The following changes since commit d0ed6a69d399ae193959225cdeaa9382746c91cc: > > Update version for v5.1.0 release (2020-08-11 17:07:03 +0100) > > are available in the Git repository at: > > git://git.kraxel.org/qemu tags/usb-20200819-pull-request > > for you to fetch changes up to d7e5b2e1a4035fb81517a2034bb955e58f28d5b9: > > hw/usb: Add U2F device autoscan to passthru mode (2020-08-19 07:35:27 +0200) > > > usb: usb_packet_map error handling for xhci/ehci > usb: add U2F devices (GSoC). Hi; this conflicts with the meson buildsystem merge, I'm afraid -- can you rebase and resend, please? thanks -- PMM
[PULL 0/6] Meson build system fixes
The following changes since commit d6f83a72a7db94a3ede9f5cc4fb39f9c8e89f954: Merge remote-tracking branch 'remotes/philmd-gitlab/tags/acceptance-testing-20200812' into staging (2020-08-21 14:51:43 +0100) are available in the Git repository at: https://gitlab.com/bonzini/qemu.git tags/for-upstream for you to fetch changes up to 460b4664c6ad2b88ccfb2d69ab4cbb7d6c9165a0: keymaps: update (2020-08-21 17:06:17 -0400) meson fixes: * --disable-tools --enable-system build * s390 no-TCG build * fdmon-io_uring * 'shift' error message in version_ge() Marc-André Lureau (1): meson: convert pc-bios/keymaps/Makefile Paolo Bonzini (2): target/s390x: fix meson.build issue keymaps: update Philippe Mathieu-Daudé (1): meson: Fix --disable-tools --enable-system builds Stefano Garzarella (2): util/meson.build: fix fdmon-io_uring build configure: silence 'shift' error message in version_ge() Makefile| 8 -- configure | 6 +- meson.build | 14 +-- pc-bios/keymaps/.gitignore | 1 + pc-bios/keymaps/Makefile| 56 -- pc-bios/keymaps/ar | 242 +++- pc-bios/keymaps/bepo| 242 +++- pc-bios/keymaps/cz | 242 +++- pc-bios/keymaps/da | 242 +++- pc-bios/keymaps/de | 242 +++- pc-bios/keymaps/de-ch | 242 +++- pc-bios/keymaps/en-gb | 242 +++- pc-bios/keymaps/en-us | 242 +++- pc-bios/keymaps/es | 242 +++- pc-bios/keymaps/et | 242 +++- pc-bios/keymaps/fi | 242 +++- pc-bios/keymaps/fo | 242 +++- pc-bios/keymaps/fr | 242 +++- pc-bios/keymaps/fr-be | 242 +++- pc-bios/keymaps/fr-ca | 242 +++- pc-bios/keymaps/fr-ch | 242 +++- pc-bios/keymaps/hr | 242 +++- pc-bios/keymaps/hu | 242 +++- pc-bios/keymaps/is | 242 +++- pc-bios/keymaps/it | 242 +++- pc-bios/keymaps/ja | 242 +++- pc-bios/keymaps/lt | 242 +++- pc-bios/keymaps/lv | 242 +++- pc-bios/keymaps/meson.build | 56 ++ pc-bios/keymaps/mk | 242 +++- pc-bios/keymaps/nl | 242 +++- pc-bios/keymaps/no | 242 +++- pc-bios/keymaps/pl | 242 +++- pc-bios/keymaps/pt | 242 +++- pc-bios/keymaps/pt-br | 242 +++- pc-bios/keymaps/ru | 242 +++- pc-bios/keymaps/th | 242 +++- pc-bios/keymaps/tr | 242 +++- pc-bios/meson.build | 1 + target/s390x/meson.build| 2 +- ui/meson.build | 2 +- util/meson.build| 2 +- 42 files changed, 7752 insertions(+), 140 deletions(-) create mode 100644 pc-bios/keymaps/.gitignore delete mode 100644 pc-bios/keymaps/Makefile create mode 100644 pc-bios/keymaps/meson.build -- 2.26.2
[PULL 1/6] target/s390x: fix meson.build issue
files() is needed to avoid ../meson.build:977:2: ERROR: File tcg-stub.c does not exist. Signed-off-by: Paolo Bonzini --- target/s390x/meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target/s390x/meson.build b/target/s390x/meson.build index d2a3315903..c42eadb7d2 100644 --- a/target/s390x/meson.build +++ b/target/s390x/meson.build @@ -21,7 +21,7 @@ s390x_ss.add(when: 'CONFIG_TCG', if_true: files( 'vec_helper.c', 'vec_int_helper.c', 'vec_string_helper.c', -), if_false: 'tcg-stub.c') +), if_false: files('tcg-stub.c')) s390x_ss.add(when: 'CONFIG_KVM', if_true: files('kvm.c'), if_false: files('kvm-stub.c')) -- 2.26.2
[PULL 5/6] meson: Fix --disable-tools --enable-system builds
From: Philippe Mathieu-Daudé Fixes: $ ../configure --disable-tools --disable-user ../tests/qemu-iotests/meson.build:7:0: ERROR: Unknown variable "qemu_block_tools". Suggested-by: Paolo Bonzini Signed-off-by: Philippe Mathieu-Daudé Acked-by: Paolo Bonzini Message-Id: <20200821150556.1235625-1-phi...@redhat.com> Signed-off-by: Paolo Bonzini --- meson.build | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/meson.build b/meson.build index dd8016c9da..df5bf728b5 100644 --- a/meson.build +++ b/meson.build @@ -1068,12 +1068,13 @@ if 'CONFIG_XKBCOMMON' in config_host dependencies: [qemuutil, xkbcommon], install: have_tools) endif +qemu_block_tools = [] if have_tools qemu_img = executable('qemu-img', [files('qemu-img.c'), hxdep], dependencies: [authz, block, crypto, io, qom, qemuutil], install: true) qemu_io = executable('qemu-io', files('qemu-io.c'), dependencies: [block, qemuutil], install: true) - qemu_block_tools = [qemu_img, qemu_io] + qemu_block_tools += [qemu_img, qemu_io] if targetos == 'linux' or targetos == 'sunos' or targetos.endswith('bsd') qemu_nbd = executable('qemu-nbd', files('qemu-nbd.c'), dependencies: [block, qemuutil], install: true) -- 2.26.2
[PULL 4/6] meson: convert pc-bios/keymaps/Makefile
From: Marc-André Lureau Note that sl and sv keymaps were not created by qemu-keymap. Signed-off-by: Marc-André Lureau Tested-by: Philippe Mathieu-Daudé Signed-off-by: Paolo Bonzini --- Makefile| 8 -- meson.build | 11 pc-bios/keymaps/.gitignore | 1 + pc-bios/keymaps/Makefile| 56 - pc-bios/keymaps/meson.build | 56 + pc-bios/meson.build | 1 + ui/meson.build | 2 +- 7 files changed, 65 insertions(+), 70 deletions(-) create mode 100644 pc-bios/keymaps/.gitignore delete mode 100644 pc-bios/keymaps/Makefile create mode 100644 pc-bios/keymaps/meson.build diff --git a/Makefile b/Makefile index 8373ddccc9..ef28ce0361 100644 --- a/Makefile +++ b/Makefile @@ -229,11 +229,6 @@ distclean: clean ninja-distclean rm -f linux-headers/asm rm -Rf .sdk -KEYMAPS=da en-gb et fr fr-ch is lt no pt-br sv \ -ar de en-us fi fr-be hr it lv nl pl ru th \ -de-ch es fo fr-ca hu ja mk pt sl tr \ -bepocz - ifdef INSTALL_BLOBS BLOBS=bios.bin bios-256k.bin bios-microvm.bin sgabios.bin vgabios.bin vgabios-cirrus.bin \ vgabios-stdvga.bin vgabios-vmware.bin vgabios-qxl.bin vgabios-virtio.bin \ @@ -298,9 +293,6 @@ endif $(INSTALL_DATA) $(SRC_PATH)/ui/qemu.desktop \ "$(DESTDIR)$(qemu_desktopdir)/qemu.desktop" $(INSTALL_DIR) "$(DESTDIR)$(qemu_datadir)/keymaps" - set -e; for x in $(KEYMAPS); do \ - $(INSTALL_DATA) $(SRC_PATH)/pc-bios/keymaps/$$x "$(DESTDIR)$(qemu_datadir)/keymaps"; \ - done ifdef CONFIG_WIN32 diff --git a/meson.build b/meson.build index 808f50b07e..dd8016c9da 100644 --- a/meson.build +++ b/meson.build @@ -1062,6 +1062,12 @@ if 'CONFIG_GUEST_AGENT' in config_host subdir('qga') endif +if 'CONFIG_XKBCOMMON' in config_host + # used for the update-keymaps target, so include rules even if !have_tools + qemu_keymap = executable('qemu-keymap', files('qemu-keymap.c', 'ui/input-keymap.c') + genh, + dependencies: [qemuutil, xkbcommon], install: have_tools) +endif + if have_tools qemu_img = executable('qemu-img', [files('qemu-img.c'), hxdep], dependencies: [authz, block, crypto, io, qom, qemuutil], install: true) @@ -1078,11 +1084,6 @@ if have_tools subdir('contrib/rdmacm-mux') subdir('contrib/elf2dmp') - if 'CONFIG_XKBCOMMON' in config_host -executable('qemu-keymap', files('qemu-keymap.c', 'ui/input-keymap.c'), - dependencies: [qemuutil, xkbcommon], install: true) - endif - executable('qemu-edid', files('qemu-edid.c', 'hw/display/edid-generate.c'), dependencies: qemuutil, install: true) diff --git a/pc-bios/keymaps/.gitignore b/pc-bios/keymaps/.gitignore new file mode 100644 index 00..f90738f4dc --- /dev/null +++ b/pc-bios/keymaps/.gitignore @@ -0,0 +1 @@ +/*.stamp diff --git a/pc-bios/keymaps/Makefile b/pc-bios/keymaps/Makefile deleted file mode 100644 index 76217b0689..00 --- a/pc-bios/keymaps/Makefile +++ /dev/null @@ -1,56 +0,0 @@ - -KEYMAP := $(shell which qemu-keymap 2>/dev/null) - -MAPS := ar bepo cz da de de-ch en-us en-gb es et fi fo \ - fr fr-be fr-ca fr-ch \ - hr hu is it ja lt lv mk nl no pl pt pt-br ru th tr - -ar : MAP_FLAGS := -l ar -bepo : MAP_FLAGS := -l fr -v dvorak -cz : MAP_FLAGS := -l cz -da : MAP_FLAGS := -l dk -de : MAP_FLAGS := -l de -v nodeadkeys -de-ch : MAP_FLAGS := -l ch -en-us : MAP_FLAGS := -l us -en-gb : MAP_FLAGS := -l gb -es : MAP_FLAGS := -l es -et : MAP_FLAGS := -l et -fi : MAP_FLAGS := -l fi -fo : MAP_FLAGS := -l fo -fr : MAP_FLAGS := -l fr -v nodeadkeys -fr-be : MAP_FLAGS := -l be -fr-ca : MAP_FLAGS := -l ca -v fr -fr-ch : MAP_FLAGS := -l ch -v fr -hr : MAP_FLAGS := -l hr -hu : MAP_FLAGS := -l hu -is : MAP_FLAGS := -l is -it : MAP_FLAGS := -l it -ja : MAP_FLAGS := -l jp -m jp106 -lt : MAP_FLAGS := -l lt -lv : MAP_FLAGS := -l lv -mk : MAP_FLAGS := -l mk -nl : MAP_FLAGS := -l nl -no : MAP_FLAGS := -l no -pl : MAP_FLAGS := -l pl -pt : MAP_FLAGS := -l pt -pt-br : MAP_FLAGS := -l br -ru : MAP_FLAGS := -l ru -th : MAP_FLAGS := -l th -tr : MAP_FLAGS := -l tr - -ifeq ($(KEYMAP),) - -all: - @echo "nothing to do (qemu-keymap not found)" - -else - -all: $(MAPS) - -clean: - rm -f $(MAPS) - -$(MAPS): $(KEYMAP) Makefile - $(KEYMAP) -f $@ $(MAP_FLAGS) - -endif diff --git a/pc-bios/keymaps/meson.build b/pc-bios/keymaps/meson.build new file mode 100644 index 00..b737c82230 --- /dev/null +++ b/pc-bios/keymaps/meson.build @@ -0,0 +1,56 @@ +keymaps = { + 'ar': '-l ar', + 'bepo': '-l fr -v dvorak', + 'cz': '-l cz', + 'da': '-l dk', + 'de': '-l de -v nodeadkeys', + 'de-ch': '-l ch', + 'en-gb': '-l
[PULL 3/6] configure: silence 'shift' error message in version_ge()
From: Stefano Garzarella If there are less than 2 arguments in version_ge(), the second 'shift' prints this error: ../configure: line 232: shift: shift count out of range As Eric suggested, we can use 'shift ${2:+2}' which works out to 'shift 2' if $2 is set, or 'shift' (implicitly shift 1) if $2 is not set. This patch replaces both 'shift; shift' occurrences in version_ge() with 'shift ${2:+2}'. Suggested-by: Eric Blake Signed-off-by: Stefano Garzarella Reviewed-by: Eric Blake Message-Id: <20200821203558.10338-1-sgarz...@redhat.com> Signed-off-by: Paolo Bonzini --- configure | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/configure b/configure index 4e5fe33211..d9ca87fbbb 100755 --- a/configure +++ b/configure @@ -228,15 +228,15 @@ version_ge () { while true; do set x $local_ver1 local_first=${2-0} -# shift 2 does nothing if there are less than 2 arguments -shift; shift +# 'shift 2' if $2 is set, or 'shift' if $2 is not set +shift ${2:+2} local_ver1=$* set x $local_ver2 # the second argument finished, the first must be greater or equal test $# = 1 && return 0 test $local_first -lt $2 && return 1 test $local_first -gt $2 && return 0 -shift; shift +shift ${2:+2} local_ver2=$* done } -- 2.26.2
[PULL 2/6] util/meson.build: fix fdmon-io_uring build
From: Stefano Garzarella libqemuutil.a build fails with this error: /usr/bin/ld: libqemuutil.a(util_fdmon-io_uring.c.o): in function `get_sqe': qemu/build/../util/fdmon-io_uring.c:83: undefined reference to `io_uring_get_sqe' /usr/bin/ld: qemu/build/../util/fdmon-io_uring.c:92: undefined reference to `io_uring_submit' /usr/bin/ld: qemu/build/../util/fdmon-io_uring.c:96: undefined reference to `io_uring_get_sqe' /usr/bin/ld: libqemuutil.a(util_fdmon-io_uring.c.o): in function `fdmon_io_uring_wait': qemu/build/../util/fdmon-io_uring.c:289: undefined reference to `io_uring_submit_and_wait' /usr/bin/ld: libqemuutil.a(util_fdmon-io_uring.c.o): in function `fdmon_io_uring_setup': qemu/build/../util/fdmon-io_uring.c:328: undefined reference to `io_uring_queue_init' /usr/bin/ld: libqemuutil.a(util_fdmon-io_uring.c.o): in function `fdmon_io_uring_destroy': qemu/build/../util/fdmon-io_uring.c:343: undefined reference to `io_uring_queue_exit' collect2: error: ld returned 1 exit status This patch fix the issue adding 'linux_io_uring' dependency for fdmon-io_uring.c Fixes: a81df1b68b ("libqemuutil, qapi, trace: convert to meson") Cc: pbonz...@redhat.com Signed-off-by: Stefano Garzarella Message-Id: <20200821154853.94379-1-sgarz...@redhat.com> Signed-off-by: Paolo Bonzini --- util/meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/util/meson.build b/util/meson.build index 23b8ad459b..e6b207a99e 100644 --- a/util/meson.build +++ b/util/meson.build @@ -4,7 +4,7 @@ util_ss.add(when: 'CONFIG_ATOMIC64', if_false: files('atomic64.c')) util_ss.add(when: 'CONFIG_POSIX', if_true: files('aio-posix.c')) util_ss.add(when: 'CONFIG_POSIX', if_true: files('fdmon-poll.c')) util_ss.add(when: 'CONFIG_EPOLL_CREATE1', if_true: files('fdmon-epoll.c')) -util_ss.add(when: 'CONFIG_LINUX_IO_URING', if_true: files('fdmon-io_uring.c')) +util_ss.add(when: ['CONFIG_LINUX_IO_URING', linux_io_uring], if_true: files('fdmon-io_uring.c')) util_ss.add(when: 'CONFIG_POSIX', if_true: files('compatfd.c')) util_ss.add(when: 'CONFIG_POSIX', if_true: files('event_notifier-posix.c')) util_ss.add(when: 'CONFIG_POSIX', if_true: files('mmap-alloc.c')) -- 2.26.2
Re: Suspicious QOM types without instance/class size
On Fri, Aug 21, 2020 at 11:43:35AM +0200, Cornelia Huck wrote: > On Thu, 20 Aug 2020 17:55:29 -0400 > Eduardo Habkost wrote: > > > While trying to convert TypeInfo declarations to the new > > OBJECT_DECLARE* macros, I've stumbled on a few suspicious cases > > where instance_size or class_size is not set, despite having type > > checker macros that use a specific type. > > > > The ones with "WARNING" are abstract types (maybe not serious if > > subclasses set the appropriate sizes). The ones with "ERROR" > > don't seem to be abstract types. > > > > ERROR: hw/s390x/virtio-ccw.c:1237:1: class_size should be set to > > sizeof(VirtioCcwBusClass)? > > ERROR: hw/virtio/virtio-pci.c:2101:1: class_size should be set to > > sizeof(VirtioPCIBusClass)? > > VirtioCcwBusClass and VirtioPCIBusClass are both simple typedefs of > VirtioBusClass (it's likely that I copied the ccw definition from the > pci one). virtio-mmio instead uses VirtioBusClass directly in its > checker macros. > > I don't see a real reason for the typedefs, maybe ccw and pci should > use the mmio approach as well? I think it's OK to keep the typedefs if the code is consistent (i.e. we set instance_size and class_size just in case the typedefs are replaced by a real struct one day). I'm not sure about the TYPE_VIRTIO_MMIO_BUS approach. If the code just needs VirtioBusState or VirtioBusClass pointers, it can already use the VIRTIO_BUS* macros. The OBJECT_DECLARE_TYPE macro Daniel sent expects each QOM type to have a separate struct being defined, which isn't true in many cases. I'm considering removing the "typedef struct Foo Foo" lines from OBJECT_DECLARE_TYPE(), to make initial conversion easier. -- Eduardo
[PULL v2 00/24] target/xtensa updates for 5.2
Hi Peter, please pull the following batch of updates for target/xtensa. Changes v1->v2: - rebase on top of build system changes, resolve trivial conflicts in the last two patches that add new files. The following changes since commit f86d9a093dada59bde5582c7ec320487c4b8: Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging (2020-08-21 17:26:52 +0100) are available in the Git repository at: git://github.com/OSLL/qemu-xtensa.git tags/20200821-xtensa for you to fetch changes up to c621b4142bf1ff8c663811c10bd1628481e494a6: target/xtensa: import DSP3400 core (2020-08-21 12:56:45 -0700) target/xtensa updates for 5.2: - add NMI support; - add DFPU option implementation; - update FPU tests to support both FPU2000 and DFPU; - add example cores with FPU2000 and DFPU. Max Filippov (24): target/xtensa: make opcode properties more dynamic target/xtensa: implement NMI support softfloat: make NO_SIGNALING_NANS runtime property softfloat: pass float_status pointer to pickNaN softfloat: add xtensa specialization for pickNaNMulAdd target/xtensa: add geometry to xtensa_get_regfile_by_name target/xtensa: support copying registers up to 64 bits wide target/xtensa: rename FPU2000 translators and helpers target/xtensa: move FSR/FCR register accessors target/xtensa: don't access BR regfile directly target/xtensa: add DFPU option target/xtensa: add DFPU registers and opcodes target/xtensa: implement FPU division and square root tests/tcg/xtensa: fix test execution on ISS tests/tcg/xtensa: update test_fp0_arith for DFPU tests/tcg/xtensa: expand madd tests tests/tcg/xtensa: update test_fp0_conv for DFPU tests/tcg/xtensa: update test_fp1 for DFPU tests/tcg/xtensa: update test_lsc for DFPU tests/tcg/xtensa: add fp0 div and sqrt tests tests/tcg/xtensa: test double precision load/store tests/tcg/xtensa: add DFP0 arithmetic tests target/xtensa: import de233_fpu core target/xtensa: import DSP3400 core fpu/softfloat-specialize.c.inc|286 +- fpu/softfloat.c | 2 +- hw/xtensa/pic_cpu.c | 6 +- include/fpu/softfloat-helpers.h | 10 + include/fpu/softfloat-types.h | 8 +- target/xtensa/core-de233_fpu.c| 58 + target/xtensa/core-de233_fpu/core-isa.h |727 + target/xtensa/core-de233_fpu/core-matmap.h|717 + target/xtensa/core-de233_fpu/gdb-config.c.inc |277 + target/xtensa/core-de233_fpu/xtensa-modules.c.inc | 20758 +++ target/xtensa/core-dsp3400.c | 58 + target/xtensa/core-dsp3400/core-isa.h |452 + target/xtensa/core-dsp3400/core-matmap.h |312 + target/xtensa/core-dsp3400/gdb-config.c.inc |400 + target/xtensa/core-dsp3400/xtensa-modules.c.inc | 171906 +++ target/xtensa/cpu.c | 5 + target/xtensa/cpu.h | 14 +- target/xtensa/exc_helper.c| 23 +- target/xtensa/fpu_helper.c|342 +- target/xtensa/helper.c| 4 +- target/xtensa/helper.h| 58 +- target/xtensa/meson.build | 2 + target/xtensa/overlay_tool.h | 30 +- target/xtensa/translate.c | 1979 +- tests/tcg/xtensa/fpu.h|142 + tests/tcg/xtensa/macros.inc | 10 +- tests/tcg/xtensa/test_dfp0_arith.S|162 + tests/tcg/xtensa/test_fp0_arith.S |282 +- tests/tcg/xtensa/test_fp0_conv.S |299 +- tests/tcg/xtensa/test_fp0_div.S | 82 + tests/tcg/xtensa/test_fp0_sqrt.S | 76 + tests/tcg/xtensa/test_fp1.S | 62 +- tests/tcg/xtensa/test_lsc.S |170 +- 33 files changed, 19 insertions(+), 831 deletions(-) create mode 100644 target/xtensa/core-de233_fpu.c create mode 100644 target/xtensa/core-de233_fpu/core-isa.h create mode 100644 target/xtensa/core-de233_fpu/core-matmap.h create mode 100644 target/xtensa/core-de233_fpu/gdb-config.c.inc create mode 100644 target/xtensa/core-de233_fpu/xtensa-modules.c.inc create mode 100644 target/xtensa/core-dsp3400.c create mode 100644 target/xtensa/core-dsp3400/core-isa.h create mode 100644 target/xtensa/core-dsp3400/core-matmap.h create mode 100644 target/xtensa/core-dsp3400/gdb-config.c.inc create mode 100644 target/xtensa/core-dsp3400/xtensa-modules.
Re: [PATCH v3] configure: silence 'shift' error message in version_ge()
On 8/21/20 3:35 PM, Stefano Garzarella wrote: If there are less than 2 arguments in version_ge(), the second 'shift' prints this error: ../configure: line 232: shift: shift count out of range As Eric suggested, we can use 'shift ${2:+2}' which works out to 'shift 2' if $2 is set, or 'shift' (implicitly shift 1) if $2 is not set. This patch replaces both 'shift; shift' occurrences in version_ge() with 'shift ${2:+2}'. Suggested-by: Eric Blake Signed-off-by: Stefano Garzarella --- v3: - use Eric's one-liner solution v2: - do not shift if there are no more arguments [Peter] --- configure | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH] tests/docker: add liburing-devel to the Fedora image
On Fri, Aug 21, 2020 at 07:55:02PM +0200, Philippe Mathieu-Daudé wrote: > On 8/21/20 6:54 PM, Stefano Garzarella wrote: > > Install liburing-devel dependencies to get better coverage on > > io-uring stuff (block/io_uring.c and util/fdmon-io_uring.c). > > > > Suggested-by: Philippe Mathieu-Daudé > > Signed-off-by: Stefano Garzarella > > --- > > tests/docker/dockerfiles/fedora.docker | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/tests/docker/dockerfiles/fedora.docker > > b/tests/docker/dockerfiles/fedora.docker > > index 70b6186bd3..9650d324fa 100644 > > --- a/tests/docker/dockerfiles/fedora.docker > > +++ b/tests/docker/dockerfiles/fedora.docker > > @@ -38,6 +38,7 @@ ENV PACKAGES \ > > libssh-devel \ > > libubsan \ > > libudev-devel \ > > +liburing-devel \ > > libusbx-devel \ > > libxml2-devel \ > > libzstd-devel \ > > > > Reviewed-by: Philippe Mathieu-Daudé > > ../util/fdmon-io_uring.c:106:17: error: address argument to atomic > operation must be a pointer to _Atomic type ('unsigned int *' invalid) > old_flags = atomic_fetch_or(&node->flags, FDMON_IO_URING_PENDING | > flags); > ^ > /usr/lib64/clang/10.0.0/include/stdatomic.h:138:42: note: expanded from > macro 'atomic_fetch_or' > #define atomic_fetch_or(object, operand) __c11_atomic_fetch_or(object, > operand, __ATOMIC_SEQ_CST) > ^ ~~ > ../util/fdmon-io_uring.c:130:14: error: address argument to atomic > operation must be a pointer to _Atomic type ('unsigned int *' invalid) > *flags = atomic_fetch_and(&node->flags, ~(FDMON_IO_URING_PENDING | > ^ mmm, I'll try to fix this issue! > > Tested-by: Philippe Mathieu-Daudé > Thanks, Stefano
[PATCH v3] configure: silence 'shift' error message in version_ge()
If there are less than 2 arguments in version_ge(), the second 'shift' prints this error: ../configure: line 232: shift: shift count out of range As Eric suggested, we can use 'shift ${2:+2}' which works out to 'shift 2' if $2 is set, or 'shift' (implicitly shift 1) if $2 is not set. This patch replaces both 'shift; shift' occurrences in version_ge() with 'shift ${2:+2}'. Suggested-by: Eric Blake Signed-off-by: Stefano Garzarella --- v3: - use Eric's one-liner solution v2: - do not shift if there are no more arguments [Peter] --- configure | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/configure b/configure index 4e5fe33211..d9ca87fbbb 100755 --- a/configure +++ b/configure @@ -228,15 +228,15 @@ version_ge () { while true; do set x $local_ver1 local_first=${2-0} -# shift 2 does nothing if there are less than 2 arguments -shift; shift +# 'shift 2' if $2 is set, or 'shift' if $2 is not set +shift ${2:+2} local_ver1=$* set x $local_ver2 # the second argument finished, the first must be greater or equal test $# = 1 && return 0 test $local_first -lt $2 && return 1 test $local_first -gt $2 && return 0 -shift; shift +shift ${2:+2} local_ver2=$* done } -- 2.26.2
Re: Why QEMU should move from C to Rust (clickbait alert ;))
Hi everyone, My name is Alex, I’m a student at the University of Michigan and I just completed an internship at IBM Research. There, I have been working on a project very related to this topic. I tested using Cloud Hypervisor’s Rust-based vhost-user virtiofs and block devices with QEMU. Bigger picture, I wanted to explore the implications of using Rust for vhost-user devices in QEMU. I agree with the points from the original post, namely: · C programming bugs are responsible for a large number of CVEs, and specifically CVEs coming from the implementations of virtual devices. · As a programming language, Rust has matured to a point where it is worth considering it more seriously for production use. It has extensive libraries and community support. Many big players in the industry are already using Rust for production workloads. Full Transparency: the Drawbacks: It would be deceptive to only showcase Rust in an ideal light. · The benchmarks I ran show a noticeable performance hit from switching to a RustVMM implementation of a virtiofsd device. · While Rust has matured greatly, it still is missing a bit. One example of this that came up was that the rust compiler does not have Control Flow Integrity (CFI) features. While these are not as important as in “unsafe” languages such as C, the ability to express unsafe portions of code does allow for some types of memory bugs – although to a much lesser extent (an interesting case of this surfaced from Firecracker, and the handling of mmio [1]). So further protections such as Control Flow Integrity can still be desirable, even with rust code. · There have been years of optimization work put into the C implementations of these devices, and it’s hard to evaluate how optimized the relatively novel rust implementations are. A piece of exciting news is that many of these drawbacks show a pathway for future improvement. Improvements to rust infrastructure are very realistic. Rust boils down to LLVM just like C, so porting over C’s CFI features should be feasible. If more development resources are put into the RustVMM project, there is no reason their implementations can’t be as optimized as the C versions, and this could be greatly aided by expertise coming from the QEMU communities familiarity with these topics. I believe vhost-user devices are an excellent place to start since It lowers the entry barrier for developing in Rust. The device only has to interface with the C-based QEMU binary through a standardized protocol. It removes many worries of moving entirely away from C, since adding a set of Rust devices would simply be giving more options and room to explore. I am putting together the scripts I used for all of the tests at this repo: https://github.com/Alex-Carter01/Qemu-Rust-Testing I am working to standardize everything to make it easier to replicate. I would love any community involvement if people wanted to see how results differ based on the hardware setup, build configuration of the devices etc. The repo also has links to a recording of my original presentation and the slides I was using if you would like to look at that format or see the discussion which came out of it.
Re: [PATCH v2] configure: silence 'shift' error message in version_ge()
On Fri, Aug 21, 2020 at 01:18:54PM -0500, Eric Blake wrote: > On 8/21/20 11:33 AM, Stefano Garzarella wrote: > > If there are less than 2 arguments in version_ge(), the second > > 'shift' prints this error: > > ../configure: line 232: shift: shift count out of range > > > > Let's skip it if there are no more arguments. > > > > Signed-off-by: Stefano Garzarella > > --- > > v2: > > - do not shift if there are no more arguments [Peter] > > --- > > configure | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/configure b/configure > > index 4e5fe33211..5f5f370e2c 100755 > > --- a/configure > > +++ b/configure > > @@ -229,7 +229,7 @@ version_ge () { > > set x $local_ver1 > > local_first=${2-0} > > # shift 2 does nothing if there are less than 2 arguments > > -shift; shift > > +shift; test $# -gt 0 && shift > > > That works. Or you could go with the shorter one-liner: > > shift ${2:+2} yeah, it is better! I'll send v3. Thanks, Stefano
[PATCH v6 14/15] block/nvme: Extract nvme_poll_queue()
As we want to do per-queue polling, extract the nvme_poll_queue() method which operates on a single queue. Reviewed-by: Stefan Hajnoczi Reviewed-by: Stefano Garzarella Signed-off-by: Philippe Mathieu-Daudé --- block/nvme.c | 44 +++- 1 file changed, 27 insertions(+), 17 deletions(-) diff --git a/block/nvme.c b/block/nvme.c index 85b235c8e6d..1cc2e9493d0 100644 --- a/block/nvme.c +++ b/block/nvme.c @@ -590,31 +590,41 @@ out: qemu_vfree(id); } +static bool nvme_poll_queue(NVMeQueuePair *q) +{ +bool progress = false; + +const size_t cqe_offset = q->cq.head * NVME_CQ_ENTRY_BYTES; +NvmeCqe *cqe = (NvmeCqe *)&q->cq.queue[cqe_offset]; + +/* + * Do an early check for completions. q->lock isn't needed because + * nvme_process_completion() only runs in the event loop thread and + * cannot race with itself. + */ +if ((le16_to_cpu(cqe->status) & 0x1) == q->cq_phase) { +return false; +} + +qemu_mutex_lock(&q->lock); +while (nvme_process_completion(q)) { +/* Keep polling */ +progress = true; +} +qemu_mutex_unlock(&q->lock); + +return progress; +} + static bool nvme_poll_queues(BDRVNVMeState *s) { bool progress = false; int i; for (i = 0; i < s->nr_queues; i++) { -NVMeQueuePair *q = s->queues[i]; -const size_t cqe_offset = q->cq.head * NVME_CQ_ENTRY_BYTES; -NvmeCqe *cqe = (NvmeCqe *)&q->cq.queue[cqe_offset]; - -/* - * Do an early check for completions. q->lock isn't needed because - * nvme_process_completion() only runs in the event loop thread and - * cannot race with itself. - */ -if ((le16_to_cpu(cqe->status) & 0x1) == q->cq_phase) { -continue; -} - -qemu_mutex_lock(&q->lock); -while (nvme_process_completion(q)) { -/* Keep polling */ +if (nvme_poll_queue(s->queues[i])) { progress = true; } -qemu_mutex_unlock(&q->lock); } return progress; } -- 2.26.2
[PATCH v6 11/15] block/nvme: Simplify nvme_init_queue() arguments
nvme_init_queue() doesn't require BlockDriverState anymore. Replace it by BDRVNVMeState to simplify. Reviewed-by: Stefan Hajnoczi Reviewed-by: Stefano Garzarella Signed-off-by: Philippe Mathieu-Daudé --- block/nvme.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/block/nvme.c b/block/nvme.c index 3742e0535aa..f98ca067144 100644 --- a/block/nvme.c +++ b/block/nvme.c @@ -165,10 +165,9 @@ static QemuOptsList runtime_opts = { }, }; -static void nvme_init_queue(BlockDriverState *bs, NVMeQueue *q, +static void nvme_init_queue(BDRVNVMeState *s, NVMeQueue *q, int nentries, int entry_bytes, Error **errp) { -BDRVNVMeState *s = bs->opaque; size_t bytes; int r; @@ -251,14 +250,14 @@ static NVMeQueuePair *nvme_create_queue_pair(BlockDriverState *bs, req->prp_list_iova = prp_list_iova + i * s->page_size; } -nvme_init_queue(bs, &q->sq, size, NVME_SQ_ENTRY_BYTES, &local_err); +nvme_init_queue(s, &q->sq, size, NVME_SQ_ENTRY_BYTES, &local_err); if (local_err) { error_propagate(errp, local_err); goto fail; } q->sq.doorbell = &s->regs->doorbells[idx * 2 * s->doorbell_scale]; -nvme_init_queue(bs, &q->cq, size, NVME_CQ_ENTRY_BYTES, &local_err); +nvme_init_queue(s, &q->cq, size, NVME_CQ_ENTRY_BYTES, &local_err); if (local_err) { error_propagate(errp, local_err); goto fail; -- 2.26.2
[PATCH v6 15/15] block/nvme: Use an array of EventNotifier
In preparation of using multiple IRQ (thus multiple eventfds) make BDRVNVMeState::irq_notifier an array (for now of a single element, the admin queue notifier). Reviewed-by: Stefan Hajnoczi Reviewed-by: Stefano Garzarella Signed-off-by: Philippe Mathieu-Daudé --- block/nvme.c | 31 +-- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/block/nvme.c b/block/nvme.c index 1cc2e9493d0..86bfd487e2c 100644 --- a/block/nvme.c +++ b/block/nvme.c @@ -106,6 +106,12 @@ QEMU_BUILD_BUG_ON(offsetof(NVMeRegs, doorbells) != 0x1000); #define INDEX_ADMIN 0 #define INDEX_IO(n) (1 + n) +/* This driver shares a single MSIX IRQ for the admin and I/O queues */ +enum { +MSIX_SHARED_IRQ_IDX = 0, +MSIX_IRQ_COUNT = 1 +}; + struct BDRVNVMeState { AioContext *aio_context; QEMUVFIOState *vfio; @@ -120,7 +126,7 @@ struct BDRVNVMeState { /* How many uint32_t elements does each doorbell entry take. */ size_t doorbell_scale; bool write_cache_supported; -EventNotifier irq_notifier; +EventNotifier irq_notifier[MSIX_IRQ_COUNT]; uint64_t nsze; /* Namespace size reported by identify command */ int nsid; /* The namespace id to read/write data. */ @@ -631,7 +637,8 @@ static bool nvme_poll_queues(BDRVNVMeState *s) static void nvme_handle_event(EventNotifier *n) { -BDRVNVMeState *s = container_of(n, BDRVNVMeState, irq_notifier); +BDRVNVMeState *s = container_of(n, BDRVNVMeState, +irq_notifier[MSIX_SHARED_IRQ_IDX]); trace_nvme_handle_event(s); event_notifier_test_and_clear(n); @@ -683,7 +690,8 @@ out_error: static bool nvme_poll_cb(void *opaque) { EventNotifier *e = opaque; -BDRVNVMeState *s = container_of(e, BDRVNVMeState, irq_notifier); +BDRVNVMeState *s = container_of(e, BDRVNVMeState, +irq_notifier[MSIX_SHARED_IRQ_IDX]); trace_nvme_poll_cb(s); return nvme_poll_queues(s); @@ -705,7 +713,7 @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace, s->device = g_strdup(device); s->nsid = namespace; s->aio_context = bdrv_get_aio_context(bs); -ret = event_notifier_init(&s->irq_notifier, 0); +ret = event_notifier_init(&s->irq_notifier[MSIX_SHARED_IRQ_IDX], 0); if (ret) { error_setg(errp, "Failed to init event notifier"); return ret; @@ -784,12 +792,13 @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace, } } -ret = qemu_vfio_pci_init_irq(s->vfio, &s->irq_notifier, +ret = qemu_vfio_pci_init_irq(s->vfio, s->irq_notifier, VFIO_PCI_MSIX_IRQ_INDEX, errp); if (ret) { goto out; } -aio_set_event_notifier(bdrv_get_aio_context(bs), &s->irq_notifier, +aio_set_event_notifier(bdrv_get_aio_context(bs), + &s->irq_notifier[MSIX_SHARED_IRQ_IDX], false, nvme_handle_event, nvme_poll_cb); nvme_identify(bs, namespace, &local_err); @@ -872,9 +881,10 @@ static void nvme_close(BlockDriverState *bs) nvme_free_queue_pair(s->queues[i]); } g_free(s->queues); -aio_set_event_notifier(bdrv_get_aio_context(bs), &s->irq_notifier, +aio_set_event_notifier(bdrv_get_aio_context(bs), + &s->irq_notifier[MSIX_SHARED_IRQ_IDX], false, NULL, NULL); -event_notifier_cleanup(&s->irq_notifier); +event_notifier_cleanup(&s->irq_notifier[MSIX_SHARED_IRQ_IDX]); qemu_vfio_pci_unmap_bar(s->vfio, 0, (void *)s->regs, 0, NVME_BAR_SIZE); qemu_vfio_close(s->vfio); @@ -1381,7 +1391,8 @@ static void nvme_detach_aio_context(BlockDriverState *bs) q->completion_bh = NULL; } -aio_set_event_notifier(bdrv_get_aio_context(bs), &s->irq_notifier, +aio_set_event_notifier(bdrv_get_aio_context(bs), + &s->irq_notifier[MSIX_SHARED_IRQ_IDX], false, NULL, NULL); } @@ -1391,7 +1402,7 @@ static void nvme_attach_aio_context(BlockDriverState *bs, BDRVNVMeState *s = bs->opaque; s->aio_context = new_context; -aio_set_event_notifier(new_context, &s->irq_notifier, +aio_set_event_notifier(new_context, &s->irq_notifier[MSIX_SHARED_IRQ_IDX], false, nvme_handle_event, nvme_poll_cb); for (int i = 0; i < s->nr_queues; i++) { -- 2.26.2
[PATCH v6 10/15] block/nvme: Replace qemu_try_blockalign(bs) by qemu_try_memalign(pg_sz)
qemu_try_blockalign() is a generic API that call back to the block driver to return its page alignment. As we call from within the very same driver, we already know to page alignment stored in our state. Remove indirections and use the value from BDRVNVMeState. This change is required to later remove the BlockDriverState argument, to make nvme_init_queue() per hardware, and not per block driver. Reviewed-by: Stefan Hajnoczi Reviewed-by: Stefano Garzarella Signed-off-by: Philippe Mathieu-Daudé --- block/nvme.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/block/nvme.c b/block/nvme.c index 7e21a2d7ece..3742e0535aa 100644 --- a/block/nvme.c +++ b/block/nvme.c @@ -174,7 +174,7 @@ static void nvme_init_queue(BlockDriverState *bs, NVMeQueue *q, bytes = ROUND_UP(nentries * entry_bytes, s->page_size); q->head = q->tail = 0; -q->queue = qemu_try_blockalign(bs, bytes); +q->queue = qemu_try_memalign(s->page_size, bytes); if (!q->queue) { error_setg(errp, "Cannot allocate queue"); return; @@ -223,7 +223,7 @@ static NVMeQueuePair *nvme_create_queue_pair(BlockDriverState *bs, if (!q) { return NULL; } -q->prp_list_pages = qemu_try_blockalign(bs, +q->prp_list_pages = qemu_try_memalign(s->page_size, s->page_size * NVME_NUM_REQS); if (!q->prp_list_pages) { goto fail; @@ -522,7 +522,7 @@ static void nvme_identify(BlockDriverState *bs, int namespace, Error **errp) .cdw10 = cpu_to_le32(0x1), }; -id = qemu_try_blockalign(bs, sizeof(*id)); +id = qemu_try_memalign(s->page_size, sizeof(*id)); if (!id) { error_setg(errp, "Cannot allocate buffer for identify response"); goto out; @@ -1141,7 +1141,7 @@ static int nvme_co_prw(BlockDriverState *bs, uint64_t offset, uint64_t bytes, return nvme_co_prw_aligned(bs, offset, bytes, qiov, is_write, flags); } trace_nvme_prw_buffered(s, offset, bytes, qiov->niov, is_write); -buf = qemu_try_blockalign(bs, bytes); +buf = qemu_try_memalign(s->page_size, bytes); if (!buf) { return -ENOMEM; @@ -1285,7 +1285,7 @@ static int coroutine_fn nvme_co_pdiscard(BlockDriverState *bs, assert(s->nr_queues > 1); -buf = qemu_try_blockalign(bs, s->page_size); +buf = qemu_try_memalign(s->page_size, s->page_size); if (!buf) { return -ENOMEM; } -- 2.26.2
[PATCH v6 13/15] block/nvme: Simplify nvme_create_queue_pair() arguments
nvme_create_queue_pair() doesn't require BlockDriverState anymore. Replace it by BDRVNVMeState and AioContext to simplify. Reviewed-by: Stefan Hajnoczi Reviewed-by: Stefano Garzarella Signed-off-by: Philippe Mathieu-Daudé --- block/nvme.c | 13 +++-- 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/block/nvme.c b/block/nvme.c index 3d49ff81fb7..85b235c8e6d 100644 --- a/block/nvme.c +++ b/block/nvme.c @@ -208,12 +208,12 @@ static void nvme_free_req_queue_cb(void *opaque) qemu_mutex_unlock(&q->lock); } -static NVMeQueuePair *nvme_create_queue_pair(BlockDriverState *bs, +static NVMeQueuePair *nvme_create_queue_pair(BDRVNVMeState *s, + AioContext *aio_context, int idx, int size, Error **errp) { int i, r; -BDRVNVMeState *s = bs->opaque; Error *local_err = NULL; NVMeQueuePair *q; uint64_t prp_list_iova; @@ -232,8 +232,7 @@ static NVMeQueuePair *nvme_create_queue_pair(BlockDriverState *bs, q->s = s; q->index = idx; qemu_co_queue_init(&q->free_req_queue); -q->completion_bh = aio_bh_new(bdrv_get_aio_context(bs), - nvme_process_completion_bh, q); +q->completion_bh = aio_bh_new(aio_context, nvme_process_completion_bh, q); r = qemu_vfio_dma_map(s->vfio, q->prp_list_pages, s->page_size * NVME_NUM_REQS, false, &prp_list_iova); @@ -637,7 +636,8 @@ static bool nvme_add_io_queue(BlockDriverState *bs, Error **errp) NvmeCmd cmd; int queue_size = NVME_QUEUE_SIZE; -q = nvme_create_queue_pair(bs, n, queue_size, errp); +q = nvme_create_queue_pair(s, bdrv_get_aio_context(bs), + n, queue_size, errp); if (!q) { return false; } @@ -683,6 +683,7 @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace, Error **errp) { BDRVNVMeState *s = bs->opaque; +AioContext *aio_context = bdrv_get_aio_context(bs); int ret; uint64_t cap; uint64_t timeout_ms; @@ -743,7 +744,7 @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace, /* Set up admin queue. */ s->queues = g_new(NVMeQueuePair *, 1); -s->queues[INDEX_ADMIN] = nvme_create_queue_pair(bs, 0, +s->queues[INDEX_ADMIN] = nvme_create_queue_pair(s, aio_context, 0, NVME_QUEUE_SIZE, errp); if (!s->queues[INDEX_ADMIN]) { -- 2.26.2
[PATCH v6 09/15] block/nvme: Replace qemu_try_blockalign0 by qemu_try_blockalign/memset
In the next commit we'll get rid of qemu_try_blockalign(). To ease review, first replace qemu_try_blockalign0() by explicit calls to qemu_try_blockalign() and memset(). Reviewed-by: Stefan Hajnoczi Reviewed-by: Stefano Garzarella Signed-off-by: Philippe Mathieu-Daudé --- block/nvme.c | 16 +--- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/block/nvme.c b/block/nvme.c index af3176a9669..7e21a2d7ece 100644 --- a/block/nvme.c +++ b/block/nvme.c @@ -174,12 +174,12 @@ static void nvme_init_queue(BlockDriverState *bs, NVMeQueue *q, bytes = ROUND_UP(nentries * entry_bytes, s->page_size); q->head = q->tail = 0; -q->queue = qemu_try_blockalign0(bs, bytes); - +q->queue = qemu_try_blockalign(bs, bytes); if (!q->queue) { error_setg(errp, "Cannot allocate queue"); return; } +memset(q->queue, 0, bytes); r = qemu_vfio_dma_map(s->vfio, q->queue, bytes, false, &q->iova); if (r) { error_setg(errp, "Cannot map queue"); @@ -223,11 +223,12 @@ static NVMeQueuePair *nvme_create_queue_pair(BlockDriverState *bs, if (!q) { return NULL; } -q->prp_list_pages = qemu_try_blockalign0(bs, +q->prp_list_pages = qemu_try_blockalign(bs, s->page_size * NVME_NUM_REQS); if (!q->prp_list_pages) { goto fail; } +memset(q->prp_list_pages, 0, s->page_size * NVME_NUM_REQS); qemu_mutex_init(&q->lock); q->s = s; q->index = idx; @@ -521,7 +522,7 @@ static void nvme_identify(BlockDriverState *bs, int namespace, Error **errp) .cdw10 = cpu_to_le32(0x1), }; -id = qemu_try_blockalign0(bs, sizeof(*id)); +id = qemu_try_blockalign(bs, sizeof(*id)); if (!id) { error_setg(errp, "Cannot allocate buffer for identify response"); goto out; @@ -531,8 +532,9 @@ static void nvme_identify(BlockDriverState *bs, int namespace, Error **errp) error_setg(errp, "Cannot map buffer for DMA"); goto out; } -cmd.prp1 = cpu_to_le64(iova); +memset(id, 0, sizeof(*id)); +cmd.prp1 = cpu_to_le64(iova); if (nvme_cmd_sync(bs, s->queues[INDEX_ADMIN], &cmd)) { error_setg(errp, "Failed to identify controller"); goto out; @@ -1283,11 +1285,11 @@ static int coroutine_fn nvme_co_pdiscard(BlockDriverState *bs, assert(s->nr_queues > 1); -buf = qemu_try_blockalign0(bs, s->page_size); +buf = qemu_try_blockalign(bs, s->page_size); if (!buf) { return -ENOMEM; } - +memset(buf, 0, s->page_size); buf->nlb = cpu_to_le32(bytes >> s->blkshift); buf->slba = cpu_to_le64(offset >> s->blkshift); buf->cattr = 0; -- 2.26.2
[PATCH v6 07/15] block/nvme: Rename local variable
We are going to modify the code in the next commit. Renaming the 'resp' variable to 'id' first makes the next commit easier to review. No logical changes. Reviewed-by: Stefan Hajnoczi Reviewed-by: Stefano Garzarella Signed-off-by: Philippe Mathieu-Daudé --- block/nvme.c | 19 +-- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/block/nvme.c b/block/nvme.c index 419178adda3..15c5202c03c 100644 --- a/block/nvme.c +++ b/block/nvme.c @@ -510,8 +510,8 @@ static void nvme_identify(BlockDriverState *bs, int namespace, Error **errp) BDRVNVMeState *s = bs->opaque; NvmeIdCtrl *idctrl; NvmeIdNs *idns; +uint8_t *id; NvmeLBAF *lbaf; -uint8_t *resp; uint16_t oncs; int r; uint64_t iova; @@ -520,14 +520,14 @@ static void nvme_identify(BlockDriverState *bs, int namespace, Error **errp) .cdw10 = cpu_to_le32(0x1), }; -resp = qemu_try_blockalign0(bs, sizeof(NvmeIdCtrl)); -if (!resp) { +id = qemu_try_blockalign0(bs, sizeof(NvmeIdCtrl)); +if (!id) { error_setg(errp, "Cannot allocate buffer for identify response"); goto out; } -idctrl = (NvmeIdCtrl *)resp; -idns = (NvmeIdNs *)resp; -r = qemu_vfio_dma_map(s->vfio, resp, sizeof(NvmeIdCtrl), true, &iova); +idctrl = (NvmeIdCtrl *)id; +idns = (NvmeIdNs *)id; +r = qemu_vfio_dma_map(s->vfio, id, sizeof(NvmeIdCtrl), true, &iova); if (r) { error_setg(errp, "Cannot map buffer for DMA"); goto out; @@ -554,8 +554,7 @@ static void nvme_identify(BlockDriverState *bs, int namespace, Error **errp) s->supports_write_zeroes = !!(oncs & NVME_ONCS_WRITE_ZEROS); s->supports_discard = !!(oncs & NVME_ONCS_DSM); -memset(resp, 0, 4096); - +memset(id, 0, 4096); cmd.cdw10 = 0; cmd.nsid = cpu_to_le32(namespace); if (nvme_cmd_sync(bs, s->queues[INDEX_ADMIN], &cmd)) { @@ -587,8 +586,8 @@ static void nvme_identify(BlockDriverState *bs, int namespace, Error **errp) s->blkshift = lbaf->ds; out: -qemu_vfio_dma_unmap(s->vfio, resp); -qemu_vfree(resp); +qemu_vfio_dma_unmap(s->vfio, id); +qemu_vfree(id); } static bool nvme_poll_queues(BDRVNVMeState *s) -- 2.26.2
[PATCH v6 12/15] block/nvme: Replace BDRV_POLL_WHILE by AIO_WAIT_WHILE
BDRV_POLL_WHILE() is defined as: #define BDRV_POLL_WHILE(bs, cond) ({ \ BlockDriverState *bs_ = (bs); \ AIO_WAIT_WHILE(bdrv_get_aio_context(bs_), \ cond); }) As we will remove the BlockDriverState use in the next commit, start by using the exploded version of BDRV_POLL_WHILE(). Reviewed-by: Stefan Hajnoczi Reviewed-by: Stefano Garzarella Signed-off-by: Philippe Mathieu-Daudé --- block/nvme.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/block/nvme.c b/block/nvme.c index f98ca067144..3d49ff81fb7 100644 --- a/block/nvme.c +++ b/block/nvme.c @@ -493,6 +493,7 @@ static void nvme_cmd_sync_cb(void *opaque, int ret) static int nvme_cmd_sync(BlockDriverState *bs, NVMeQueuePair *q, NvmeCmd *cmd) { +AioContext *aio_context = bdrv_get_aio_context(bs); NVMeRequest *req; int ret = -EINPROGRESS; req = nvme_get_free_req(q); @@ -501,7 +502,7 @@ static int nvme_cmd_sync(BlockDriverState *bs, NVMeQueuePair *q, } nvme_submit_command(q, req, cmd, nvme_cmd_sync_cb, &ret); -BDRV_POLL_WHILE(bs, ret == -EINPROGRESS); +AIO_WAIT_WHILE(aio_context, ret == -EINPROGRESS); return ret; } -- 2.26.2
[PATCH v6 05/15] block/nvme: Improve error message when IO queue creation failed
Do not use the same error message for different failures. Display a different error whether it is the CQ or the SQ. Reviewed-by: Stefan Hajnoczi Reviewed-by: Stefano Garzarella Signed-off-by: Philippe Mathieu-Daudé --- block/nvme.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/block/nvme.c b/block/nvme.c index b4c1a6690e4..c63629d3b45 100644 --- a/block/nvme.c +++ b/block/nvme.c @@ -648,7 +648,7 @@ static bool nvme_add_io_queue(BlockDriverState *bs, Error **errp) .cdw11 = cpu_to_le32(0x3), }; if (nvme_cmd_sync(bs, s->queues[INDEX_ADMIN], &cmd)) { -error_setg(errp, "Failed to create io queue [%d]", n); +error_setg(errp, "Failed to create CQ io queue [%d]", n); nvme_free_queue_pair(q); return false; } @@ -659,7 +659,7 @@ static bool nvme_add_io_queue(BlockDriverState *bs, Error **errp) .cdw11 = cpu_to_le32(0x1 | (n << 16)), }; if (nvme_cmd_sync(bs, s->queues[INDEX_ADMIN], &cmd)) { -error_setg(errp, "Failed to create io queue [%d]", n); +error_setg(errp, "Failed to create SQ io queue [%d]", n); nvme_free_queue_pair(q); return false; } -- 2.26.2
[PATCH v6 03/15] block/nvme: Let nvme_create_queue_pair() fail gracefully
As nvme_create_queue_pair() is allowed to fail, replace the alloc() calls by try_alloc() to avoid aborting QEMU. Reviewed-by: Stefan Hajnoczi Reviewed-by: Stefano Garzarella Signed-off-by: Philippe Mathieu-Daudé --- block/nvme.c | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/block/nvme.c b/block/nvme.c index 8c30a5fee28..a6e5537aaaf 100644 --- a/block/nvme.c +++ b/block/nvme.c @@ -213,14 +213,22 @@ static NVMeQueuePair *nvme_create_queue_pair(BlockDriverState *bs, int i, r; BDRVNVMeState *s = bs->opaque; Error *local_err = NULL; -NVMeQueuePair *q = g_new0(NVMeQueuePair, 1); +NVMeQueuePair *q; uint64_t prp_list_iova; +q = g_try_new0(NVMeQueuePair, 1); +if (!q) { +return NULL; +} +q->prp_list_pages = qemu_try_blockalign0(bs, + s->page_size * NVME_NUM_REQS); +if (!q->prp_list_pages) { +goto fail; +} qemu_mutex_init(&q->lock); q->s = s; q->index = idx; qemu_co_queue_init(&q->free_req_queue); -q->prp_list_pages = qemu_blockalign0(bs, s->page_size * NVME_NUM_REQS); q->completion_bh = aio_bh_new(bdrv_get_aio_context(bs), nvme_process_completion_bh, q); r = qemu_vfio_dma_map(s->vfio, q->prp_list_pages, -- 2.26.2
[PATCH v6 08/15] block/nvme: Use union of NvmeIdCtrl / NvmeIdNs structures
We allocate an unique chunk of memory then use it for two different structures. By using an union, we make it clear the data is overlapping (and we can remove the casts). Suggested-by: Stefan Hajnoczi Reviewed-by: Stefan Hajnoczi Reviewed-by: Stefano Garzarella Signed-off-by: Philippe Mathieu-Daudé --- block/nvme.c | 31 +++ 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/block/nvme.c b/block/nvme.c index 15c5202c03c..af3176a9669 100644 --- a/block/nvme.c +++ b/block/nvme.c @@ -508,9 +508,10 @@ static int nvme_cmd_sync(BlockDriverState *bs, NVMeQueuePair *q, static void nvme_identify(BlockDriverState *bs, int namespace, Error **errp) { BDRVNVMeState *s = bs->opaque; -NvmeIdCtrl *idctrl; -NvmeIdNs *idns; -uint8_t *id; +union { +NvmeIdCtrl ctrl; +NvmeIdNs ns; +} *id; NvmeLBAF *lbaf; uint16_t oncs; int r; @@ -520,14 +521,12 @@ static void nvme_identify(BlockDriverState *bs, int namespace, Error **errp) .cdw10 = cpu_to_le32(0x1), }; -id = qemu_try_blockalign0(bs, sizeof(NvmeIdCtrl)); +id = qemu_try_blockalign0(bs, sizeof(*id)); if (!id) { error_setg(errp, "Cannot allocate buffer for identify response"); goto out; } -idctrl = (NvmeIdCtrl *)id; -idns = (NvmeIdNs *)id; -r = qemu_vfio_dma_map(s->vfio, id, sizeof(NvmeIdCtrl), true, &iova); +r = qemu_vfio_dma_map(s->vfio, id, sizeof(*id), true, &iova); if (r) { error_setg(errp, "Cannot map buffer for DMA"); goto out; @@ -539,22 +538,22 @@ static void nvme_identify(BlockDriverState *bs, int namespace, Error **errp) goto out; } -if (le32_to_cpu(idctrl->nn) < namespace) { +if (le32_to_cpu(id->ctrl.nn) < namespace) { error_setg(errp, "Invalid namespace"); goto out; } -s->write_cache_supported = le32_to_cpu(idctrl->vwc) & 0x1; -s->max_transfer = (idctrl->mdts ? 1 << idctrl->mdts : 0) * s->page_size; +s->write_cache_supported = le32_to_cpu(id->ctrl.vwc) & 0x1; +s->max_transfer = (id->ctrl.mdts ? 1 << id->ctrl.mdts : 0) * s->page_size; /* For now the page list buffer per command is one page, to hold at most * s->page_size / sizeof(uint64_t) entries. */ s->max_transfer = MIN_NON_ZERO(s->max_transfer, s->page_size / sizeof(uint64_t) * s->page_size); -oncs = le16_to_cpu(idctrl->oncs); +oncs = le16_to_cpu(id->ctrl.oncs); s->supports_write_zeroes = !!(oncs & NVME_ONCS_WRITE_ZEROS); s->supports_discard = !!(oncs & NVME_ONCS_DSM); -memset(id, 0, 4096); +memset(id, 0, sizeof(*id)); cmd.cdw10 = 0; cmd.nsid = cpu_to_le32(namespace); if (nvme_cmd_sync(bs, s->queues[INDEX_ADMIN], &cmd)) { @@ -562,11 +561,11 @@ static void nvme_identify(BlockDriverState *bs, int namespace, Error **errp) goto out; } -s->nsze = le64_to_cpu(idns->nsze); -lbaf = &idns->lbaf[NVME_ID_NS_FLBAS_INDEX(idns->flbas)]; +s->nsze = le64_to_cpu(id->ns.nsze); +lbaf = &id->ns.lbaf[NVME_ID_NS_FLBAS_INDEX(id->ns.flbas)]; -if (NVME_ID_NS_DLFEAT_WRITE_ZEROES(idns->dlfeat) && -NVME_ID_NS_DLFEAT_READ_BEHAVIOR(idns->dlfeat) == +if (NVME_ID_NS_DLFEAT_WRITE_ZEROES(id->ns.dlfeat) && +NVME_ID_NS_DLFEAT_READ_BEHAVIOR(id->ns.dlfeat) == NVME_ID_NS_DLFEAT_READ_BEHAVIOR_ZEROES) { bs->supported_write_flags |= BDRV_REQ_MAY_UNMAP; } -- 2.26.2
[PATCH v6 06/15] block/nvme: Use common error path in nvme_add_io_queue()
Rearrange nvme_add_io_queue() by using a common error path. This will be proven useful in few commits where we add IRQ notification to the IO queues. Reviewed-by: Stefan Hajnoczi Reviewed-by: Stefano Garzarella Signed-off-by: Philippe Mathieu-Daudé --- block/nvme.c | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/block/nvme.c b/block/nvme.c index c63629d3b45..419178adda3 100644 --- a/block/nvme.c +++ b/block/nvme.c @@ -649,8 +649,7 @@ static bool nvme_add_io_queue(BlockDriverState *bs, Error **errp) }; if (nvme_cmd_sync(bs, s->queues[INDEX_ADMIN], &cmd)) { error_setg(errp, "Failed to create CQ io queue [%d]", n); -nvme_free_queue_pair(q); -return false; +goto out_error; } cmd = (NvmeCmd) { .opcode = NVME_ADM_CMD_CREATE_SQ, @@ -660,13 +659,15 @@ static bool nvme_add_io_queue(BlockDriverState *bs, Error **errp) }; if (nvme_cmd_sync(bs, s->queues[INDEX_ADMIN], &cmd)) { error_setg(errp, "Failed to create SQ io queue [%d]", n); -nvme_free_queue_pair(q); -return false; +goto out_error; } s->queues = g_renew(NVMeQueuePair *, s->queues, n + 1); s->queues[n] = q; s->nr_queues++; return true; +out_error: +nvme_free_queue_pair(q); +return false; } static bool nvme_poll_cb(void *opaque) -- 2.26.2
[PATCH v6 01/15] block/nvme: Replace magic value by SCALE_MS definition
Use self-explicit SCALE_MS definition instead of magic value. Reviewed-by: Stefan Hajnoczi Reviewed-by: Stefano Garzarella Signed-off-by: Philippe Mathieu-Daudé --- block/nvme.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/block/nvme.c b/block/nvme.c index 374e2689157..2f5e3c2adfa 100644 --- a/block/nvme.c +++ b/block/nvme.c @@ -715,7 +715,7 @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace, /* Reset device to get a clean state. */ s->regs->cc = cpu_to_le32(le32_to_cpu(s->regs->cc) & 0xFE); /* Wait for CSTS.RDY = 0. */ -deadline = qemu_clock_get_ns(QEMU_CLOCK_REALTIME) + timeout_ms * 100ULL; +deadline = qemu_clock_get_ns(QEMU_CLOCK_REALTIME) + timeout_ms * SCALE_MS; while (le32_to_cpu(s->regs->csts) & 0x1) { if (qemu_clock_get_ns(QEMU_CLOCK_REALTIME) > deadline) { error_setg(errp, "Timeout while waiting for device to reset (%" -- 2.26.2
[PATCH v6 00/15] block/nvme: Various cleanups required to use multiple queues
Hi Kevin, This series is mostly code rearrangement (cleanups) to be able to split the hardware code from the block driver code, to be able to use multiple queues on the same hardware, or multiple block drivers on the same hardware. All this series is reviewed. Since v5: - audit rebase on "block/nvme: support nested aio_poll" - addressed Stefano's review comments - added Stefano's R-b tags Since v4: - added 'block/nvme: Use an array of EventNotifier' patch Since v3: - renamed QUEUE_INDEX_{ADMIN/IO} -> INDEX{ADMIN/IO} - added stefanha tags Since v2: - addressed stefanha review comments - added 4 trivial patches (to simplify the last one) - register IRQ notifier for each queuepair (admin and io) Since v1: - rebased - use SCALE_MS definition - added Stefan's R-b - addressed Stefan's review comments - use union { NvmeIdCtrl / NvmeIdNs } - move irq_notifier to NVMeQueuePair - removed patches depending on "a tracable hardware stateo object instead of BDRVNVMeState". Phil. Philippe Mathieu-Daudé (15): block/nvme: Replace magic value by SCALE_MS definition block/nvme: Avoid further processing if trace event not enabled block/nvme: Let nvme_create_queue_pair() fail gracefully block/nvme: Define INDEX macros to ease code review block/nvme: Improve error message when IO queue creation failed block/nvme: Use common error path in nvme_add_io_queue() block/nvme: Rename local variable block/nvme: Use union of NvmeIdCtrl / NvmeIdNs structures block/nvme: Replace qemu_try_blockalign0 by qemu_try_blockalign/memset block/nvme: Replace qemu_try_blockalign(bs) by qemu_try_memalign(pg_sz) block/nvme: Simplify nvme_init_queue() arguments block/nvme: Replace BDRV_POLL_WHILE by AIO_WAIT_WHILE block/nvme: Simplify nvme_create_queue_pair() arguments block/nvme: Extract nvme_poll_queue() block/nvme: Use an array of EventNotifier block/nvme.c | 211 ++- 1 file changed, 125 insertions(+), 86 deletions(-) -- 2.26.2
[PATCH v6 04/15] block/nvme: Define INDEX macros to ease code review
Use definitions instead of '0' or '1' indexes. Also this will be useful when using multi-queues later. Reviewed-by: Stefan Hajnoczi Reviewed-by: Stefano Garzarella Signed-off-by: Philippe Mathieu-Daudé --- block/nvme.c | 33 +++-- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/block/nvme.c b/block/nvme.c index a6e5537aaaf..b4c1a6690e4 100644 --- a/block/nvme.c +++ b/block/nvme.c @@ -103,6 +103,9 @@ typedef volatile struct { QEMU_BUILD_BUG_ON(offsetof(NVMeRegs, doorbells) != 0x1000); +#define INDEX_ADMIN 0 +#define INDEX_IO(n) (1 + n) + struct BDRVNVMeState { AioContext *aio_context; QEMUVFIOState *vfio; @@ -531,7 +534,7 @@ static void nvme_identify(BlockDriverState *bs, int namespace, Error **errp) } cmd.prp1 = cpu_to_le64(iova); -if (nvme_cmd_sync(bs, s->queues[0], &cmd)) { +if (nvme_cmd_sync(bs, s->queues[INDEX_ADMIN], &cmd)) { error_setg(errp, "Failed to identify controller"); goto out; } @@ -555,7 +558,7 @@ static void nvme_identify(BlockDriverState *bs, int namespace, Error **errp) cmd.cdw10 = 0; cmd.nsid = cpu_to_le32(namespace); -if (nvme_cmd_sync(bs, s->queues[0], &cmd)) { +if (nvme_cmd_sync(bs, s->queues[INDEX_ADMIN], &cmd)) { error_setg(errp, "Failed to identify namespace"); goto out; } @@ -644,7 +647,7 @@ static bool nvme_add_io_queue(BlockDriverState *bs, Error **errp) .cdw10 = cpu_to_le32(((queue_size - 1) << 16) | (n & 0x)), .cdw11 = cpu_to_le32(0x3), }; -if (nvme_cmd_sync(bs, s->queues[0], &cmd)) { +if (nvme_cmd_sync(bs, s->queues[INDEX_ADMIN], &cmd)) { error_setg(errp, "Failed to create io queue [%d]", n); nvme_free_queue_pair(q); return false; @@ -655,7 +658,7 @@ static bool nvme_add_io_queue(BlockDriverState *bs, Error **errp) .cdw10 = cpu_to_le32(((queue_size - 1) << 16) | (n & 0x)), .cdw11 = cpu_to_le32(0x1 | (n << 16)), }; -if (nvme_cmd_sync(bs, s->queues[0], &cmd)) { +if (nvme_cmd_sync(bs, s->queues[INDEX_ADMIN], &cmd)) { error_setg(errp, "Failed to create io queue [%d]", n); nvme_free_queue_pair(q); return false; @@ -739,16 +742,18 @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace, /* Set up admin queue. */ s->queues = g_new(NVMeQueuePair *, 1); -s->queues[0] = nvme_create_queue_pair(bs, 0, NVME_QUEUE_SIZE, errp); -if (!s->queues[0]) { +s->queues[INDEX_ADMIN] = nvme_create_queue_pair(bs, 0, + NVME_QUEUE_SIZE, + errp); +if (!s->queues[INDEX_ADMIN]) { ret = -EINVAL; goto out; } s->nr_queues = 1; QEMU_BUILD_BUG_ON(NVME_QUEUE_SIZE & 0xF000); s->regs->aqa = cpu_to_le32((NVME_QUEUE_SIZE << 16) | NVME_QUEUE_SIZE); -s->regs->asq = cpu_to_le64(s->queues[0]->sq.iova); -s->regs->acq = cpu_to_le64(s->queues[0]->cq.iova); +s->regs->asq = cpu_to_le64(s->queues[INDEX_ADMIN]->sq.iova); +s->regs->acq = cpu_to_le64(s->queues[INDEX_ADMIN]->cq.iova); /* After setting up all control registers we can enable device now. */ s->regs->cc = cpu_to_le32((ctz32(NVME_CQ_ENTRY_BYTES) << 20) | @@ -839,7 +844,7 @@ static int nvme_enable_disable_write_cache(BlockDriverState *bs, bool enable, .cdw11 = cpu_to_le32(enable ? 0x01 : 0x00), }; -ret = nvme_cmd_sync(bs, s->queues[0], &cmd); +ret = nvme_cmd_sync(bs, s->queues[INDEX_ADMIN], &cmd); if (ret) { error_setg(errp, "Failed to configure NVMe write cache"); } @@ -1056,7 +1061,7 @@ static coroutine_fn int nvme_co_prw_aligned(BlockDriverState *bs, { int r; BDRVNVMeState *s = bs->opaque; -NVMeQueuePair *ioq = s->queues[1]; +NVMeQueuePair *ioq = s->queues[INDEX_IO(0)]; NVMeRequest *req; uint32_t cdw12 = (((bytes >> s->blkshift) - 1) & 0x) | @@ -1171,7 +1176,7 @@ static coroutine_fn int nvme_co_pwritev(BlockDriverState *bs, static coroutine_fn int nvme_co_flush(BlockDriverState *bs) { BDRVNVMeState *s = bs->opaque; -NVMeQueuePair *ioq = s->queues[1]; +NVMeQueuePair *ioq = s->queues[INDEX_IO(0)]; NVMeRequest *req; NvmeCmd cmd = { .opcode = NVME_CMD_FLUSH, @@ -1202,7 +1207,7 @@ static coroutine_fn int nvme_co_pwrite_zeroes(BlockDriverState *bs, BdrvRequestFlags flags) { BDRVNVMeState *s = bs->opaque; -NVMeQueuePair *ioq = s->queues[1]; +NVMeQueuePair *ioq = s->queues[INDEX_IO(0)]; NVMeRequest *req; uint32_t cdw12 = ((bytes >> s->blkshift) - 1) & 0x; @@ -1255,7 +1260,7 @@ static int coroutine_fn nvme_co_pdiscard(BlockDriverState *bs, int bytes) { BDRVNVMeState *s = bs->opaque; -NVMeQueuePair *ioq = s->queues[1]; +N
[PATCH v6 02/15] block/nvme: Avoid further processing if trace event not enabled
Avoid further processing if TRACE_NVME_SUBMIT_COMMAND_RAW is not enabled. This is an untested intend of performance optimization. Reviewed-by: Stefan Hajnoczi Signed-off-by: Philippe Mathieu-Daudé --- block/nvme.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/block/nvme.c b/block/nvme.c index 2f5e3c2adfa..8c30a5fee28 100644 --- a/block/nvme.c +++ b/block/nvme.c @@ -441,6 +441,9 @@ static void nvme_trace_command(const NvmeCmd *cmd) { int i; +if (!trace_event_get_state_backends(TRACE_NVME_SUBMIT_COMMAND_RAW)) { +return; +} for (i = 0; i < 8; ++i) { uint8_t *cmdp = (uint8_t *)cmd + i * 8; trace_nvme_submit_command_raw(cmdp[0], cmdp[1], cmdp[2], cmdp[3], -- 2.26.2
Re: [PATCH v2 05/58] allwinner-h3: Rename memmap enum constants
Op do 20 aug. 2020 02:12 schreef Eduardo Habkost : > Some of the enum constant names conflict with the QOM type check > macros (AW_H3_CCU, AW_H3_SYSCTRL). This needs to be addressed to > allow us to transform the QOM type check macros into functions > generated by OBJECT_DECLARE_TYPE(). > > Rename all the constants to AW_H3_DEV_*, to avoid conflicts. > > Reviewed-by: Daniel P. Berrangé > Reviewed-by: Niek Linnenbank > Signed-off-by: Eduardo Habkost > --- > Changes v1 -> v2: > * Added more details to commit message > > --- > Cc: Beniamino Galvani > Cc: Peter Maydell > Cc: Niek Linnenbank > Cc: qemu-...@nongnu.org > Cc: qemu-devel@nongnu.org > --- > include/hw/arm/allwinner-h3.h | 62 - > hw/arm/allwinner-h3.c | 124 +- > hw/arm/orangepi.c | 6 +- > 3 files changed, 96 insertions(+), 96 deletions(-) > > diff --git a/include/hw/arm/allwinner-h3.h b/include/hw/arm/allwinner-h3.h > index 82e4e59216..626139dcb3 100644 > --- a/include/hw/arm/allwinner-h3.h > +++ b/include/hw/arm/allwinner-h3.h > @@ -61,37 +61,37 @@ > * @see AwH3State > */ > enum { > -AW_H3_SRAM_A1, > -AW_H3_SRAM_A2, > -AW_H3_SRAM_C, > -AW_H3_SYSCTRL, > -AW_H3_MMC0, > -AW_H3_SID, > -AW_H3_EHCI0, > -AW_H3_OHCI0, > -AW_H3_EHCI1, > -AW_H3_OHCI1, > -AW_H3_EHCI2, > -AW_H3_OHCI2, > -AW_H3_EHCI3, > -AW_H3_OHCI3, > -AW_H3_CCU, > -AW_H3_PIT, > -AW_H3_UART0, > -AW_H3_UART1, > -AW_H3_UART2, > -AW_H3_UART3, > -AW_H3_EMAC, > -AW_H3_DRAMCOM, > -AW_H3_DRAMCTL, > -AW_H3_DRAMPHY, > -AW_H3_GIC_DIST, > -AW_H3_GIC_CPU, > -AW_H3_GIC_HYP, > -AW_H3_GIC_VCPU, > -AW_H3_RTC, > -AW_H3_CPUCFG, > -AW_H3_SDRAM > +AW_H3_DEV_SRAM_A1, > +AW_H3_DEV_SRAM_A2, > +AW_H3_DEV_SRAM_C, > +AW_H3_DEV_SYSCTRL, > +AW_H3_DEV_MMC0, > +AW_H3_DEV_SID, > +AW_H3_DEV_EHCI0, > +AW_H3_DEV_OHCI0, > +AW_H3_DEV_EHCI1, > +AW_H3_DEV_OHCI1, > +AW_H3_DEV_EHCI2, > +AW_H3_DEV_OHCI2, > +AW_H3_DEV_EHCI3, > +AW_H3_DEV_OHCI3, > +AW_H3_DEV_CCU, > +AW_H3_DEV_PIT, > +AW_H3_DEV_UART0, > +AW_H3_DEV_UART1, > +AW_H3_DEV_UART2, > +AW_H3_DEV_UART3, > +AW_H3_DEV_EMAC, > +AW_H3_DEV_DRAMCOM, > +AW_H3_DEV_DRAMCTL, > +AW_H3_DEV_DRAMPHY, > +AW_H3_DEV_GIC_DIST, > +AW_H3_DEV_GIC_CPU, > +AW_H3_DEV_GIC_HYP, > +AW_H3_DEV_GIC_VCPU, > +AW_H3_DEV_RTC, > +AW_H3_DEV_CPUCFG, > +AW_H3_DEV_SDRAM > }; > > /** Total number of CPU cores in the H3 SoC */ > diff --git a/hw/arm/allwinner-h3.c b/hw/arm/allwinner-h3.c > index ff92ded82c..341abe6718 100644 > --- a/hw/arm/allwinner-h3.c > +++ b/hw/arm/allwinner-h3.c > @@ -35,37 +35,37 @@ > > /* Memory map */ > const hwaddr allwinner_h3_memmap[] = { > -[AW_H3_SRAM_A1]= 0x, > -[AW_H3_SRAM_A2]= 0x00044000, > -[AW_H3_SRAM_C] = 0x0001, > -[AW_H3_SYSCTRL]= 0x01c0, > -[AW_H3_MMC0] = 0x01c0f000, > -[AW_H3_SID]= 0x01c14000, > -[AW_H3_EHCI0] = 0x01c1a000, > -[AW_H3_OHCI0] = 0x01c1a400, > -[AW_H3_EHCI1] = 0x01c1b000, > -[AW_H3_OHCI1] = 0x01c1b400, > -[AW_H3_EHCI2] = 0x01c1c000, > -[AW_H3_OHCI2] = 0x01c1c400, > -[AW_H3_EHCI3] = 0x01c1d000, > -[AW_H3_OHCI3] = 0x01c1d400, > -[AW_H3_CCU]= 0x01c2, > -[AW_H3_PIT]= 0x01c20c00, > -[AW_H3_UART0] = 0x01c28000, > -[AW_H3_UART1] = 0x01c28400, > -[AW_H3_UART2] = 0x01c28800, > -[AW_H3_UART3] = 0x01c28c00, > -[AW_H3_EMAC] = 0x01c3, > -[AW_H3_DRAMCOM]= 0x01c62000, > -[AW_H3_DRAMCTL]= 0x01c63000, > -[AW_H3_DRAMPHY]= 0x01c65000, > -[AW_H3_GIC_DIST] = 0x01c81000, > -[AW_H3_GIC_CPU]= 0x01c82000, > -[AW_H3_GIC_HYP]= 0x01c84000, > -[AW_H3_GIC_VCPU] = 0x01c86000, > -[AW_H3_RTC]= 0x01f0, > -[AW_H3_CPUCFG] = 0x01f01c00, > -[AW_H3_SDRAM] = 0x4000 > +[AW_H3_DEV_SRAM_A1]= 0x, > +[AW_H3_DEV_SRAM_A2]= 0x00044000, > +[AW_H3_DEV_SRAM_C] = 0x0001, > +[AW_H3_DEV_SYSCTRL]= 0x01c0, > +[AW_H3_DEV_MMC0] = 0x01c0f000, > +[AW_H3_DEV_SID]= 0x01c14000, > +[AW_H3_DEV_EHCI0] = 0x01c1a000, > +[AW_H3_DEV_OHCI0] = 0x01c1a400, > +[AW_H3_DEV_EHCI1] = 0x01c1b000, > +[AW_H3_DEV_OHCI1] = 0x01c1b400, > +[AW_H3_DEV_EHCI2] = 0x01c1c000, > +[AW_H3_DEV_OHCI2] = 0x01c1c400, > +[AW_H3_DEV_EHCI3] = 0x01c1d000, > +[AW_H3_DEV_OHCI3] = 0x01c1d400, > +[AW_H3_DEV_CCU]= 0x01c2, > +[AW_H3_DEV_PIT]= 0x01c20c00, > +[AW_H3_DEV_UART0] = 0x01c28000, > +[AW_H3_DEV_UART1] = 0x01c28400, > +[AW_H3_DEV_UART2] = 0x01c28800, > +[AW_H3_DEV_UART3] = 0x01c28c00, > +[AW_H3_DEV_EMAC] = 0x01c3,
[Bug 1892544] Re: meson qemu 5.2 can not built with msys2
error output ``` $ ../qemu.org/configure \ > --python=python3 \ > --cross-prefix=x86_64-w64-mingw32- --enable-gtk --enable-sdl \ > --enable-capstone=git \ > --enable-stack-protector \ > --enable-gnutls \ > --enable-nettle \ > --enable-vnc \ > --enable-vnc-sasl \ > --enable-vnc-jpeg \ > --enable-vnc-png \ > --enable-membarrier \ > --enable-slirp=git \ > --disable-kvm \ > --enable-hax \ > --enable-whpx \ > --disable-spice \ > --enable-lzo \ > --enable-snappy \ > --enable-bzip2 \ > --enable-vdi \ > --enable-qcow1 \ > --enable-tools \ > --enable-libusb \ > --enable-usb-redir \ > --disable-libnfs \ > --enable-libssh ln: 无法创建符号链接 'aarch64-softmmu/qemu-system-aarch64': No such file or directory ln: 无法创建符号链接 'alpha-softmmu/qemu-system-alpha': No such file or directory ln: 无法创建符号链接 'arm-softmmu/qemu-system-arm': No such file or directory ln: 无法创建符号链接 'avr-softmmu/qemu-system-avr': No such file or directory ln: 无法创建符号链接 'cris-softmmu/qemu-system-cris': No such file or directory ln: 无法创建符号链接 'hppa-softmmu/qemu-system-hppa': No such file or directory ln: 无法创建符号链接 'i386-softmmu/qemu-system-i386': No such file or directory ln: 无法创建符号链接 'lm32-softmmu/qemu-system-lm32': No such file or directory ln: 无法创建符号链接 'm68k-softmmu/qemu-system-m68k': No such file or directory ln: 无法创建符号链接 'microblazeel-softmmu/qemu-system-microblazeel': No such file or directory ln: 无法创建符号链接 'microblaze-softmmu/qemu-system-microblaze': No such file or directory ln: 无法创建符号链接 'mips64el-softmmu/qemu-system-mips64el': No such file or directory ln: 无法创建符号链接 'mips64-softmmu/qemu-system-mips64': No such file or directory ln: 无法创建符号链接 'mipsel-softmmu/qemu-system-mipsel': No such file or directory ln: 无法创建符号链接 'mips-softmmu/qemu-system-mips': No such file or directory ln: 无法创建符号链接 'moxie-softmmu/qemu-system-moxie': No such file or directory ln: 无法创建符号链接 'nios2-softmmu/qemu-system-nios2': No such file or directory ln: 无法创建符号链接 'or1k-softmmu/qemu-system-or1k': No such file or directory ln: 无法创建符号链接 'ppc64-softmmu/qemu-system-ppc64': No such file or directory ln: 无法创建符号链接 'ppc-softmmu/qemu-system-ppc': No such file or directory ln: 无法创建符号链接 'riscv32-softmmu/qemu-system-riscv32': No such file or directory ln: 无法创建符号链接 'riscv64-softmmu/qemu-system-riscv64': No such file or directory ln: 无法创建符号链接 'rx-softmmu/qemu-system-rx': No such file or directory ln: 无法创建符号链接 's390x-softmmu/qemu-system-s390x': No such file or directory ln: 无法创建符号链接 'sh4eb-softmmu/qemu-system-sh4eb': No such file or directory ln: 无法创建符号链接 'sh4-softmmu/qemu-system-sh4': No such file or directory ln: 无法创建符号链接 'sparc64-softmmu/qemu-system-sparc64': No such file or directory ln: 无法创建符号链接 'sparc-softmmu/qemu-system-sparc': No such file or directory ln: 无法创建符号链接 'tricore-softmmu/qemu-system-tricore': No such file or directory ln: 无法创建符号链接 'unicore32-softmmu/qemu-system-unicore32': No such file or directory ln: 无法创建符号链接 'x86_64-softmmu/qemu-system-x86_64': No such file or directory ln: 无法创建符号链接 'xtensaeb-softmmu/qemu-system-xtensaeb': No such file or directory ln: 无法创建符号链接 'xtensa-softmmu/qemu-system-xtensa': No such file or directory cross containers no NOTE: guest cross-compilers enabled: cc The Meson build system Version: 0.55.0 Source dir: E:/CI-Cor-Ready/xemu/qemu.org Build dir: E:/CI-Cor-Ready/xemu/qemu.org-x64 Build type: cross build Using 'PKG_CONFIG_PATH' from environment with value: 'C:\\CI-Tools\\msys64\\mingw64\\lib\\pkgconfig;C:\\CI-Tools\\msys64\\mingw64\\share\\pkgconfig' Project name: qemu Project version: 5.1.50 C compiler for the build machine: cc (gcc 10.2.0 "cc (Rev1, Built by MSYS2 project) 10.2.0") C linker for the build machine: cc ld.bfd 2.34 C compiler for the host machine: x86_64-w64-mingw32-gcc (gcc 10.2.0 "x86_64-w64-mingw32-gcc (Rev1, Built by MSYS2 project) 10.2.0") C linker for the host machine: x86_64-w64-mingw32-gcc ld.bfd 2.34 Build machine cpu family: x86_64 Build machine cpu: x86_64 Host machine cpu family: x86 Host machine cpu: x86_64 Target machine cpu family: x86 Target machine cpu: x86_64 ../qemu.org/meson.build:9: WARNING: Module unstable-keyval has no backwards or forwards compatibility and might not exist in future releases. Program sh found: YES Program python3 found: YES (C:/CI-Tools/msys64/mingw64/bin/python3.exe) C++ compiler for the host machine: x86_64-w64-mingw32-g++ (gcc 10.2.0 "x86_64-w64-mingw32-g++ (Rev1, Built by MSYS2 project) 10.2.0") C++ linker for the host machine: x86_64-w64-mingw32-g++ ld.bfd 2.34 Configuring ninjatool using configuration Library m found: YES Library util found: NO Library ws2_32 found: YES Library winmm found: YES Windows resource compiler: GNU windres (GNU Binutils) 2.34 Library aio found: NO Library rt found: NO Found pkg-config: C:\CI-Tools\msys64\mingw64\bin/x86_64-w64-mingw32-pkg-config.EXE (0.29.2) Using 'PKG_CONFIG_PATH' from environment with value: 'C:\\CI-Tools\\msys64\\mingw64\\lib\\pkgconfig;C:\\CI-Tools\\msys64\\mingw64\\share\\pkgconfig' Run-time
[Bug 1892544] [NEW] meson qemu 5.2 can not built with msys2
Public bug reported: pacman -S base-devel mingw-w64-x86_64-toolchain git pacman -S mingw-w64-x86_64-python mingw-w64-x86_64-python-setuptools pacman -S mingw-w64-x86_64-spice-protocol cd /mingw64/bin cp x86_64-w64-mingw32-gcc-ar.exe x86_64-w64-mingw32-ar.exe cp x86_64-w64-mingw32-gcc-ranlib.exe x86_64-w64-mingw32-ranlib.exe cp windres.exe x86_64-w64-mingw32-windres.exe cp nm.exe x86_64-w64-mingw32-nm.exe cp objcopy.exe x86_64-w64-mingw32-objcopy.exe cd ~ cd qemu.org-x64 ../qemu.org/configure \ --python=python3 \ --cross-prefix=x86_64-w64-mingw32- --enable-gtk --enable-sdl \ --enable-capstone=git \ --enable-stack-protector \ --enable-gnutls \ --enable-nettle \ --enable-vnc \ --enable-vnc-sasl \ --enable-vnc-jpeg \ --enable-vnc-png \ --enable-membarrier \ --enable-slirp=git \ --disable-kvm \ --enable-hax \ --enable-whpx \ --disable-spice \ --enable-lzo \ --enable-snappy \ --enable-bzip2 \ --enable-vdi \ --enable-qcow1 \ --enable-tools \ --enable-libusb \ --enable-usb-redir \ --disable-libnfs \ --enable-libssh ** Affects: qemu Importance: Undecided Status: New -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1892544 Title: meson qemu 5.2 can not built with msys2 Status in QEMU: New Bug description: pacman -S base-devel mingw-w64-x86_64-toolchain git pacman -S mingw-w64-x86_64-python mingw-w64-x86_64-python-setuptools pacman -S mingw-w64-x86_64-spice-protocol cd /mingw64/bin cp x86_64-w64-mingw32-gcc-ar.exe x86_64-w64-mingw32-ar.exe cp x86_64-w64-mingw32-gcc-ranlib.exe x86_64-w64-mingw32-ranlib.exe cp windres.exe x86_64-w64-mingw32-windres.exe cp nm.exe x86_64-w64-mingw32-nm.exe cp objcopy.exe x86_64-w64-mingw32-objcopy.exe cd ~ cd qemu.org-x64 ../qemu.org/configure \ --python=python3 \ --cross-prefix=x86_64-w64-mingw32- --enable-gtk --enable-sdl \ --enable-capstone=git \ --enable-stack-protector \ --enable-gnutls \ --enable-nettle \ --enable-vnc \ --enable-vnc-sasl \ --enable-vnc-jpeg \ --enable-vnc-png \ --enable-membarrier \ --enable-slirp=git \ --disable-kvm \ --enable-hax \ --enable-whpx \ --disable-spice \ --enable-lzo \ --enable-snappy \ --enable-bzip2 \ --enable-vdi \ --enable-qcow1 \ --enable-tools \ --enable-libusb \ --enable-usb-redir \ --disable-libnfs \ --enable-libssh To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1892544/+subscriptions
[Bug 1892541] [NEW] qemu 5.1 on windows 10 with whpx can not install Windows 7 guest
Public bug reported: Command install and start win7 qemu-system-x86_64 -smbios type=1,uuid=e77aacd6-0acb-4a5c-9a83-a80d029b36f1 -smp 2,sockets=1,cores=2,maxcpus=2 -nodefaults -boot menu=on,strict=on,reboot-timeout=1000 -m 8192 ^ -readconfig pve-q35-4.0.cfg ^ -device vmgenid,guid=6d4865f5-353e-4cf1-b8ca-f5abbd062736 -device usb-tablet,id=tablet,bus=ehci.0,port=1 -device VGA,id=vga,bus=pcie.0,addr=0x1 ^ -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 ^ -drive file=en_windows_7_ultimate_with_sp1_x64_dvd_u_677332.iso,if=none,id=drive-ide2,media=cdrom,aio=threads ^ -device ide-cd,bus=ide.1,unit=0,drive=drive-ide2,id=ide2,bootindex=200 -device ahci,id=ahci0,multifunction=on,bus=pci.0,addr=0x7 ^ -drive id=drive-sata0,if=none,file=win7.qcow2,format=qcow2,cache=none,aio=native,detect-zeroes=on ^ -device ide-hd,bus=ahci0.0,drive=drive-sata0,id=sata0,bootindex=100 ^ -netdev type=tap,id=mynet0,ifname=tap1,script=no,downscript=no ^ -device e1000,netdev=mynet0,mac=52:55:00:d1:55:10,bus=pci.0,addr=0x12,id=net0,bootindex=300 ^ -machine type=q35,accel=whpx ** Affects: qemu Importance: Undecided Status: New -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1892541 Title: qemu 5.1 on windows 10 with whpx can not install Windows 7 guest Status in QEMU: New Bug description: Command install and start win7 qemu-system-x86_64 -smbios type=1,uuid=e77aacd6-0acb-4a5c-9a83-a80d029b36f1 -smp 2,sockets=1,cores=2,maxcpus=2 -nodefaults -boot menu=on,strict=on,reboot-timeout=1000 -m 8192 ^ -readconfig pve-q35-4.0.cfg ^ -device vmgenid,guid=6d4865f5-353e-4cf1-b8ca-f5abbd062736 -device usb-tablet,id=tablet,bus=ehci.0,port=1 -device VGA,id=vga,bus=pcie.0,addr=0x1 ^ -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 ^ -drive file=en_windows_7_ultimate_with_sp1_x64_dvd_u_677332.iso,if=none,id=drive-ide2,media=cdrom,aio=threads ^ -device ide-cd,bus=ide.1,unit=0,drive=drive-ide2,id=ide2,bootindex=200 -device ahci,id=ahci0,multifunction=on,bus=pci.0,addr=0x7 ^ -drive id=drive-sata0,if=none,file=win7.qcow2,format=qcow2,cache=none,aio=native,detect-zeroes=on ^ -device ide-hd,bus=ahci0.0,drive=drive-sata0,id=sata0,bootindex=100 ^ -netdev type=tap,id=mynet0,ifname=tap1,script=no,downscript=no ^ -device e1000,netdev=mynet0,mac=52:55:00:d1:55:10,bus=pci.0,addr=0x12,id=net0,bootindex=300 ^ -machine type=q35,accel=whpx To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1892541/+subscriptions
Re: [PULL 00/14] Linux user for 5.2 patches
Hello, On 21.8.20. 20:45, Laurent Vivier wrote: Filip, could you have a look to see what's going wrong? Thanks, LAurent Le 21/08/2020 à 18:23, Peter Maydell a écrit : On Thu, 13 Aug 2020 at 07:51, Laurent Vivier wrote: The following changes since commit d0ed6a69d399ae193959225cdeaa9382746c91cc: Update version for v5.1.0 release (2020-08-11 17:07:03 +0100) are available in the Git repository at: git://github.com/vivier/qemu.git tags/linux-user-for-5.2-pull-request for you to fetch changes up to 04275cad60c8f99e0dd7f56aecda68ceac926da8: linux-user: Fix 'utimensat()' implementation (2020-08-12 10:09:58 +0200) Add btrfs ioctls Add clock_getres_time64, timer_gettime64, timer_settime64, timerfd_gettime64, timerfd_settime64 Some fixes (page protection, print_fdset, timerspec, itimerspec) Fails to compile: ../../linux-user/syscall_types.h:407:28: error: ‘BTRFS_INO_LOOKUP_USER_PATH_MAX’ undeclared here (not in a function); did you mean ‘BTRFS_INO_LOOKUP_PATH_MAX’? MK_ARRAY(TYPE_CHAR, BTRFS_INO_LOOKUP_USER_PATH_MAX)) /* path */ ^ ../../linux-user/syscall_types.h:451:17: error: ‘BTRFS_MAX_ROOTREF_BUFFER_NUM’ undeclared here (not in a function) BTRFS_MAX_ROOTREF_BUFFER_NUM), /* rootref */ ^ On PPC, even more errors, relating to not having BTRFS_PATH_NAME_MAX, PTRFS_VOL_NAME_MAX, etc. Not sure if this was a semantic conflict with the meson conversion, or just an assumption of a newer btrfs.h than some systems have. Yes, this is because of asumption that a newer btrfs.h file is present than some systems have. There have been significant changes in btrfs.h from kernel version 3.9 (since it was introduced). For example, BTRFS_INO_LOOKPU_USER_PATH_MAX is used by BTRRS_IOC_INO_LOOKUP_USER which was introduced a little later which is why this error ocurres. I am guessing that this should be fixed by putting an #ifdef around thunk structure definitions in 'syscall_types.h' to check whether the values that are used by these structures are present (or if ioctls that use this structures are defined). I am sorry that I didn't notice this at first. I will try to send a fix as soon as possible. Best regards, Filip thanks -- PMM
[Bug 1892540] [NEW] qemu can no longer boot NetBSD/sparc
Public bug reported: Booting NetBSD/sparc in qemu no longer works. It broke between qemu version 5.0.0 and 5.1.0, and a bisection identified the following as the offending commit: [5d971f9e672507210e77d020d89e0e89165c8fc9] memory: Revert "memory: accept mismatching sizes in memory_region_access_valid" It's still broken as of 7fd51e68c34fcefdb4d6fd646ed3346f780f89f4. To reproduce, run wget http://ftp.netbsd.org/pub/NetBSD/NetBSD-9.0/images/NetBSD-9.0-sparc.iso qemu-system-sparc -nographic -cdrom NetBSD-9.0-sparc.iso -boot d The expected behavior is that the guest boots to the prompt Installation medium to load the additional utilities from: The observed behavior is a panic: [ 1.050] system[0]: trap 0x29: pc=0xf0046b14 sfsr=0xb6 sfva=0x5400 [ 1.050] cpu0: data fault: pc=0xf0046b14 addr=0x5400 sfsr=0xb6 [ 1.050] panic: kernel fault [ 1.050] halted ** Affects: qemu Importance: Undecided Status: New -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1892540 Title: qemu can no longer boot NetBSD/sparc Status in QEMU: New Bug description: Booting NetBSD/sparc in qemu no longer works. It broke between qemu version 5.0.0 and 5.1.0, and a bisection identified the following as the offending commit: [5d971f9e672507210e77d020d89e0e89165c8fc9] memory: Revert "memory: accept mismatching sizes in memory_region_access_valid" It's still broken as of 7fd51e68c34fcefdb4d6fd646ed3346f780f89f4. To reproduce, run wget http://ftp.netbsd.org/pub/NetBSD/NetBSD-9.0/images/NetBSD-9.0-sparc.iso qemu-system-sparc -nographic -cdrom NetBSD-9.0-sparc.iso -boot d The expected behavior is that the guest boots to the prompt Installation medium to load the additional utilities from: The observed behavior is a panic: [ 1.050] system[0]: trap 0x29: pc=0xf0046b14 sfsr=0xb6 sfva=0x5400 [ 1.050] cpu0: data fault: pc=0xf0046b14 addr=0x5400 sfsr=0xb6 [ 1.050] panic: kernel fault [ 1.050] halted To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1892540/+subscriptions
Re: [PULL 0/3] Block patches
On Mon, 17 Aug 2020 at 16:16, Stefan Hajnoczi wrote: > > The following changes since commit d0ed6a69d399ae193959225cdeaa9382746c91cc: > > Update version for v5.1.0 release (2020-08-11 17:07:03 +0100) > > are available in the Git repository at: > > https://github.com/stefanha/qemu.git tags/block-pull-request > > for you to fetch changes up to 44277bf914471962c9e88e09c859aae65ae109c4: > > aio-posix: keep aio_notify_me disabled during polling (2020-08-13 13:34:14 = > +0100) > > > Pull request > > Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/5.2 for any user-visible changes. -- PMM
Re: [PULL 00/14] Linux user for 5.2 patches
Le 21/08/2020 à 18:23, Peter Maydell a écrit : > On Thu, 13 Aug 2020 at 07:51, Laurent Vivier wrote: >> >> The following changes since commit d0ed6a69d399ae193959225cdeaa9382746c91cc: >> >> Update version for v5.1.0 release (2020-08-11 17:07:03 +0100) >> >> are available in the Git repository at: >> >> git://github.com/vivier/qemu.git tags/linux-user-for-5.2-pull-request >> >> for you to fetch changes up to 04275cad60c8f99e0dd7f56aecda68ceac926da8: >> >> linux-user: Fix 'utimensat()' implementation (2020-08-12 10:09:58 +0200) >> >> >> Add btrfs ioctls >> Add clock_getres_time64, timer_gettime64, timer_settime64, >> timerfd_gettime64, timerfd_settime64 >> Some fixes (page protection, print_fdset, timerspec, itimerspec) >> >> > > Fails to compile: > > ../../linux-user/syscall_types.h:407:28: error: > ‘BTRFS_INO_LOOKUP_USER_PATH_MAX’ undeclared here (not in a function); > did you mean ‘BTRFS_INO_LOOKUP_PATH_MAX’? > MK_ARRAY(TYPE_CHAR, BTRFS_INO_LOOKUP_USER_PATH_MAX)) /* path */ > ^ > > ../../linux-user/syscall_types.h:451:17: error: > ‘BTRFS_MAX_ROOTREF_BUFFER_NUM’ undeclared here (not in a function) > BTRFS_MAX_ROOTREF_BUFFER_NUM), /* rootref */ > ^ > > On PPC, even more errors, relating to not having > BTRFS_PATH_NAME_MAX, PTRFS_VOL_NAME_MAX, etc. > > Not sure if this was a semantic conflict with the meson > conversion, or just an assumption of a newer btrfs.h > than some systems have. Well, for me master is also broken (F32, linux-user only, static): Compiling C object libqemuutil.a.p/util_compatfd.c.o ../../../Projects/qemu/util/compatfd.c: In function 'qemu_signalfd': ../../../Projects/qemu/util/compatfd.c:103:19: error: 'SYS_signalfd' undeclared (first use in this function); did you mean 'SYS_signalfd4'? 103 | ret = syscall(SYS_signalfd, -1, mask, _NSIG / 8); | ^~~~ | SYS_signalfd4 ../../../Projects/qemu/util/compatfd.c:103:19: note: each undeclared identifier is reported only once for each function it appears in Thanks, Laurent
Re: [PATCH 08/41] opentitan: Rename memmap enum constants
On Mon, Aug 17, 2020 at 2:53 AM Bin Meng wrote: > > On Sat, Aug 15, 2020 at 1:56 AM Philippe Mathieu-Daudé > wrote: > > > > On 8/14/20 12:25 AM, Eduardo Habkost wrote: > > > Some of the enum constant names conflict with the QOM type check > > > macros. This needs to be addressed to allow us to transform the > > > QOM type check macros into functions generated by > > > OBJECT_DECLARE_TYPE(). > > > > > > Rename all the constants to IBEX_DEV_*, to avoid conflicts. > > > > > > Signed-off-by: Eduardo Habkost > > > --- > > > include/hw/riscv/opentitan.h | 38 > > > hw/riscv/opentitan.c | 84 ++-- > > > 2 files changed, 61 insertions(+), 61 deletions(-) > > > > > > diff --git a/include/hw/riscv/opentitan.h b/include/hw/riscv/opentitan.h > > > index 8f29b9cbbf..835a80f896 100644 > > > --- a/include/hw/riscv/opentitan.h > > > +++ b/include/hw/riscv/opentitan.h > > > @@ -49,25 +49,25 @@ typedef struct OpenTitanState { > > > } OpenTitanState; > > > > > > enum { > > > -IBEX_ROM, > > > -IBEX_RAM, > > > -IBEX_FLASH, > > > -IBEX_UART, > > > -IBEX_GPIO, > > > -IBEX_SPI, > > > -IBEX_FLASH_CTRL, > > > -IBEX_RV_TIMER, > > > -IBEX_AES, > > > -IBEX_HMAC, > > > -IBEX_PLIC, > > > -IBEX_PWRMGR, > > > -IBEX_RSTMGR, > > > -IBEX_CLKMGR, > > > -IBEX_PINMUX, > > > -IBEX_ALERT_HANDLER, > > > -IBEX_NMI_GEN, > > > -IBEX_USBDEV, > > > -IBEX_PADCTRL, > > > +IBEX_DEV_ROM, > > > +IBEX_DEV_RAM, > > > +IBEX_DEV_FLASH, > > > +IBEX_DEV_UART, > > > +IBEX_DEV_GPIO, > > > +IBEX_DEV_SPI, > > > +IBEX_DEV_FLASH_CTRL, > > > +IBEX_DEV_RV_TIMER, > > > +IBEX_DEV_AES, > > > +IBEX_DEV_HMAC, > > > +IBEX_DEV_PLIC, > > > +IBEX_DEV_PWRMGR, > > > +IBEX_DEV_RSTMGR, > > > +IBEX_DEV_CLKMGR, > > > +IBEX_DEV_PINMUX, > > > +IBEX_DEV_ALERT_HANDLER, > > > +IBEX_DEV_NMI_GEN, > > > +IBEX_DEV_USBDEV, > > > +IBEX_DEV_PADCTRL, > > > > Similarly, why is this enum in a public header and not local > > to opentitan.c, only place where it is used? > > > > I believe the reason is that opentitan.c implements both SoC and board > stuff. These enums are helpful to define SoC peripherals hence > technically another board that uses the same SoC may utilize these > macros. > > Unfortunately this is the case for all RISC-V boards so far. Should we > clean this up, for example, splitting SoC and board codes into 2 > files? Yeah the hw/riscv directory is a bit of a mess. We need to look at moving non machine parts out (like sifive_uart for example) and then it's probably worth splitting SoC and machine. It does result in some extra files (some of which are very simple) but the current setup is pretty confusing. Are you volunteering Bin? Otherwise I can look at it. Alistair > > Regards, > Bin >
Re: [PATCH 18/18] hw/riscv: microchip_pfsoc: Document the software used for testing
On Fri, Aug 14, 2020 at 9:49 AM Bin Meng wrote: > > From: Bin Meng > > Add some useful comments to document the software used for testing. > including how to patch HSS to bypass the DDR memory initialization, > HSS and Yocto BSP build instructions, etc. > > To launch this machine for testing: > $ qemu-system-riscv64 -M microchip-icicle-kit -smp 5 \ > -bios path/to/hss.bin -sd path/to/sdcard.img \ > -nic user,model=cadence_gem \ > -nic tap,ifname=tap,model=cadence_gem \ > -display none -serial stdio \ > -chardev socket,id=serial1,path=serial1.sock,server,wait \ > -serial chardev:serial1 > > Signed-off-by: Bin Meng > > --- > > hw/riscv/microchip_pfsoc.c | 21 + > 1 file changed, 21 insertions(+) > > diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c > index e8b7f86..1575fef 100644 > --- a/hw/riscv/microchip_pfsoc.c > +++ b/hw/riscv/microchip_pfsoc.c > @@ -56,6 +56,27 @@ > /* > * The BIOS image used by this machine is called Hart Software Services > (HSS). > * See https://github.com/polarfire-soc/hart-software-services > + * > + * As of now the DDR memory controller in the Microchip PolarFire SoC has not > + * been modeled. Simply creating unimplemented devices does not make HSS > happy. > + * Emulating the DDR memory controller is tedious, so a patched HSS should be > + * used as the BIOS for this machine. > + * > + * To patch HSS, open boards/icicle-kit-es/hss_board_init.c in the HSS source > + * tree, find the boardInitFunctions[] array that contains the initialization > + * routines for this board, and remove the line that contains 'HSS_DDRInit'. > + * > + * QEMU does not support eMMC hence the SD configuration shall be used in the > + * HSS and Yocto BSP build. The eMMC configuration is not supported. > + * > + * Instructions to build HSS: > + * $ cp boards/icicle-kit-es/def_config.sdcard .config > + * $ make BOARD=icicle-kit-es > + * > + * For Yocto build, "MACHINE=icicle-kit-es-sd" should be specified, otherwise > + * when booting Linux kernel the rootfs cannot be mounted. The generated > image > + * is something like: mpfs-dev-cli-icicle-kit-es-sd.rootfs.wic. Resize the > file > + * with 'qemu-image' to a power of 2 before passing to QEMU '-sd' command > line. Instead of adding comments in the code (where it's hard for users to find and prone to getting out of date) can you drop this patch and instead update the RISC-V QEMU wiki? https://wiki.qemu.org/Documentation/Platforms/RISCV Just add an Icicle section. Alistair > */ > #define BIOS_FILENAME "hss.bin" > #define RESET_VECTOR0x2022 > -- > 2.7.4 > >
Re: [PATCH 16/18] hw/riscv: microchip_pfsoc: Hook GPIO controllers
On Fri, Aug 14, 2020 at 9:53 AM Bin Meng wrote: > > From: Bin Meng > > Microchip PolarFire SoC integrates 3 GPIOs controllers. It seems > enough to create unimplemented devices to cover their register > spaces at this point. > > With this commit, QEMU can boot to U-Boot (2nd stage bootloader) > all the way to the Linux shell login prompt, with a modified HSS > (1st stage bootloader). > > Signed-off-by: Bin Meng Reviewed-by: Alistair Francis Alistair > --- > > hw/riscv/microchip_pfsoc.c | 14 ++ > include/hw/riscv/microchip_pfsoc.h | 3 +++ > 2 files changed, 17 insertions(+) > > diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c > index 625b511..139284a 100644 > --- a/hw/riscv/microchip_pfsoc.c > +++ b/hw/riscv/microchip_pfsoc.c > @@ -89,6 +89,9 @@ static const struct MemmapEntry { > [MICROCHIP_PFSOC_MMUART4] = { 0x20106000, 0x1000 }, > [MICROCHIP_PFSOC_GEM0] ={ 0x2011, 0x2000 }, > [MICROCHIP_PFSOC_GEM1] ={ 0x20112000, 0x2000 }, > +[MICROCHIP_PFSOC_GPIO0] = { 0x2012, 0x1000 }, > +[MICROCHIP_PFSOC_GPIO1] = { 0x20121000, 0x1000 }, > +[MICROCHIP_PFSOC_GPIO2] = { 0x20122000, 0x1000 }, > [MICROCHIP_PFSOC_ENVM_CFG] ={ 0x2020, 0x1000 }, > [MICROCHIP_PFSOC_ENVM_DATA] = { 0x2022,0x2 }, > [MICROCHIP_PFSOC_IOSCB_CFG] = { 0x3708, 0x1000 }, > @@ -308,6 +311,17 @@ static void microchip_pfsoc_soc_realize(DeviceState > *dev, Error **errp) > sysbus_connect_irq(SYS_BUS_DEVICE(&s->gem1), 0, > qdev_get_gpio_in(DEVICE(s->plic), MICROCHIP_PFSOC_GEM1_IRQ)); > > +/* GPIOs */ > +create_unimplemented_device("microchip.pfsoc.gpio0", > +memmap[MICROCHIP_PFSOC_GPIO0].base, > +memmap[MICROCHIP_PFSOC_GPIO0].size); > +create_unimplemented_device("microchip.pfsoc.gpio1", > +memmap[MICROCHIP_PFSOC_GPIO1].base, > +memmap[MICROCHIP_PFSOC_GPIO1].size); > +create_unimplemented_device("microchip.pfsoc.gpio2", > +memmap[MICROCHIP_PFSOC_GPIO2].base, > +memmap[MICROCHIP_PFSOC_GPIO2].size); > + > /* eNVM */ > memory_region_init_rom(envm_data, OBJECT(dev), > "microchip.pfsoc.envm.data", > memmap[MICROCHIP_PFSOC_ENVM_DATA].size, > diff --git a/include/hw/riscv/microchip_pfsoc.h > b/include/hw/riscv/microchip_pfsoc.h > index 60f994c..993b17c 100644 > --- a/include/hw/riscv/microchip_pfsoc.h > +++ b/include/hw/riscv/microchip_pfsoc.h > @@ -89,6 +89,9 @@ enum { > MICROCHIP_PFSOC_MMUART4, > MICROCHIP_PFSOC_GEM0, > MICROCHIP_PFSOC_GEM1, > +MICROCHIP_PFSOC_GPIO0, > +MICROCHIP_PFSOC_GPIO1, > +MICROCHIP_PFSOC_GPIO2, > MICROCHIP_PFSOC_ENVM_CFG, > MICROCHIP_PFSOC_ENVM_DATA, > MICROCHIP_PFSOC_IOSCB_CFG, > -- > 2.7.4 > >
Re: [PATCH 15/18] hw/riscv: microchip_pfsoc: Connect 2 Cadence GEMs
On Fri, Aug 14, 2020 at 9:51 AM Bin Meng wrote: > > From: Bin Meng > > Microchip PolarFire SoC integrates 2 Candence GEMs to provide > IEEE 802.3 standard-compliant 10/100/1000 Mbps ethernet interface. > > On the Icicle Kit board, GEM0 connects to a PHY at address 8 while > GEM1 connects to a PHY at address 9. > > The 2nd stage bootloader (U-Boot) is using GEM1 by default, so we > must specify 2 '-nic' options from the command line in order to get > a working ethernet. > > Signed-off-by: Bin Meng Reviewed-by: Alistair Francis Alistair > --- > > hw/riscv/microchip_pfsoc.c | 39 > ++ > include/hw/riscv/microchip_pfsoc.h | 7 +++ > 2 files changed, 46 insertions(+) > > diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c > index 1c67cbc..625b511 100644 > --- a/hw/riscv/microchip_pfsoc.c > +++ b/hw/riscv/microchip_pfsoc.c > @@ -14,6 +14,7 @@ > * 3) MMUARTs (Multi-Mode UART) > * 4) Cadence eMMC/SDHC controller and an SD card connected to it > * 5) DMA (Direct Memory Access Controller) > + * 6) GEM (Gigabit Ethernet MAC Controller) > * > * This board currently generates devicetree dynamically that indicates at > least > * two harts and up to five harts. > @@ -59,6 +60,9 @@ > #define BIOS_FILENAME "hss.bin" > #define RESET_VECTOR0x2022 > > +/* GEM version */ > +#define GEM_REVISION0x0107010c > + > static const struct MemmapEntry { > hwaddr base; > hwaddr size; > @@ -83,6 +87,8 @@ static const struct MemmapEntry { > [MICROCHIP_PFSOC_MMUART2] = { 0x20102000, 0x1000 }, > [MICROCHIP_PFSOC_MMUART3] = { 0x20104000, 0x1000 }, > [MICROCHIP_PFSOC_MMUART4] = { 0x20106000, 0x1000 }, > +[MICROCHIP_PFSOC_GEM0] ={ 0x2011, 0x2000 }, > +[MICROCHIP_PFSOC_GEM1] ={ 0x20112000, 0x2000 }, > [MICROCHIP_PFSOC_ENVM_CFG] ={ 0x2020, 0x1000 }, > [MICROCHIP_PFSOC_ENVM_DATA] = { 0x2022,0x2 }, > [MICROCHIP_PFSOC_IOSCB_CFG] = { 0x3708, 0x1000 }, > @@ -119,6 +125,9 @@ static void microchip_pfsoc_soc_instance_init(Object *obj) > object_initialize_child(obj, "dma-controller", &s->dma, > TYPE_MCHP_PFSOC_DMA); > > +object_initialize_child(obj, "gem0", &s->gem0, TYPE_CADENCE_GEM); > +object_initialize_child(obj, "gem1", &s->gem1, TYPE_CADENCE_GEM); > + > object_initialize_child(obj, "sd-controller", &s->sdhci, > TYPE_CADENCE_SDHCI); > object_initialize_child(OBJECT(&s->sdhci), "sd-controller.sdhci", > @@ -136,6 +145,7 @@ static void microchip_pfsoc_soc_realize(DeviceState *dev, > Error **errp) > MemoryRegion *envm_data = g_new(MemoryRegion, 1); > char *plic_hart_config; > size_t plic_hart_config_len; > +NICInfo *nd; > int i; > > sysbus_realize(SYS_BUS_DEVICE(&s->e_cpus), &error_abort); > @@ -269,6 +279,35 @@ static void microchip_pfsoc_soc_realize(DeviceState > *dev, Error **errp) > qdev_get_gpio_in(DEVICE(s->plic), MICROCHIP_PFSOC_MMUART4_IRQ), > serial_hd(4)); > > +/* GEMs */ > + > +nd = &nd_table[0]; > +if (nd->used) { > +qemu_check_nic_model(nd, TYPE_CADENCE_GEM); > +qdev_set_nic_properties(DEVICE(&s->gem0), nd); > +} > +nd = &nd_table[1]; > +if (nd->used) { > +qemu_check_nic_model(nd, TYPE_CADENCE_GEM); > +qdev_set_nic_properties(DEVICE(&s->gem1), nd); > +} > + > +object_property_set_int(OBJECT(&s->gem0), "revision", GEM_REVISION, > errp); > +object_property_set_int(OBJECT(&s->gem0), "phy-addr", 8, errp); > +sysbus_realize(SYS_BUS_DEVICE(&s->gem0), errp); > +sysbus_mmio_map(SYS_BUS_DEVICE(&s->gem0), 0, > +memmap[MICROCHIP_PFSOC_GEM0].base); > +sysbus_connect_irq(SYS_BUS_DEVICE(&s->gem0), 0, > +qdev_get_gpio_in(DEVICE(s->plic), MICROCHIP_PFSOC_GEM0_IRQ)); > + > +object_property_set_int(OBJECT(&s->gem1), "revision", GEM_REVISION, > errp); > +object_property_set_int(OBJECT(&s->gem1), "phy-addr", 9, errp); > +sysbus_realize(SYS_BUS_DEVICE(&s->gem1), errp); > +sysbus_mmio_map(SYS_BUS_DEVICE(&s->gem1), 0, > +memmap[MICROCHIP_PFSOC_GEM1].base); > +sysbus_connect_irq(SYS_BUS_DEVICE(&s->gem1), 0, > +qdev_get_gpio_in(DEVICE(s->plic), MICROCHIP_PFSOC_GEM1_IRQ)); > + > /* eNVM */ > memory_region_init_rom(envm_data, OBJECT(dev), > "microchip.pfsoc.envm.data", > memmap[MICROCHIP_PFSOC_ENVM_DATA].size, > diff --git a/include/hw/riscv/microchip_pfsoc.h > b/include/hw/riscv/microchip_pfsoc.h > index 7825935..60f994c 100644 > --- a/include/hw/riscv/microchip_pfsoc.h > +++ b/include/hw/riscv/microchip_pfsoc.h > @@ -24,6 +24,7 @@ > > #include "hw/char/mchp_pfsoc_mmuart.h" > #include "hw/dma/mchp_pfsoc_dma.h" > +#include "hw/net/cadence_gem.h" > #include "hw/sd/cadence_sdhc
Re: [PATCH v2] stdvga+bochs-display: add dummy mmio handler
On Fri, Aug 21, 2020 at 11:33 AM Alistair Francis wrote: > > On Mon, Mar 9, 2020 at 3:00 AM Gerd Hoffmann wrote: > > > > The bochs-display mmio bar has some sub-regions with the actual hardware > > registers. What happens when the guest access something outside those > > regions depends on the archirecture. On x86 those reads succeed (and > > return 0xff I think). On risc-v qemu aborts. > > > > This patch adds handlers for the parent region, to make the wanted > > behavior explicit and to make things consistent across architectures. > > > > v2: > > - use existing unassigned_io_ops. > > - also cover stdvga. > > > > Cc: Alistair Francis > > Signed-off-by: Gerd Hoffmann > > + QEMU stable. > > Can this be back ported to 5.0? Sorry, I meant 4.2 Alistair > > Without this patch the bochs device doesn't work with RISC-V. > > Alistair > > > --- > > hw/display/bochs-display.c | 4 ++-- > > hw/display/vga-pci.c | 8 > > 2 files changed, 6 insertions(+), 6 deletions(-) > > > > diff --git a/hw/display/bochs-display.c b/hw/display/bochs-display.c > > index 62085f9fc063..70eb619ef424 100644 > > --- a/hw/display/bochs-display.c > > +++ b/hw/display/bochs-display.c > > @@ -284,8 +284,8 @@ static void bochs_display_realize(PCIDevice *dev, Error > > **errp) > > memory_region_init_io(&s->qext, obj, &bochs_display_qext_ops, s, > >"qemu extended regs", PCI_VGA_QEXT_SIZE); > > > > -memory_region_init(&s->mmio, obj, "bochs-display-mmio", > > - PCI_VGA_MMIO_SIZE); > > +memory_region_init_io(&s->mmio, obj, &unassigned_io_ops, NULL, > > + "bochs-display-mmio", PCI_VGA_MMIO_SIZE); > > memory_region_add_subregion(&s->mmio, PCI_VGA_BOCHS_OFFSET, &s->vbe); > > memory_region_add_subregion(&s->mmio, PCI_VGA_QEXT_OFFSET, &s->qext); > > > > diff --git a/hw/display/vga-pci.c b/hw/display/vga-pci.c > > index b34632467399..6b9db86e363c 100644 > > --- a/hw/display/vga-pci.c > > +++ b/hw/display/vga-pci.c > > @@ -249,8 +249,8 @@ static void pci_std_vga_realize(PCIDevice *dev, Error > > **errp) > > > > /* mmio bar for vga register access */ > > if (d->flags & (1 << PCI_VGA_FLAG_ENABLE_MMIO)) { > > -memory_region_init(&d->mmio, NULL, "vga.mmio", > > - PCI_VGA_MMIO_SIZE); > > +memory_region_init_io(&d->mmio, OBJECT(dev), &unassigned_io_ops, > > NULL, > > + "vga.mmio", PCI_VGA_MMIO_SIZE); > > > > if (d->flags & (1 << PCI_VGA_FLAG_ENABLE_QEXT)) { > > qext = true; > > @@ -285,8 +285,8 @@ static void pci_secondary_vga_realize(PCIDevice *dev, > > Error **errp) > > s->con = graphic_console_init(DEVICE(dev), 0, s->hw_ops, s); > > > > /* mmio bar */ > > -memory_region_init(&d->mmio, OBJECT(dev), "vga.mmio", > > - PCI_VGA_MMIO_SIZE); > > +memory_region_init_io(&d->mmio, OBJECT(dev), &unassigned_io_ops, NULL, > > + "vga.mmio", PCI_VGA_MMIO_SIZE); > > > > if (d->flags & (1 << PCI_VGA_FLAG_ENABLE_QEXT)) { > > qext = true; > > -- > > 2.18.2 > >
Re: [PULL 00/14] Linux user for 5.2 patches
Filip, could you have a look to see what's going wrong? Thanks, LAurent Le 21/08/2020 à 18:23, Peter Maydell a écrit : > On Thu, 13 Aug 2020 at 07:51, Laurent Vivier wrote: >> >> The following changes since commit d0ed6a69d399ae193959225cdeaa9382746c91cc: >> >> Update version for v5.1.0 release (2020-08-11 17:07:03 +0100) >> >> are available in the Git repository at: >> >> git://github.com/vivier/qemu.git tags/linux-user-for-5.2-pull-request >> >> for you to fetch changes up to 04275cad60c8f99e0dd7f56aecda68ceac926da8: >> >> linux-user: Fix 'utimensat()' implementation (2020-08-12 10:09:58 +0200) >> >> >> Add btrfs ioctls >> Add clock_getres_time64, timer_gettime64, timer_settime64, >> timerfd_gettime64, timerfd_settime64 >> Some fixes (page protection, print_fdset, timerspec, itimerspec) >> >> > > Fails to compile: > > ../../linux-user/syscall_types.h:407:28: error: > ‘BTRFS_INO_LOOKUP_USER_PATH_MAX’ undeclared here (not in a function); > did you mean ‘BTRFS_INO_LOOKUP_PATH_MAX’? > MK_ARRAY(TYPE_CHAR, BTRFS_INO_LOOKUP_USER_PATH_MAX)) /* path */ > ^ > > ../../linux-user/syscall_types.h:451:17: error: > ‘BTRFS_MAX_ROOTREF_BUFFER_NUM’ undeclared here (not in a function) > BTRFS_MAX_ROOTREF_BUFFER_NUM), /* rootref */ > ^ > > On PPC, even more errors, relating to not having > BTRFS_PATH_NAME_MAX, PTRFS_VOL_NAME_MAX, etc. > > Not sure if this was a semantic conflict with the meson > conversion, or just an assumption of a newer btrfs.h > than some systems have. > > thanks > -- PMM >
Re: [PATCH v2] stdvga+bochs-display: add dummy mmio handler
On Mon, Mar 9, 2020 at 3:00 AM Gerd Hoffmann wrote: > > The bochs-display mmio bar has some sub-regions with the actual hardware > registers. What happens when the guest access something outside those > regions depends on the archirecture. On x86 those reads succeed (and > return 0xff I think). On risc-v qemu aborts. > > This patch adds handlers for the parent region, to make the wanted > behavior explicit and to make things consistent across architectures. > > v2: > - use existing unassigned_io_ops. > - also cover stdvga. > > Cc: Alistair Francis > Signed-off-by: Gerd Hoffmann + QEMU stable. Can this be back ported to 5.0? Without this patch the bochs device doesn't work with RISC-V. Alistair > --- > hw/display/bochs-display.c | 4 ++-- > hw/display/vga-pci.c | 8 > 2 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/hw/display/bochs-display.c b/hw/display/bochs-display.c > index 62085f9fc063..70eb619ef424 100644 > --- a/hw/display/bochs-display.c > +++ b/hw/display/bochs-display.c > @@ -284,8 +284,8 @@ static void bochs_display_realize(PCIDevice *dev, Error > **errp) > memory_region_init_io(&s->qext, obj, &bochs_display_qext_ops, s, >"qemu extended regs", PCI_VGA_QEXT_SIZE); > > -memory_region_init(&s->mmio, obj, "bochs-display-mmio", > - PCI_VGA_MMIO_SIZE); > +memory_region_init_io(&s->mmio, obj, &unassigned_io_ops, NULL, > + "bochs-display-mmio", PCI_VGA_MMIO_SIZE); > memory_region_add_subregion(&s->mmio, PCI_VGA_BOCHS_OFFSET, &s->vbe); > memory_region_add_subregion(&s->mmio, PCI_VGA_QEXT_OFFSET, &s->qext); > > diff --git a/hw/display/vga-pci.c b/hw/display/vga-pci.c > index b34632467399..6b9db86e363c 100644 > --- a/hw/display/vga-pci.c > +++ b/hw/display/vga-pci.c > @@ -249,8 +249,8 @@ static void pci_std_vga_realize(PCIDevice *dev, Error > **errp) > > /* mmio bar for vga register access */ > if (d->flags & (1 << PCI_VGA_FLAG_ENABLE_MMIO)) { > -memory_region_init(&d->mmio, NULL, "vga.mmio", > - PCI_VGA_MMIO_SIZE); > +memory_region_init_io(&d->mmio, OBJECT(dev), &unassigned_io_ops, > NULL, > + "vga.mmio", PCI_VGA_MMIO_SIZE); > > if (d->flags & (1 << PCI_VGA_FLAG_ENABLE_QEXT)) { > qext = true; > @@ -285,8 +285,8 @@ static void pci_secondary_vga_realize(PCIDevice *dev, > Error **errp) > s->con = graphic_console_init(DEVICE(dev), 0, s->hw_ops, s); > > /* mmio bar */ > -memory_region_init(&d->mmio, OBJECT(dev), "vga.mmio", > - PCI_VGA_MMIO_SIZE); > +memory_region_init_io(&d->mmio, OBJECT(dev), &unassigned_io_ops, NULL, > + "vga.mmio", PCI_VGA_MMIO_SIZE); > > if (d->flags & (1 << PCI_VGA_FLAG_ENABLE_QEXT)) { > qext = true; > -- > 2.18.2 >
Re: [PULL 00/24] target/xtensa updates for 5.2
On Fri, Aug 21, 2020 at 9:24 AM Peter Maydell wrote: > On Thu, 13 Aug 2020 at 00:24, Max Filippov wrote: > > please pull the following batch of updates for target/xtensa. > > Hi; this conflicts with the meson buildsystem merge, I'm > afraid -- can you rebase and resend, please? Sure. -- Thanks. -- Max
Re: [PATCH 00/18] hw/riscv: Add Microchip PolarFire SoC Icicle Kit board support
On Wed, Aug 19, 2020 at 3:13 AM wrote: > > On 8/19/20 2:34 AM, Bin Meng wrote: > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the > > content is safe > > > > On Tue, Aug 18, 2020 at 9:55 PM Anup Patel wrote: > >> On Tue, Aug 18, 2020 at 6:39 PM wrote: > >>> On 8/18/20 7:17 AM, Anup Patel wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know > the content is safe > > On Tue, Aug 18, 2020 at 1:23 AM wrote: > > On 8/17/20 8:28 PM, Alistair Francis wrote: > >> EXTERNAL EMAIL: Do not click links or open attachments unless you know > >> the content is safe > >> > >> On Mon, Aug 17, 2020 at 11:12 AM via wrote: > >>> Hi Anup, > >>> > >>> On 8/17/20 11:30 AM, Bin Meng wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you > know the content is safe > > Hi Anup, > > On Sat, Aug 15, 2020 at 1:44 AM Anup Patel > wrote: > > On Fri, Aug 14, 2020 at 10:12 PM Bin Meng > > wrote: > >> From: Bin Meng > >> > >> This adds support for Microchip PolarFire SoC Icicle Kit board. > >> The Icicle Kit board integrates a PolarFire SoC, with one SiFive's > >> E51 plus four U54 cores and many on-chip peripherals and an FPGA. > > Nice Work !!! This is very helpful. > Thanks! > > > The Microchip HSS is quite convoluted. It has: > > 1. DDR Init > > 2. Boot device support > > 3. SBI support using OpenSBI as library > > 4. Simple TEE support > > > > I think point 1) and 2) above should be part of U-Boot SPL. > > The point 3) can be OpenSBI FW_DYNAMIC. > > > > Lastly,for point 4), we are working on a new OpenSBI feature using > > which we can run independent Secure OS and Non-Secure OS using > > U-Boot_SPL+OpenSBI (for both SiFive Unleashed and Microchip > > PolarFire). > > > > Do you have plans for adding U-Boot SPL support for this board ?? > + Cyril Jean from Microchip > > I will have to leave this question to Cyril to comment. > > >>> I currently do not have a plan to support U-Boot SPL. The idea of the > >>> HSS is to contain all the silicon specific initialization and > >>> configuration code within the HSS before jumping to U-Boot S-mode. I > >>> would rather keep all this within the HSS for the time being. I would > >>> wait until we reach production silicon before attempting to move this > >>> to > >>> U-Boot SPL as the HSS is likely to contain some opaque silicon related > >>> changes for another while. > >> That is unfortunate, a lot of work has gone into making the boot flow > >> simple and easy to use. > >> > >> QEMU now includes OpenSBI by default to make it easy for users to boot > >> Linux. The Icicle Kit board is now the most difficult QEMU board to > >> boot Linux on. Not to mention it makes it hard to impossible to > >> support it in standard tool flows such as meta-riscv. > >> > >> Alistair > > If it is such a problem we can add a U-Boot SPL stage and the HSS can be > > treated as standard SoC ROM code. > It's not mandatory for U-Boot SPL to have stable DRAM calibration > settings > from the start itself. The initial U-Boot SPL support for most > platforms (accross > architectures) usually include basic working DRAM calibration settings > which > is later on updated with separate patches. Also, we don't need all U-Boot > drivers to be upstreamed in one-go as this can be done in phases. > > The advantage we have with PolarFire SoC Icicle board is that we already > have a U-Boot S-mode. and we believe the OpenSBI generic platform will > work fine for PolarFire SoC Icicle board so the only thing missing right > now > is the U-Boot SPL support for OpenSource boot-flow. > > It will certainly accelerate open-source development if we have boot-flow > U-Boot_SPL => OpenSBI (FW_DYNAMIC) => U-Boot_S-mode working > on PolarFire SoC Icicle board. Initially, we just need DRAM, SD/eMMC, > and Serial port support for U-Boot SPL and U-Boot S-mode. Later on, > more patches can add ethernet and other booting device drivers to U-Boot. > > Regarding security services of HSS, we are working on a OpenSBI > feature which will allow HSS security services to run as independent > binary in M-mode (not linked to OpenSBI) and OpenSBI FW_DYNAMIC > will be a separate binary acting as a secure monitor. > > Regards, > Anup > >>> What I have in mind is that the external memory will be up and running > >>> by the time we get to U-Boot SPL. In the case of PolarFire SoC the ROM > >>> code equivalent brings up
Re: Suspicious QOM types without instance/class size
On Fri, Aug 21, 2020 at 11:40:12AM +0200, David Hildenbrand wrote: > On 20.08.20 23:55, Eduardo Habkost wrote: > > While trying to convert TypeInfo declarations to the new > > OBJECT_DECLARE* macros, I've stumbled on a few suspicious cases > > where instance_size or class_size is not set, despite having type > > checker macros that use a specific type. > > > > The ones with "WARNING" are abstract types (maybe not serious if > > subclasses set the appropriate sizes). The ones with "ERROR" > > don't seem to be abstract types. [...] > > ERROR: hw/s390x/virtio-ccw.c:1237:1: class_size should be set to > > sizeof(VirtioCcwBusClass)? > > The parent of TYPE_VIRTIO_CCW_BUS is TYPE_VIRTIO_BUS. > > typedef struct VirtioBusClass VirtioCcwBusClass; > > So I guess the sizes match? Anyhow, setting doesn't hurt. Thanks for checking. Yeah, the sizes match today. It's a good idea to set it, just in case a real VirtioCcwBusClass struct gets created one day. -- Eduardo
Re: [PATCH v2 12/58] virtio-ccw: Fix definition of VIRTIO_CCW_BUS_GET_CLASS
On 20.08.20 02:11, Eduardo Habkost wrote: > The macro was incorrectly defined using OBJECT_CHECK. > > Acked-by: Cornelia Huck > Reviewed-by: Daniel P. Berrangé > Signed-off-by: Eduardo Habkost > --- > Changes v1 -> v2: none > > --- > Cc: "Michael S. Tsirkin" > Cc: Cornelia Huck > Cc: Halil Pasic > Cc: Christian Borntraeger > Cc: Richard Henderson > Cc: David Hildenbrand > Cc: Thomas Huth > Cc: qemu-s3...@nongnu.org > Cc: qemu-devel@nongnu.org > --- > hw/s390x/virtio-ccw.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/hw/s390x/virtio-ccw.h b/hw/s390x/virtio-ccw.h > index c0e3355248..b281896f7d 100644 > --- a/hw/s390x/virtio-ccw.h > +++ b/hw/s390x/virtio-ccw.h > @@ -65,9 +65,9 @@ typedef struct VirtioBusClass VirtioCcwBusClass; > > #define TYPE_VIRTIO_CCW_BUS "virtio-ccw-bus" > #define VIRTIO_CCW_BUS(obj) \ > - OBJECT_CHECK(VirtioCcwBus, (obj), TYPE_VIRTIO_CCW_BUS) > + OBJECT_CHECK(VirtioCcwBusState, (obj), TYPE_VIRTIO_CCW_BUS) > #define VIRTIO_CCW_BUS_GET_CLASS(obj) \ > -OBJECT_CHECK(VirtioCcwBusState, (obj), TYPE_VIRTIO_CCW_BUS) > +OBJECT_GET_CLASS(VirtioCcwBusClass, (obj), TYPE_VIRTIO_CCW_BUS) > #define VIRTIO_CCW_BUS_CLASS(klass) \ > OBJECT_CLASS_CHECK(VirtioCcwBusClass, klass, TYPE_VIRTIO_CCW_BUS) > > Reviewed-by: David Hildenbrand -- Thanks, David / dhildenb
Re: [PATCH v2] configure: silence 'shift' error message in version_ge()
On 8/21/20 11:33 AM, Stefano Garzarella wrote: If there are less than 2 arguments in version_ge(), the second 'shift' prints this error: ../configure: line 232: shift: shift count out of range Let's skip it if there are no more arguments. Signed-off-by: Stefano Garzarella --- v2: - do not shift if there are no more arguments [Peter] --- configure | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure b/configure index 4e5fe33211..5f5f370e2c 100755 --- a/configure +++ b/configure @@ -229,7 +229,7 @@ version_ge () { set x $local_ver1 local_first=${2-0} # shift 2 does nothing if there are less than 2 arguments -shift; shift +shift; test $# -gt 0 && shift That works. Or you could go with the shorter one-liner: shift ${2:+2} -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH v1 0/2] pc-bios: petalogix: Re-generate DTB and add DTS files
On Fri, Aug 21, 2020 at 01:02:59PM +0200, Philippe Mathieu-Daudé wrote: > Hi Edgar, > > On 8/20/20 9:43 PM, Edgar E. Iglesias wrote: > > From: "Edgar E. Iglesias" > > > > This adds missing device-tree source files for the petalogix boards > > with recompiled DTBs. > > > > Cheers, > > Edgar > > > > Edgar E. Iglesias (2): > > microblaze: petalogix-ml605: Add device-tree source > > microblaze: petalogix-s3adsp1800: Add device-tree source > > > > pc-bios/petalogix-ml605.dtb | Bin 9982 -> 9882 bytes > > pc-bios/petalogix-ml605.dts | 350 +++ > > pc-bios/petalogix-s3adsp1800.dtb | Bin 8259 -> 8161 bytes > > pc-bios/petalogix-s3adsp1800.dts | 282 + > > 4 files changed, 632 insertions(+) > > create mode 100644 pc-bios/petalogix-ml605.dts > > create mode 100644 pc-bios/petalogix-s3adsp1800.dts > > > > I suppose the test_microblaze_s3adsp1800() test in > tests/acceptance/boot_linux_console.py has the dtb embedded after the > kernel. Not sure, but I'll see if I can figure it out. > > Do you mind adding another test explicitly using this dtb? > That would cover any further changes in these files. Sounds like a good idea! Thanks, Edgar > > Also, can you add a test for the kc705 EVB (and previous) > boards as you suggested here? > https://www.mail-archive.com/qemu-devel@nongnu.org/msg605124.html > > I already added a test for the cris/etraxfs board, will send > it soon. Well you might wait for it and use it as template too. > > Anyway, > Reviewed-by: Philippe Mathieu-Daudé >
Re: Suspicious QOM types without instance/class size
On Fri, Aug 21, 2020 at 11:47:32AM +1000, David Gibson wrote: > On Thu, Aug 20, 2020 at 05:55:29PM -0400, Eduardo Habkost wrote: > > While trying to convert TypeInfo declarations to the new > > OBJECT_DECLARE* macros, I've stumbled on a few suspicious cases > > where instance_size or class_size is not set, despite having type > > checker macros that use a specific type. > > > > The ones with "WARNING" are abstract types (maybe not serious if > > subclasses set the appropriate sizes). The ones with "ERROR" > > don't seem to be abstract types. > > > Comment on the ones within my area: > > > > WARNING: hw/input/adb.c:310:1: class_size should be set to > > sizeof(ADBDeviceClass)? > > Yeah, that looks like a bug (though we'll get away with it because > it's abstract). Right, luckily we are not touching any ADBDeviceClass field inside adb_device_class_init(). > > > WARNING: hw/ppc/pnv_lpc.c:771:1: instance_size should be set to > > sizeof(PnvLpcController)? > > Ditto. Agreed. > > Should I make fixes for these, or will you? Please send the fixes, and I will apply them before running the TypeInfo conversion script. > > > ERROR: hw/ppc/spapr_drc.c:771:1: instance_size should be set to > > sizeof(SpaprDrc)? > > I'm confused by this one. I'm not exactly sure which definition is > tripping the error, and AFAICT they should all be correctly inheriting > instance_size from either TYPE_SPAPR_DR_CONNECTOR or > TYPE_SPAPR_DRC_PHSYICAL. If anything, it looks like > TYPE_SPAPR_DRC_PHB could drop it's explicit override of instance_size. The error is triggered because of this type checking macro at include/hw/ppc/spapr_drc.h: #define SPAPR_DRC_PCI(obj) OBJECT_CHECK(SpaprDrc, (obj), \ TYPE_SPAPR_DRC_PCI) The expectation is that whatever type you use in OBJECT_CHECK will be the one used for instance_size. The script also looks at the parent type, to reduce false positives, but this case was flagged because SPAPR_DRC_PCI uses SpaprDrc, but the parent type (SPAPR_DRC_PHYSICAL) uses SpaprDrcPhysical. Now, I don't understand why we have so many instance checker macros that use the same typedef (SpaprDrc). If the code needs a valid SpaprDrc pointer, it can just use SPAPR_DR_CONNECTOR(). -- Eduardo
Re: [PATCH] hw/ssi/ssi: Set abstract TYPE_SSI_SLAVE instance size
On Fri, Aug 21, 2020 at 10:46 AM Philippe Mathieu-Daudé wrote: > > Set the abstract TYPE_SSI_SLAVE instance size in case an > implementation forgot to set it. > > Reported-by: Eduardo Habkost > Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Alistair Francis Alistair > --- > See: > https://www.mail-archive.com/qemu-devel@nongnu.org/msg731954.html > --- > hw/ssi/ssi.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/hw/ssi/ssi.c b/hw/ssi/ssi.c > index a35d7ebb266..b3e82470381 100644 > --- a/hw/ssi/ssi.c > +++ b/hw/ssi/ssi.c > @@ -85,6 +85,7 @@ static void ssi_slave_class_init(ObjectClass *klass, void > *data) > static const TypeInfo ssi_slave_info = { > .name = TYPE_SSI_SLAVE, > .parent = TYPE_DEVICE, > +.instance_size = sizeof(SSISlave), > .class_init = ssi_slave_class_init, > .class_size = sizeof(SSISlaveClass), > .abstract = true, > -- > 2.26.2 > >
Re: [PATCH] configure: silence 'shift' error message in version_ge()
On 8/21/20 10:21 AM, Peter Maydell wrote: On Fri, 21 Aug 2020 at 16:00, Stefano Garzarella wrote: If there are less than 2 arguments in version_ge(), the second shift prints this error: ../configure: line 232: shift: shift count out of range Let's shut it up, since we're expecting this situation. Signed-off-by: Stefano Garzarella --- configure | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure b/configure index 4e5fe33211..de4bd0df36 100755 --- a/configure +++ b/configure @@ -229,7 +229,7 @@ version_ge () { set x $local_ver1 local_first=${2-0} # shift 2 does nothing if there are less than 2 arguments -shift; shift +shift; shift 2>/dev/null POSIX says https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#shift "If the n operand is invalid or is greater than "$#", this may be considered a syntax error and a non-interactive shell may exit" so I think that we need to actually avoid the excess shift, not just suppress any warning it might print. (I'm not sure Philippe's "shift || shift" patch can work for that, though, as the exit status doesn't distinguish "valid shift but don't do it again" from "valid shift and more args to come".) Indeed. 'shift || shift' is not going to work. 'shift; shift 2>/dev/null' is risky, as it can cause the shell to exit. But this does exactly what you want: shift ${2:+2} which works out to 'shift 2' if $2 is set, or 'shift' (implicitly shift 1) if $2 is not set. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH] target/s390x: fix meson.build issue
On 8/21/20 5:52 PM, Paolo Bonzini wrote: > files() is needed to avoid > > ../meson.build:977:2: ERROR: File tcg-stub.c does not exist. > > Signed-off-by: Paolo Bonzini > --- > target/s390x/meson.build | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/target/s390x/meson.build b/target/s390x/meson.build > index d2a3315903..c42eadb7d2 100644 > --- a/target/s390x/meson.build > +++ b/target/s390x/meson.build > @@ -21,7 +21,7 @@ s390x_ss.add(when: 'CONFIG_TCG', if_true: files( >'vec_helper.c', >'vec_int_helper.c', >'vec_string_helper.c', > -), if_false: 'tcg-stub.c') > +), if_false: files('tcg-stub.c')) > > s390x_ss.add(when: 'CONFIG_KVM', if_true: files('kvm.c'), if_false: > files('kvm-stub.c')) > Reviewed-by: Philippe Mathieu-Daudé
[Bug 1892533] Re: Meson: Missing config-host.mak
btw, I'm surprised README does not mention meson, shouldn't you instruct that it's a build-dep? Maybe suggest pip install command? -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1892533 Title: Meson: Missing config-host.mak Status in QEMU: New Bug description: Wanted to give a try to the new build system, but a simple "meson build" gives that error: meson.build:15:0: ERROR: Failed to load /home/xclaesse/programmation/qemu/build/config-host.mak: [Errno 2] No such file or directory: '/home/xclaesse/programmation/qemu/build /config-host.mak' To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1892533/+subscriptions
Re: [External] Re: [PATCH] virtiofsd: add -o allow_directio|no_directio option
On Fri, Aug 21, 2020 at 9:40 PM Philippe Mathieu-Daudé wrote: > On 8/21/20 1:58 PM, Daniel P. Berrangé wrote: > > On Fri, Aug 21, 2020 at 11:41:26AM +0800, Jiachen Zhang wrote: > >> Due to the commit 65da4539803373ec4eec97ffc49ee90083e56efd, the O_DIRECT > >> open flag of guest applications will be discarded by virtiofsd. While > >> this behavior makes it consistent with the virtio-9p scheme when guest > >> applications using direct I/O, we no longer have any chance to bypass > >> the host page cache. > >> > >> Therefore, we add a flag 'allow_directio' to lo_data. If '-o > no_directio' > >> option is added, or none of '-o no_directio' or '-o allow_directio' is > >> added, the 'allow_directio' will be set to 0, and virtiofsd discards > >> O_DIRECT as before. If '-o allow_directio' is added to the stariting > >> command-line, 'allow_directio' will be set to 1, so that the O_DIRECT > >> flags will be retained and host page cache can be bypassed. > >> > >> Signed-off-by: Jiachen Zhang > >> --- > >> tools/virtiofsd/helper.c | 4 > >> tools/virtiofsd/passthrough_ll.c | 20 ++-- > >> 2 files changed, 18 insertions(+), 6 deletions(-) > >> > >> diff --git a/tools/virtiofsd/helper.c b/tools/virtiofsd/helper.c > >> index 3105b6c23a..534ff52c64 100644 > >> --- a/tools/virtiofsd/helper.c > >> +++ b/tools/virtiofsd/helper.c > >> @@ -180,6 +180,10 @@ void fuse_cmdline_help(void) > >> " (0 leaves rlimit > unchanged)\n" > >> " default: min(100, > fs.file-max - 16384)\n" > >> "if the current > rlimit is lower\n" > >> + "-o allow_directio|no_directio\n" > >> + " retain/discard O_DIRECT > flags passed down\n" > >> + " to virtiofsd from guest > applications.\n" > >> + " default: no_directio\n" > >> ); > > > > The standard naming convention from existing options is to use > > $OPTNAME and no_$OPTNAME. > > > > IOW, don't use the "allow_" prefix. The options should be just > > "directio" and "no_directio" > > As we have 'max_idle_threads' (and not maxidlethreads), can we > use 'direct_io' instead? > > Thanks. I will split them in the next version. Jiachen > > > Regards, > > Daniel > > > >
Re: [PATCH] keymaps: update
On 8/21/20 6:14 PM, Paolo Bonzini wrote: > Looks like update-keymaps has not been run in a while. > > Signed-off-by: Paolo Bonzini > --- > pc-bios/keymaps/ar| 242 +- > pc-bios/keymaps/bepo | 242 +- > pc-bios/keymaps/cz| 242 +- > pc-bios/keymaps/da| 242 +- > pc-bios/keymaps/de| 242 +- > pc-bios/keymaps/de-ch | 242 +- > pc-bios/keymaps/en-gb | 242 +- > pc-bios/keymaps/en-us | 242 +- > pc-bios/keymaps/es| 242 +- > pc-bios/keymaps/et| 242 +- > pc-bios/keymaps/fi| 242 +- > pc-bios/keymaps/fo| 242 +- > pc-bios/keymaps/fr| 242 +- > pc-bios/keymaps/fr-be | 242 +- > pc-bios/keymaps/fr-ca | 242 +- > pc-bios/keymaps/fr-ch | 242 +- > pc-bios/keymaps/hr| 242 +- > pc-bios/keymaps/hu| 242 +- > pc-bios/keymaps/is| 242 +- > pc-bios/keymaps/it| 242 +- > pc-bios/keymaps/ja| 242 +- > pc-bios/keymaps/lt| 242 +- > pc-bios/keymaps/lv| 242 +- > pc-bios/keymaps/mk| 242 +- > pc-bios/keymaps/nl| 242 +- > pc-bios/keymaps/no| 242 +- > pc-bios/keymaps/pl| 242 +- > pc-bios/keymaps/pt| 242 +- > pc-bios/keymaps/pt-br | 242 +- > pc-bios/keymaps/ru| 242 +- > pc-bios/keymaps/th| 242 +- > pc-bios/keymaps/tr| 242 +- > 32 files changed, 7680 insertions(+), 64 deletions(-) Typical pre-release CI job.
[Bug 1892533] Re: Meson: Missing config-host.mak
Meson is still hidden, you need to use ../configure. "can't shift that many" will be fixed shortly (patch already on the list). -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1892533 Title: Meson: Missing config-host.mak Status in QEMU: New Bug description: Wanted to give a try to the new build system, but a simple "meson build" gives that error: meson.build:15:0: ERROR: Failed to load /home/xclaesse/programmation/qemu/build/config-host.mak: [Errno 2] No such file or directory: '/home/xclaesse/programmation/qemu/build /config-host.mak' To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1892533/+subscriptions
Re: [PATCH v8 1/1] audio/jack: fix use after free segfault
On 21/08/20 19:34, Christian Schoenebeck wrote: >> >> static void qjack_fini_out(HWVoiceOut *hw) >> { >> QJackOut *jo = (QJackOut *)hw; >> qjack_client_fini(&jo->c); >> + >> +qemu_bh_delete(jo->c.shutdown_bh); > Paolo wrapped that qemu_bh_delete() call inside the lock as well. So I guess > it makes a difference for the BH API? It is not a problem as long as qjack_client_fini is idempotent. >> +qemu_mutex_destroy(&jo->c.shutdown_lock); >> } > > Hmmm, is this qemu_mutex_destroy() safe at this point? Perhaps make the mutex global and not destroy it at all. Paolo
Re: [PATCH] tests/docker: add liburing-devel to the Fedora image
On 8/21/20 6:54 PM, Stefano Garzarella wrote: > Install liburing-devel dependencies to get better coverage on > io-uring stuff (block/io_uring.c and util/fdmon-io_uring.c). > > Suggested-by: Philippe Mathieu-Daudé > Signed-off-by: Stefano Garzarella > --- > tests/docker/dockerfiles/fedora.docker | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/tests/docker/dockerfiles/fedora.docker > b/tests/docker/dockerfiles/fedora.docker > index 70b6186bd3..9650d324fa 100644 > --- a/tests/docker/dockerfiles/fedora.docker > +++ b/tests/docker/dockerfiles/fedora.docker > @@ -38,6 +38,7 @@ ENV PACKAGES \ > libssh-devel \ > libubsan \ > libudev-devel \ > +liburing-devel \ > libusbx-devel \ > libxml2-devel \ > libzstd-devel \ > Reviewed-by: Philippe Mathieu-Daudé ../util/fdmon-io_uring.c:106:17: error: address argument to atomic operation must be a pointer to _Atomic type ('unsigned int *' invalid) old_flags = atomic_fetch_or(&node->flags, FDMON_IO_URING_PENDING | flags); ^ /usr/lib64/clang/10.0.0/include/stdatomic.h:138:42: note: expanded from macro 'atomic_fetch_or' #define atomic_fetch_or(object, operand) __c11_atomic_fetch_or(object, operand, __ATOMIC_SEQ_CST) ^ ~~ ../util/fdmon-io_uring.c:130:14: error: address argument to atomic operation must be a pointer to _Atomic type ('unsigned int *' invalid) *flags = atomic_fetch_and(&node->flags, ~(FDMON_IO_URING_PENDING | ^ Tested-by: Philippe Mathieu-Daudé
[Bug 1892533] Re: Meson: Missing config-host.mak
configure does not seems to work better: build$ ../configure ../configure: 232: shift: can't shift that many -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1892533 Title: Meson: Missing config-host.mak Status in QEMU: New Bug description: Wanted to give a try to the new build system, but a simple "meson build" gives that error: meson.build:15:0: ERROR: Failed to load /home/xclaesse/programmation/qemu/build/config-host.mak: [Errno 2] No such file or directory: '/home/xclaesse/programmation/qemu/build /config-host.mak' To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1892533/+subscriptions
Re: [External] Re: [PATCH] virtiofsd: add -o allow_directio|no_directio option
On Fri, Aug 21, 2020 at 7:58 PM Daniel P. Berrangé wrote: > On Fri, Aug 21, 2020 at 11:41:26AM +0800, Jiachen Zhang wrote: > > Due to the commit 65da4539803373ec4eec97ffc49ee90083e56efd, the O_DIRECT > > open flag of guest applications will be discarded by virtiofsd. While > > this behavior makes it consistent with the virtio-9p scheme when guest > > applications using direct I/O, we no longer have any chance to bypass > > the host page cache. > > > > Therefore, we add a flag 'allow_directio' to lo_data. If '-o no_directio' > > option is added, or none of '-o no_directio' or '-o allow_directio' is > > added, the 'allow_directio' will be set to 0, and virtiofsd discards > > O_DIRECT as before. If '-o allow_directio' is added to the stariting > > command-line, 'allow_directio' will be set to 1, so that the O_DIRECT > > flags will be retained and host page cache can be bypassed. > > > > Signed-off-by: Jiachen Zhang > > --- > > tools/virtiofsd/helper.c | 4 > > tools/virtiofsd/passthrough_ll.c | 20 ++-- > > 2 files changed, 18 insertions(+), 6 deletions(-) > > > > diff --git a/tools/virtiofsd/helper.c b/tools/virtiofsd/helper.c > > index 3105b6c23a..534ff52c64 100644 > > --- a/tools/virtiofsd/helper.c > > +++ b/tools/virtiofsd/helper.c > > @@ -180,6 +180,10 @@ void fuse_cmdline_help(void) > > " (0 leaves rlimit > unchanged)\n" > > " default: min(100, > fs.file-max - 16384)\n" > > "if the current > rlimit is lower\n" > > + "-o allow_directio|no_directio\n" > > + " retain/discard O_DIRECT > flags passed down\n" > > + " to virtiofsd from guest > applications.\n" > > + " default: no_directio\n" > > ); > > The standard naming convention from existing options is to use > $OPTNAME and no_$OPTNAME. > > IOW, don't use the "allow_" prefix. The options should be just > "directio" and "no_directio" > > Thanks, Daniel. I did consider using "directio" instead of "allow_directio" before I send out this patch. Although "-o directio" makes it consistent with other option names, it may confuse the users of virtiofsd. Because currently, virtiofsd will not add an O_DIRECT to the open flag, it will just retain or discard the O_DIRECT added by guest applications. But "-o direct" may make the users think that virtiofsd will do direct IO all the time. Jiachen > > Regards, > Daniel > -- > |: https://berrange.com -o- > https://www.flickr.com/photos/dberrange :| > |: https://libvirt.org -o- > https://fstop138.berrange.com :| > |: https://entangle-photo.org-o- > https://www.instagram.com/dberrange :| >
Re: Suspicious QOM types without instance/class size
On Fri, Aug 21, 2020 at 01:29:38PM -0400, Eduardo Habkost wrote: > On Fri, Aug 21, 2020 at 01:53:52PM +0300, Roman Bolshakov wrote: > > On Thu, Aug 20, 2020 at 05:55:29PM -0400, Eduardo Habkost wrote: > > > While trying to convert TypeInfo declarations to the new > > > OBJECT_DECLARE* macros, I've stumbled on a few suspicious cases > > > where instance_size or class_size is not set, despite having type > > > checker macros that use a specific type. > > > > > > The ones with "WARNING" are abstract types (maybe not serious if > > > subclasses set the appropriate sizes). The ones with "ERROR" > > > don't seem to be abstract types. > > > > > > > > ERROR: target/i386/hvf/hvf.c:908:1: instance_size should be set to > > > sizeof(HVFState)? > > > > Hi Eduardo, > > > > How do you get the error? > > My script looks for corresponding type checking macros, and check > if instance_size is set to sizeof(T) with the right type from the > type checking macro. > > The code is here: > https://github.com/ehabkost/qemu-hacks/blob/920b2c521ad2a29fa663256854e24ed2059ba9cd/scripts/codeconverter/codeconverter/qom_type_info.py#L136 > > > > > > Given your changes, instance size should really be sizeof(HVFState). > > > > The changes I've made shouldn't make any difference (if there's > an issue, it is there before or after my series). > > > BTW, the object definition for hvf seems different from KVM (and perhaps > > wrong?), e.g. HVFState is allocated within init_machine handler and then > > assigned to a global variable: > > Interesting. It looks like hvf_state is _not_ the actual QOM > object instance. The actual TYPE_HVF_ACCEL instance is created > by do_configure_accelerator(). That would explain why the lack > of instance_init never caused any problems. > > Luckily, no code ever used the HVF_STATE macro. If > HVF_STATE(hvf_state) got called, it would crash because of > uninitialized object instance data. If HVF_STATE(machine->accel) > got called, it would return an invalid HVFState pointer (not > hvf_state). > > I believe the simplest short term solution here is to just delete > the HVF_STATE macro and HVFState::parent field. We can worry > about actually moving hvf_state to the machine->accel QOM object > later. Actually, it might be easier to do the full QOM conversion in a single patch instead of deleting the incomplete code. Can you check if the following patch works? I don't have a host where I can test it. Signed-off-by: Eduardo Habkost --- diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c index d81f569aed..81d1662d06 100644 --- a/target/i386/hvf/hvf.c +++ b/target/i386/hvf/hvf.c @@ -878,13 +878,11 @@ static int hvf_accel_init(MachineState *ms) { int x; hv_return_t ret; -HVFState *s; +HVFState *s = HVF_STATE(ms->accelerator); ret = hv_vm_create(HV_VM_DEFAULT); assert_hvf_ok(ret); -s = g_new0(HVFState, 1); - s->num_slots = 32; for (x = 0; x < s->num_slots; ++x) { s->slots[x].size = 0; @@ -908,6 +906,7 @@ static void hvf_accel_class_init(ObjectClass *oc, void *data) static const TypeInfo hvf_accel_type = { .name = TYPE_HVF_ACCEL, .parent = TYPE_ACCEL, +.instance_size = sizeof(HVFState), .class_init = hvf_accel_class_init, }; -- Eduardo
[PULL 23/23] hw/sd: Correct the maximum size of a Standard Capacity SD Memory Card
From: Bin Meng Per the SD spec, Standard Capacity SD Memory Card (SDSC) supports capacity up to and including 2 GiB. Fixes: 2d7adea4fe ("hw/sd: Support SDHC size cards") Signed-off-by: Bin Meng Reviewed-by: Philippe Mathieu-Daudé Tested-by: Sai Pavan Boddu Message-Id: <1598021136-49525-2-git-send-email-bmeng...@gmail.com> Signed-off-by: Philippe Mathieu-Daudé --- hw/sd/sd.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/hw/sd/sd.c b/hw/sd/sd.c index 805e21fc883..483c4f17204 100644 --- a/hw/sd/sd.c +++ b/hw/sd/sd.c @@ -51,6 +51,8 @@ //#define DEBUG_SD 1 +#define SDSC_MAX_CAPACITY (2 * GiB) + typedef enum { sd_r0 = 0,/* no response */ sd_r1,/* normal response command */ @@ -314,7 +316,7 @@ static void sd_ocr_powerup(void *opaque) /* card power-up OK */ sd->ocr = FIELD_DP32(sd->ocr, OCR, CARD_POWER_UP, 1); -if (sd->size > 1 * GiB) { +if (sd->size > SDSC_MAX_CAPACITY) { sd->ocr = FIELD_DP32(sd->ocr, OCR, CARD_CAPACITY, 1); } } @@ -386,7 +388,7 @@ static void sd_set_csd(SDState *sd, uint64_t size) uint32_t sectsize = (1 << (SECTOR_SHIFT + 1)) - 1; uint32_t wpsize = (1 << (WPGROUP_SHIFT + 1)) - 1; -if (size <= 1 * GiB) { /* Standard Capacity SD */ +if (size <= SDSC_MAX_CAPACITY) { /* Standard Capacity SD */ sd->csd[0] = 0x00; /* CSD structure */ sd->csd[1] = 0x26; /* Data read access-time-1 */ sd->csd[2] = 0x00; /* Data read access-time-2 */ -- 2.26.2