* [PULL 1/3] amd_iommu: Fix kvm_enable_x2apic link error with clang in non-KVM builds
2024-11-28 19:03 [PULL 0/3] Bugfixes for QEMU 9.2 rc3 Paolo Bonzini
@ 2024-11-28 19:03 ` Paolo Bonzini
2024-11-28 19:03 ` [PULL 2/3] hvf: complete 1G page support Paolo Bonzini
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Paolo Bonzini @ 2024-11-28 19:03 UTC (permalink / raw)
To: qemu-devel; +Cc: Sairaj Kodilkar, Santosh Shukla, Phil Dennis-Jordan
From: Sairaj Kodilkar <sarunkod@amd.com>
Commit b12cb3819 (amd_iommu: Check APIC ID > 255 for XTSup) throws
linking error for the `kvm_enable_x2apic` when kvm is disabled
and Clang is used for compilation.
This issue comes up because Clang does not remove the function callsite
(kvm_enable_x2apic in this case) during optimization when if condition
have variable. Intel IOMMU driver solves this issue by creating separate
if condition for checking variables, which causes call site being
optimized away by virtue of `kvm_irqchip_is_split()` being defined as 0.
Implement same solution for the AMD driver.
Fixes: b12cb3819baf (amd_iommu: Check APIC ID > 255 for XTSup)
Signed-off-by: Sairaj Kodilkar <sarunkod@amd.com>
Signed-off-by: Santosh Shukla <santosh.shukla@amd.com>
Tested-by: Phil Dennis-Jordan <phil@philjordan.eu>
Link: https://lore.kernel.org/r/20241114114509.15350-1-sarunkod@amd.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/i386/amd_iommu.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index 13af7211e11..af0f4da1f69 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -1657,9 +1657,11 @@ static void amdvi_sysbus_realize(DeviceState *dev, Error **errp)
error_report("AMD IOMMU with x2APIC confguration requires xtsup=on");
exit(EXIT_FAILURE);
}
- if (s->xtsup && kvm_irqchip_is_split() && !kvm_enable_x2apic()) {
- error_report("AMD IOMMU xtsup=on requires support on the KVM side");
- exit(EXIT_FAILURE);
+ if (s->xtsup) {
+ if (kvm_irqchip_is_split() && !kvm_enable_x2apic()) {
+ error_report("AMD IOMMU xtsup=on requires support on the KVM side");
+ exit(EXIT_FAILURE);
+ }
}
pci_setup_iommu(bus, &amdvi_iommu_ops, s);
--
2.47.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PULL 2/3] hvf: complete 1G page support
2024-11-28 19:03 [PULL 0/3] Bugfixes for QEMU 9.2 rc3 Paolo Bonzini
2024-11-28 19:03 ` [PULL 1/3] amd_iommu: Fix kvm_enable_x2apic link error with clang in non-KVM builds Paolo Bonzini
@ 2024-11-28 19:03 ` Paolo Bonzini
2024-11-28 19:03 ` [PULL 3/3] scsi: megasas: Internal cdbs have 16-byte length Paolo Bonzini
2024-11-29 15:45 ` [PULL 0/3] Bugfixes for QEMU 9.2 rc3 Peter Maydell
3 siblings, 0 replies; 5+ messages in thread
From: Paolo Bonzini @ 2024-11-28 19:03 UTC (permalink / raw)
To: qemu-devel
Cc: Alexander Graf, Akihiro Suda, Philippe Mathieu-Daudé,
Phil Dennis-Jordan
From: Alexander Graf <agraf@csgraf.de>
Hvf on x86 only supported 2MiB large pages, but never bothered to strip
out the 1GiB page size capability from -cpu host. With QEMU 8.0.0 this
became a problem because OVMF started to use 1GiB pages by default.
Let's just unconditionally add 1GiB page walk support to the walker.
With this fix applied, I can successfully run OVMF again.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1603
Signed-off-by: Alexander Graf <agraf@csgraf.de>
Reported-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
Reported-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Phil Dennis-Jordan <phil@philjordan.eu>
Link: https://lore.kernel.org/r/20230420225258.58009-1-agraf@csgraf.de
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/i386/hvf/x86_mmu.c | 30 ++++++++++++++++++++----------
1 file changed, 20 insertions(+), 10 deletions(-)
diff --git a/target/i386/hvf/x86_mmu.c b/target/i386/hvf/x86_mmu.c
index 649074a7d24..579d0c3a4cc 100644
--- a/target/i386/hvf/x86_mmu.c
+++ b/target/i386/hvf/x86_mmu.c
@@ -38,6 +38,7 @@
#define LEGACY_PTE_PAGE_MASK (0xffffffffllu << 12)
#define PAE_PTE_PAGE_MASK ((-1llu << 12) & ((1llu << 52) - 1))
#define PAE_PTE_LARGE_PAGE_MASK ((-1llu << (21)) & ((1llu << 52) - 1))
+#define PAE_PTE_SUPER_PAGE_MASK ((-1llu << (30)) & ((1llu << 52) - 1))
struct gpt_translation {
target_ulong gva;
@@ -96,7 +97,7 @@ static bool get_pt_entry(CPUState *cpu, struct gpt_translation *pt,
/* test page table entry */
static bool test_pt_entry(CPUState *cpu, struct gpt_translation *pt,
- int level, bool *is_large, bool pae)
+ int level, int *largeness, bool pae)
{
uint64_t pte = pt->pte[level];
@@ -118,9 +119,9 @@ static bool test_pt_entry(CPUState *cpu, struct gpt_translation *pt,
goto exit;
}
- if (1 == level && pte_large_page(pte)) {
+ if (level && pte_large_page(pte)) {
pt->err_code |= MMU_PAGE_PT;
- *is_large = true;
+ *largeness = level;
}
if (!level) {
pt->err_code |= MMU_PAGE_PT;
@@ -152,9 +153,18 @@ static inline uint64_t pse_pte_to_page(uint64_t pte)
return ((pte & 0x1fe000) << 19) | (pte & 0xffc00000);
}
-static inline uint64_t large_page_gpa(struct gpt_translation *pt, bool pae)
+static inline uint64_t large_page_gpa(struct gpt_translation *pt, bool pae,
+ int largeness)
{
- VM_PANIC_ON(!pte_large_page(pt->pte[1]))
+ VM_PANIC_ON(!pte_large_page(pt->pte[largeness]))
+
+ /* 1Gib large page */
+ if (pae && largeness == 2) {
+ return (pt->pte[2] & PAE_PTE_SUPER_PAGE_MASK) | (pt->gva & 0x3fffffff);
+ }
+
+ VM_PANIC_ON(largeness != 1)
+
/* 2Mb large page */
if (pae) {
return (pt->pte[1] & PAE_PTE_LARGE_PAGE_MASK) | (pt->gva & 0x1fffff);
@@ -170,7 +180,7 @@ static bool walk_gpt(CPUState *cpu, target_ulong addr, int err_code,
struct gpt_translation *pt, bool pae)
{
int top_level, level;
- bool is_large = false;
+ int largeness = 0;
target_ulong cr3 = rvmcs(cpu->accel->fd, VMCS_GUEST_CR3);
uint64_t page_mask = pae ? PAE_PTE_PAGE_MASK : LEGACY_PTE_PAGE_MASK;
@@ -186,19 +196,19 @@ static bool walk_gpt(CPUState *cpu, target_ulong addr, int err_code,
for (level = top_level; level > 0; level--) {
get_pt_entry(cpu, pt, level, pae);
- if (!test_pt_entry(cpu, pt, level - 1, &is_large, pae)) {
+ if (!test_pt_entry(cpu, pt, level - 1, &largeness, pae)) {
return false;
}
- if (is_large) {
+ if (largeness) {
break;
}
}
- if (!is_large) {
+ if (!largeness) {
pt->gpa = (pt->pte[0] & page_mask) | (pt->gva & 0xfff);
} else {
- pt->gpa = large_page_gpa(pt, pae);
+ pt->gpa = large_page_gpa(pt, pae, largeness);
}
return true;
--
2.47.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PULL 3/3] scsi: megasas: Internal cdbs have 16-byte length
2024-11-28 19:03 [PULL 0/3] Bugfixes for QEMU 9.2 rc3 Paolo Bonzini
2024-11-28 19:03 ` [PULL 1/3] amd_iommu: Fix kvm_enable_x2apic link error with clang in non-KVM builds Paolo Bonzini
2024-11-28 19:03 ` [PULL 2/3] hvf: complete 1G page support Paolo Bonzini
@ 2024-11-28 19:03 ` Paolo Bonzini
2024-11-29 15:45 ` [PULL 0/3] Bugfixes for QEMU 9.2 rc3 Peter Maydell
3 siblings, 0 replies; 5+ messages in thread
From: Paolo Bonzini @ 2024-11-28 19:03 UTC (permalink / raw)
To: qemu-devel
Cc: Guenter Roeck, Fabiano Rosas, Philippe Mathieu-Daudé,
Fiona Ebner, qemu-stable
From: Guenter Roeck <linux@roeck-us.net>
Host drivers do not necessarily set cdb_len in megasas io commands.
With commits 6d1511cea0 ("scsi: Reject commands if the CDB length
exceeds buf_len") and fe9d8927e2 ("scsi: Add buf_len parameter to
scsi_req_new()"), this results in failures to boot Linux from affected
SCSI drives because cdb_len is set to 0 by the host driver.
Set the cdb length to its actual size to solve the problem.
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Tested-by: Fiona Ebner <f.ebner@proxmox.com>
Link: https://lore.kernel.org/r/20230228171129.4094709-1-linux@roeck-us.net
Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/scsi/megasas.c | 14 ++------------
1 file changed, 2 insertions(+), 12 deletions(-)
diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index 221b06d6aaf..b33229d71a4 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -1781,7 +1781,7 @@ static int megasas_handle_io(MegasasState *s, MegasasCmd *cmd, int frame_cmd)
uint8_t cdb[16];
int len;
struct SCSIDevice *sdev = NULL;
- int target_id, lun_id, cdb_len;
+ int target_id, lun_id;
lba_count = le32_to_cpu(cmd->frame->io.header.data_len);
lba_start_lo = le32_to_cpu(cmd->frame->io.lba_lo);
@@ -1790,7 +1790,6 @@ static int megasas_handle_io(MegasasState *s, MegasasCmd *cmd, int frame_cmd)
target_id = cmd->frame->header.target_id;
lun_id = cmd->frame->header.lun_id;
- cdb_len = cmd->frame->header.cdb_len;
if (target_id < MFI_MAX_LD && lun_id == 0) {
sdev = scsi_device_find(&s->bus, 0, target_id, lun_id);
@@ -1805,15 +1804,6 @@ static int megasas_handle_io(MegasasState *s, MegasasCmd *cmd, int frame_cmd)
return MFI_STAT_DEVICE_NOT_FOUND;
}
- if (cdb_len > 16) {
- trace_megasas_scsi_invalid_cdb_len(
- mfi_frame_desc(frame_cmd), 1, target_id, lun_id, cdb_len);
- megasas_write_sense(cmd, SENSE_CODE(INVALID_OPCODE));
- cmd->frame->header.scsi_status = CHECK_CONDITION;
- s->event_count++;
- return MFI_STAT_SCSI_DONE_WITH_ERROR;
- }
-
cmd->iov_size = lba_count * sdev->blocksize;
if (megasas_map_sgl(s, cmd, &cmd->frame->io.sgl)) {
megasas_write_sense(cmd, SENSE_CODE(TARGET_FAILURE));
@@ -1824,7 +1814,7 @@ static int megasas_handle_io(MegasasState *s, MegasasCmd *cmd, int frame_cmd)
megasas_encode_lba(cdb, lba_start, lba_count, is_write);
cmd->req = scsi_req_new(sdev, cmd->index,
- lun_id, cdb, cdb_len, cmd);
+ lun_id, cdb, sizeof(cdb), cmd);
if (!cmd->req) {
trace_megasas_scsi_req_alloc_failed(
mfi_frame_desc(frame_cmd), target_id, lun_id);
--
2.47.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PULL 0/3] Bugfixes for QEMU 9.2 rc3
2024-11-28 19:03 [PULL 0/3] Bugfixes for QEMU 9.2 rc3 Paolo Bonzini
` (2 preceding siblings ...)
2024-11-28 19:03 ` [PULL 3/3] scsi: megasas: Internal cdbs have 16-byte length Paolo Bonzini
@ 2024-11-29 15:45 ` Peter Maydell
3 siblings, 0 replies; 5+ messages in thread
From: Peter Maydell @ 2024-11-29 15:45 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel
On Thu, 28 Nov 2024 at 19:05, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> The following changes since commit 24602b77f5658ae8377958c15fdef2f44affc743:
>
> Merge tag 'for_upstream' of https://git.kernel.org/pub/scm/virt/kvm/mst/qemu into staging (2024-11-28 10:50:20 +0000)
>
> are available in the Git repository at:
>
> https://gitlab.com/bonzini/qemu.git tags/for-upstream
>
> for you to fetch changes up to 3abb67323aeecf06a27191076ab50424ec21f334:
>
> scsi: megasas: Internal cdbs have 16-byte length (2024-11-28 18:02:22 +0100)
>
> ----------------------------------------------------------------
> * amd_iommu: fix clang failure on non-KVM targets
> * target/i386/hvf: fix advertised 1G page support
> * megasas: fix CDB length
>
Applied, thanks.
Please update the changelog at https://wiki.qemu.org/ChangeLog/9.2
for any user-visible changes.
-- PMM
^ permalink raw reply [flat|nested] 5+ messages in thread