Re: [PATCH] hw/display/virtio-vga: made vga memory size configurable
On Mon, Mar 15, 2021 at 4:24 PM Gerd Hoffmann wrote: > > On Mon, Mar 15, 2021 at 12:29:16PM +0100, Vitaly Chipounov wrote: > > On Mon, Mar 15, 2021 at 8:21 AM Gerd Hoffmann wrote: > > > > > > If your guest has no virtio driver use stdvga instead of running > > > virtio-vga permanently in vga compatibility mode. > > > > I tried -device VGA,vgamem_mb=32. I did not see any resolution above > > 1080p on a Windows 10 guest. > > Try "-device VGA,vgamem_mb=32,edid=off". Windows seems to not like our > edid block for some reason. > This worked, thanks for the tip. Regarding the patch, I can resubmit it with an amended commit message if people think it's still better to have a configurable memory size here. Vitaly
Re: [PATCH] hw/display/virtio-vga: made vga memory size configurable
Yes, it's a typo in the commit message, sorry.
Vitaly
On 3/14/21 1:45 PM, BALATON Zoltan wrote:
On Sun, 14 Mar 2021, vit...@cyberhaven.com wrote:
From: Vitaly Chipounov
This enables higher resolutions. The default is still 8MB for
backwards compatibility with existing snapshots.
The property name "vgamem_fb" is similar to that of the other
Isn't that vgamem_mb? Code has that so it's just a typo in commit
message.
Regards,
BALATON Zoltan
graphic adapters.
seabios/vgasrc/svgamodes.c needs to be updated as well.
For example, adding the following line would expose
a 3840x2160 resolution to the guest.
{ 0x199, { MM_DIRECT, 3840, 2160, 32, 8, 16, SEG_GRAPH } },
Signed-off-by: Vitaly Chipounov
---
hw/display/virtio-vga.c | 3 ++-
hw/virtio/virtio-pci.h | 2 ++
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/hw/display/virtio-vga.c b/hw/display/virtio-vga.c
index d3c6404061..657fafc48f 100644
--- a/hw/display/virtio-vga.c
+++ b/hw/display/virtio-vga.c
@@ -118,7 +118,7 @@ static void
virtio_vga_base_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
int i;
/* init vga compat bits */
- vga->vram_size_mb = 8;
+ vga->vram_size_mb = vpci_dev->vgamem_size_mb;
vga_common_init(vga, OBJECT(vpci_dev));
vga_init(vga, OBJECT(vpci_dev),
pci_address_space(&vpci_dev->pci_dev),
pci_address_space_io(&vpci_dev->pci_dev), true);
@@ -204,6 +204,7 @@ static void virtio_vga_set_big_endian_fb(Object
*obj, bool value, Error **errp)
static Property virtio_vga_base_properties[] = {
DEFINE_VIRTIO_GPU_PCI_PROPERTIES(VirtIOPCIProxy),
+ DEFINE_PROP_UINT32("vgamem_mb", VirtIOPCIProxy, vgamem_size_mb, 8),
DEFINE_PROP_END_OF_LIST(),
};
diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h
index d7d5d403a9..8684311a8d 100644
--- a/hw/virtio/virtio-pci.h
+++ b/hw/virtio/virtio-pci.h
@@ -151,6 +151,8 @@ struct VirtIOPCIProxy {
VirtIOIRQFD *vector_irqfd;
int nvqs_with_notifiers;
VirtioBusState bus;
+
+ uint32_t vgamem_size_mb;
};
static inline bool virtio_pci_modern(VirtIOPCIProxy *proxy)
Re: [PATCH] hw/display/virtio-vga: made vga memory size configurable
On Mon, Mar 15, 2021 at 8:21 AM Gerd Hoffmann wrote: > > On Sun, Mar 14, 2021 at 01:23:14PM +0100, vit...@cyberhaven.com wrote: > > From: Vitaly Chipounov > > > > This enables higher resolutions. > > No. virtio-vga supports higher resolutions just fine once the guest > driver is loaded. The video memory is used at boot only, before the > guest driver is loaded, and 8MB just for a boot display is more than > generous. > > If your guest has no virtio driver use stdvga instead of running > virtio-vga permanently in vga compatibility mode. I tried -device VGA,vgamem_mb=32. I did not see any resolution above 1080p on a Windows 10 guest. virtio-vga has many more resolutions available, it was just missing 4k. I have the virtio-win-0.1.190 driver pack installed. I don't use qxl, because it makes the Windows GUI sluggish for me. I don't have problems with VGA or virtio-vga. Best regards, Vitaly
[Qemu-devel] [PATCH] x86: Fixed incorrect segment base address addition
An instruction with address and segment size override triggers the bug.
inc dword ptr gs:260h[ebx*4] gets incorrectly translated to:
(uint32_t)(gs.base + ebx * 4 + 0x260)
instead of
gs.base + (uint32_t)(ebx * 4 + 0x260)
Signed-off-by: Vitaly Chipounov
---
target-i386/translate.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/target-i386/translate.c b/target-i386/translate.c
index a902f4a..9ca7375 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -459,10 +459,10 @@ static inline void gen_op_movl_A0_seg(int reg)
static inline void gen_op_addl_A0_seg(int reg)
{
tcg_gen_ld_tl(cpu_tmp0, cpu_env, offsetof(CPUX86State, segs[reg].base));
-tcg_gen_add_tl(cpu_A0, cpu_A0, cpu_tmp0);
#ifdef TARGET_X86_64
tcg_gen_andi_tl(cpu_A0, cpu_A0, 0x);
#endif
+tcg_gen_add_tl(cpu_A0, cpu_A0, cpu_tmp0);
}
#ifdef TARGET_X86_64
--
1.7.4.1
Re: [Qemu-devel] [PATCH] x86: Fixed incorrect segment base address addition
Max,
On 02.07.2012 17:18, Max Filippov wrote:
> On Mon, Jul 2, 2012 at 2:29 PM, Vitaly Chipounov
> wrote:
>> An instruction with address and segment size override triggers the bug.
>> inc dword ptr gs:260h[ebx*4] gets incorrectly translated to:
>> (uint32_t)(gs.base + ebx * 4 + 0x260)
>> instead of
>> gs.base + (uint32_t)(ebx * 4 + 0x260)
> Do I understand it right that this fixes address calculation for
> 64-bit mode but breaks it for compatibility mode?
You are right, it indeed breaks compatibility mode. Thanks for the
reference from the Intel manual.
I will send an updated patch.
Vitaly
>> Signed-off-by: Vitaly Chipounov
>> ---
>> target-i386/translate.c |2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/target-i386/translate.c b/target-i386/translate.c
>> index a902f4a..9ca7375 100644
>> --- a/target-i386/translate.c
>> +++ b/target-i386/translate.c
>> @@ -459,10 +459,10 @@ static inline void gen_op_movl_A0_seg(int reg)
>> static inline void gen_op_addl_A0_seg(int reg)
>> {
>> tcg_gen_ld_tl(cpu_tmp0, cpu_env, offsetof(CPUX86State, segs[reg].base));
>> -tcg_gen_add_tl(cpu_A0, cpu_A0, cpu_tmp0);
>> #ifdef TARGET_X86_64
>> tcg_gen_andi_tl(cpu_A0, cpu_A0, 0x);
>> #endif
>> +tcg_gen_add_tl(cpu_A0, cpu_A0, cpu_tmp0);
>> }
>>
>> #ifdef TARGET_X86_64
>> --
>> 1.7.4.1
>>
>>
>
>
[Qemu-devel] [PATCH v2] x86: Fixed incorrect segment base address addition in 64-bits mode
According to the Intel manual
"Intel® 64 and IA-32 Architectures Software Developer’s Manual
Volume 3", "3.4.4 Segment Loading Instructions in IA-32e Mode":
"When in compatibility mode, FS and GS overrides operate as defined by
32-bit mode behavior regardless of the value loaded into the upper 32
linear-address bits of the hidden descriptor register base field.
Compatibility mode ignores the upper 32 bits when calculating an effective
address."
However, the code misses the 64-bit mode case, where an instruction with
address and segment size override would be translated incorrectly. For example,
inc dword ptr gs:260h[ebx*4] gets incorrectly translated to:
(uint32_t)(gs.base + ebx * 4 + 0x260)
instead of
gs.base + (uint32_t)(ebx * 4 + 0x260)
Signed-off-by: Vitaly Chipounov
---
target-i386/translate.c | 43 +--
1 files changed, 25 insertions(+), 18 deletions(-)
diff --git a/target-i386/translate.c b/target-i386/translate.c
index a902f4a..dfec735 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -456,12 +456,19 @@ static inline void gen_op_movl_A0_seg(int reg)
tcg_gen_ld32u_tl(cpu_A0, cpu_env, offsetof(CPUX86State, segs[reg].base) +
REG_L_OFFSET);
}
-static inline void gen_op_addl_A0_seg(int reg)
+static inline void gen_op_addl_A0_seg(DisasContext *s, int reg)
{
tcg_gen_ld_tl(cpu_tmp0, cpu_env, offsetof(CPUX86State, segs[reg].base));
-tcg_gen_add_tl(cpu_A0, cpu_A0, cpu_tmp0);
#ifdef TARGET_X86_64
-tcg_gen_andi_tl(cpu_A0, cpu_A0, 0x);
+if (CODE64(s)) {
+tcg_gen_andi_tl(cpu_A0, cpu_A0, 0x);
+tcg_gen_add_tl(cpu_A0, cpu_A0, cpu_tmp0);
+} else {
+tcg_gen_add_tl(cpu_A0, cpu_A0, cpu_tmp0);
+tcg_gen_andi_tl(cpu_A0, cpu_A0, 0x);
+}
+#else
+tcg_gen_add_tl(cpu_A0, cpu_A0, cpu_tmp0);
#endif
}
@@ -617,7 +624,7 @@ static inline void gen_string_movl_A0_ESI(DisasContext *s)
override = R_DS;
gen_op_movl_A0_reg(R_ESI);
gen_op_andl_A0_();
-gen_op_addl_A0_seg(override);
+gen_op_addl_A0_seg(s, override);
}
}
@@ -638,7 +645,7 @@ static inline void gen_string_movl_A0_EDI(DisasContext *s)
} else {
gen_op_movl_A0_reg(R_EDI);
gen_op_andl_A0_();
-gen_op_addl_A0_seg(R_ES);
+gen_op_addl_A0_seg(s, R_ES);
}
}
@@ -2063,7 +2070,7 @@ static void gen_lea_modrm(DisasContext *s, int modrm, int
*reg_ptr, int *offset_
} else
#endif
{
-gen_op_addl_A0_seg(override);
+gen_op_addl_A0_seg(s, override);
}
}
} else {
@@ -2130,7 +2137,7 @@ static void gen_lea_modrm(DisasContext *s, int modrm, int
*reg_ptr, int *offset_
else
override = R_DS;
}
-gen_op_addl_A0_seg(override);
+gen_op_addl_A0_seg(s, override);
}
}
@@ -2207,7 +2214,7 @@ static void gen_add_A0_ds_seg(DisasContext *s)
} else
#endif
{
-gen_op_addl_A0_seg(override);
+gen_op_addl_A0_seg(s, override);
}
}
}
@@ -2460,12 +2467,12 @@ static void gen_push_T0(DisasContext *s)
if (s->ss32) {
if (s->addseg) {
tcg_gen_mov_tl(cpu_T[1], cpu_A0);
-gen_op_addl_A0_seg(R_SS);
+gen_op_addl_A0_seg(s, R_SS);
}
} else {
gen_op_andl_A0_();
tcg_gen_mov_tl(cpu_T[1], cpu_A0);
-gen_op_addl_A0_seg(R_SS);
+gen_op_addl_A0_seg(s, R_SS);
}
gen_op_st_T0_A0(s->dflag + 1 + s->mem_index);
if (s->ss32 && !s->addseg)
@@ -2500,11 +2507,11 @@ static void gen_push_T1(DisasContext *s)
gen_op_addl_A0_im(-4);
if (s->ss32) {
if (s->addseg) {
-gen_op_addl_A0_seg(R_SS);
+gen_op_addl_A0_seg(s, R_SS);
}
} else {
gen_op_andl_A0_();
-gen_op_addl_A0_seg(R_SS);
+gen_op_addl_A0_seg(s, R_SS);
}
gen_op_st_T1_A0(s->dflag + 1 + s->mem_index);
@@ -2528,10 +2535,10 @@ static void gen_pop_T0(DisasContext *s)
gen_op_movl_A0_reg(R_ESP);
if (s->ss32) {
if (s->addseg)
-gen_op_addl_A0_seg(R_SS);
+gen_op_addl_A0_seg(s, R_SS);
} else {
gen_op_andl_A0_();
-gen_op_addl_A0_seg(R_SS);
+gen_op_addl_A0_seg(s, R_SS);
}
gen_op_ld_T0_A0(s->dflag + 1 + s->mem_index);
}
@@ -2556,7 +2563,7 @@ static void gen_stack_A0(DisasContext *s)
gen_op_andl_A0_();
tcg_gen_mov_tl(cpu_T[1], cpu_A0);
if (s->addseg)
-gen_op_addl_A0_seg(R_SS);
+gen_op_addl_A0_seg(s, R_SS);
}
/* NOTE: wrap around in 16 b
[Qemu-devel] [PATCH] slirp: fixed potential use-after-free of a socket
A socket may still have references to it in various queues
at the time it is freed, causing memory corruptions.
Signed-off-by: Vitaly Chipounov
---
slirp/socket.c | 29 +
1 file changed, 29 insertions(+)
diff --git a/slirp/socket.c b/slirp/socket.c
index 77b0c98..8a7adc8 100644
--- a/slirp/socket.c
+++ b/slirp/socket.c
@@ -55,6 +55,33 @@ socreate(Slirp *slirp)
return(so);
}
+/**
+ * It may happen that a socket is still referenced in various
+ * mbufs at the time it is freed. Clear all references to the
+ * socket here.
+ */
+static void soremovefromqueues(struct socket *so)
+{
+if (!so->so_queued) {
+return;
+}
+
+Slirp *slirp = so->slirp;
+struct mbuf *ifm;
+
+for (ifm = slirp->if_fastq.ifq_next; ifm != &slirp->if_fastq; ifm =
ifm->ifq_next) {
+if (ifm->ifq_so == so) {
+ifm->ifq_so = NULL;
+}
+}
+
+for (ifm = slirp->if_batchq.ifq_next; ifm != &slirp->if_batchq; ifm =
ifm->ifq_next) {
+if (ifm->ifq_so == so) {
+ifm->ifq_so = NULL;
+}
+}
+}
+
/*
* remque and free a socket, clobber cache
*/
@@ -79,6 +106,8 @@ sofree(struct socket *so)
if(so->so_next && so->so_prev)
remque(so); /* crashes if so is not in a queue */
+ soremovefromqueues(so);
+
free(so);
}
--
1.7.10
Re: [Qemu-devel] [PATCH] slirp: fixed potential use-after-free of a socket
Hi,
On 21.02.2013 15:33, Jan Kiszka wrote:
> On 2013-02-15 12:00, Vitaly Chipounov wrote:
>> A socket may still have references to it in various queues
>> at the time it is freed, causing memory corruptions.
> Did you see it in practice? Or is this patch based on code review? What
> will happen if those queued mbufs find their ifq_so NULL?
I have a packet trace that triggers this problem when it is injected
into the guest's NIC.
I am still not quite sure why this happens, but suspect that it could be
caused by malformed/partial TCP/IP streams.
Setting ifq_so to NULL shouldn't be an issue because there seems to be
checks for that (e.g., in if.c).
It doesn't seem to affect normal web browsing/downloading in the guest.
Vitaly
>
>> Signed-off-by: Vitaly Chipounov
>> ---
>> slirp/socket.c | 29 +
>> 1 file changed, 29 insertions(+)
>>
>> diff --git a/slirp/socket.c b/slirp/socket.c
>> index 77b0c98..8a7adc8 100644
>> --- a/slirp/socket.c
>> +++ b/slirp/socket.c
>> @@ -55,6 +55,33 @@ socreate(Slirp *slirp)
>>return(so);
>> }
>>
>> +/**
>> + * It may happen that a socket is still referenced in various
>> + * mbufs at the time it is freed. Clear all references to the
>> + * socket here.
>> + */
>> +static void soremovefromqueues(struct socket *so)
>> +{
>> +if (!so->so_queued) {
>> +return;
>> +}
>> +
>> +Slirp *slirp = so->slirp;
>> +struct mbuf *ifm;
>> +
>> +for (ifm = slirp->if_fastq.ifq_next; ifm != &slirp->if_fastq; ifm =
>> ifm->ifq_next) {
> This line and the one below smells like checkpatch.pl wasn't run against
> it. Granted, slirp patches easy make checkpatch upset as the input is
> not properly formatted, but please don't add more violations.
>
>> +if (ifm->ifq_so == so) {
>> +ifm->ifq_so = NULL;
>> +}
>> +}
>> +
>> +for (ifm = slirp->if_batchq.ifq_next; ifm != &slirp->if_batchq; ifm =
>> ifm->ifq_next) {
>> +if (ifm->ifq_so == so) {
>> +ifm->ifq_so = NULL;
>> +}
>> +}
>> +}
>> +
>> /*
>> * remque and free a socket, clobber cache
>> */
>> @@ -79,6 +106,8 @@ sofree(struct socket *so)
>>if(so->so_next && so->so_prev)
>> remque(so); /* crashes if so is not in a queue */
>>
>> + soremovefromqueues(so);
>> +
>>free(so);
>> }
>>
>>
> Sorry for the late feedback,
> Jan
>
