Re: [Qemu-devel] [PATCH] configure: Fix bad shell expression for non-Linux hosts
On 29/07/11 22:40, Stefan Weil wrote:
> With vhost_net="" (most non-Linux hosts), configure prints an
> error message:
>
> test: 2551: =: unexpected operator
>
> Fix this and similar code by adding the missing "".
>
> Cc: Wolfgang Mauerer
> Cc: Stefan Hajnoczi
> Signed-off-by: Stefan Weil
good catch, I've tested the code only on Linux hosts.
Cheers, Wolfgang
> ---
> configure |8
> 1 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/configure b/configure
> index b582f5e..001208d 100755
> --- a/configure
> +++ b/configure
> @@ -2521,7 +2521,7 @@ fi
> # __sync_fetch_and_and requires at least -march=i486. Many toolchains
> # use i686 as default anyway, but for those that don't, an explicit
> # specification is necessary
> -if test $vhost_net = "yes" && test $cpu = "i386"; then
> +if test "$vhost_net" = "yes" && test "$cpu" = "i386"; then
>cat > $TMPC << EOF
> int sfaa(unsigned *ptr)
> {
> @@ -2700,7 +2700,7 @@ echo "nss used $smartcard_nss"
> echo "usb net redir $usb_redir"
> echo "OpenGL support$opengl"
>
> -if test $sdl_too_old = "yes"; then
> +if test "$sdl_too_old" = "yes"; then
> echo "-> Your SDL version is too old - please upgrade to have SDL support"
> fi
>
> @@ -2788,7 +2788,7 @@ fi
> if test "$static" = "yes" ; then
>echo "CONFIG_STATIC=y" >> $config_host_mak
> fi
> -if test $profiler = "yes" ; then
> +if test "$profiler" = "yes" ; then
>echo "CONFIG_PROFILER=y" >> $config_host_mak
> fi
> if test "$slirp" = "yes" ; then
> @@ -3342,7 +3342,7 @@ case "$target_arch2" in
>\( "$target_arch2" = "x86_64" -a "$cpu" = "i386" \) -o \
>\( "$target_arch2" = "i386" -a "$cpu" = "x86_64" \) \) ; then
>echo "CONFIG_KVM=y" >> $config_target_mak
> - if test $vhost_net = "yes" ; then
> + if test "$vhost_net" = "yes" ; then
> echo "CONFIG_VHOST_NET=y" >> $config_target_mak
>fi
> fi
Re: [Qemu-devel] [RFC] Introduce vm_stop_permanent()
On Thu, Jul 28, 2011 at 9:22 PM, Luiz Capitulino wrote: > On Thu, 28 Jul 2011 20:04:58 +0200 > Jan Kiszka wrote: > >> On 2011-07-28 20:00, Luiz Capitulino wrote: >> > On Thu, 28 Jul 2011 19:52:31 +0200 >> > Jan Kiszka wrote: >> > >> >> On 2011-07-28 19:48, Luiz Capitulino wrote: >> >>> On Thu, 28 Jul 2011 14:39:23 -0300 >> >>> Luiz Capitulino wrote: >> >>> >> On Thu, 28 Jul 2011 17:20:41 +0200 >> Jan Kiszka wrote: >> >> > On 2011-07-28 17:18, Luiz Capitulino wrote: >> >> On Thu, 28 Jul 2011 16:19:19 +0200 >> >> Jan Kiszka wrote: >> >> >> >>> On 2011-07-28 15:37, Avi Kivity wrote: >> On 07/28/2011 04:31 PM, Luiz Capitulino wrote: >> > On Thu, 28 Jul 2011 10:23:22 +0300 >> > Avi Kivity wrote: >> > >> >> On 07/28/2011 12:44 AM, Blue Swirl wrote: >> >> > On Wed, Jul 27, 2011 at 9:42 PM, Luiz >> > Capitulino wrote: >> >> > > This function should be used when the VM is not supposed >> >> to >> > resume >> >> > > execution (eg. by issuing 'cont' monitor command). >> >> > > >> >> > > Today, we allow the user to resume execution even when: >> >> > > >> >> > > o the guest shuts down and -no-shutdown is used >> >> > > o there's a kvm internal error >> >> > > o loading the VM state with -loadvm or "loadvm" in the >> > monitor fails >> >> > > >> >> > > I think only badness can happen from the cases above. >> >> > >> >> > I'd suppose a system_reset should bring the system back to >> > sanity and >> >> > then clear vm_permanent_stopped (where's -ly?) >> > >> > What's -ly? >> > >> >> permanent-ly. >> >> >> > except maybe for KVM >> >> > internal error if that can't be recovered. Then it would not >> >> very >> >> > permanent anymore, so the name would need adjusting. >> >> >> >> Currently, all kvm internal errors are recoverable by reset (and >> >> possibly by fiddling with memory/registers). >> > >> > Ok, but a poweroff in the guest isn't recoverable with system_reset >> > right? Or does it depend on the guest? >> >> Right, it's not recoverable if you shut the power down where the >> tractor >> beam is coupled to the main reactor. >> >>> >> >>> system_reset will bring all emulated devices back into their power-on >> >>> state - unless we have remaining bugs to fix. Actually, one may >> >>> consider >> >>> issuing this reset automatically on vm_start after "permant" vm_stop. >> >> The only permanent vm_stop we'd have is poweroff when -no-shutdown is >> used. >> >> Are you saying that system_reset should be able to recover from that >> too? >> >>> >> >>> It already does, so we don't have permanent stops. >> >> >> >> Exactly. We just have stops over inconsistent states that require a >> >> reset to continue with anything useful. >> > >> > Yes. If I got you right, you suggest that we do the reset automatically. >> > >> > I think it's better to let the user do it, because s/he might want to >> > do something else before resetting. For example, for the kvm error the >> > user might want to save the vm state. >> >> Associating the reset with a cont means requesting an explicit action >> from the user. I'm not suggesting to do the reset when the stop state is >> entered. > > I see. But automatically resetting on cont might be unexpected to the > user, even on a bad state. > > Another option would be to add a force option to cont, where the reset is > done when the state is invalid (otherwise cont will return an error). > > I still prefer to let the user do it manually though. > >> > For the poweroff case with -no-shutdown it's probably fine, but I don't >> > want to hard code special cases. It's better and easier to treat them all >> > as "require system_reset to recover". >> >> In any case, we need to tag the current state as stopped-and-invalid or >> so vs. a normal stop. That remains a valuable first step. How to deal >> with that information is the second one. I think the right way to fix this is to disable 'cont' until a system_reset is issued. 'cont' should not perform reset but print an error message about inconsistent state and suggest issuing a 'system_reset'.
Re: [Qemu-devel] [PATCH] Fix forcing multicast msgs to loopback on OpenBSD.
On Sat, Jul 30, 2011 at 2:15 AM, Brad wrote:
> Fix forcing multicast msgs to loopback on OpenBSD.
>
> e.g.
> $ sudo qemu -m 128 -no-fd-bootchk \
> -hda virtual.img -boot n -nographic \
> -net nic,vlan=0,model=rtl8139,macaddr=52:54:00:12:34:03 \
> -net user -tftp /usr/src/sys/arch/i386/compile/TEST -bootp pxeboot \
> -net nic,vlan=1,model=rtl8139,macaddr=52:54:00:23:03:01 \
> -net tap,vlan=1,script=no \
> -net nic,vlan=3,model=rtl8139,macaddr=52:54:00:23:03:03 \
> -net socket,vlan=3,mcast=230.0.0.1:10003
> setsockopt(SOL_IP, IP_MULTICAST_LOOP): Invalid argument
> qemu: -net socket,vlan=3,mcast=230.0.0.1:10003: Device 'socket' could not be
> initialized
>
>
> Signed-off-by: Brad Smith
>
> ---
> net/socket.c | 10 --
> 1 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/net/socket.c b/net/socket.c
> index 11fe5f3..4c27097 100644
> --- a/net/socket.c
> +++ b/net/socket.c
> @@ -154,6 +154,12 @@ static int net_socket_mcast_create(struct sockaddr_in
> *mcastaddr, struct in_addr
> struct ip_mreq imr;
> int fd;
> int val, ret;
> +#ifdef __OpenBSD__
> + u_char loop;
Please use plain char, nonstandard types like u_char should not be used.
> +#else
> + int loop;
> +#endif
> +
> if (!IN_MULTICAST(ntohl(mcastaddr->sin_addr.s_addr))) {
> fprintf(stderr, "qemu: error: specified mcastaddr \"%s\" (0x%08x) does
> not contain a multicast address\n",
> inet_ntoa(mcastaddr->sin_addr),
> @@ -197,9 +203,9 @@ static int net_socket_mcast_create(struct sockaddr_in
> *mcastaddr, struct in_addr
> }
>
> /* Force mcast msgs to loopback (eg. several QEMUs in same host */
> - val = 1;
> + loop = 1;
> ret=setsockopt(fd, IPPROTO_IP, IP_MULTICAST_LOOP,
> - (const char *)&val, sizeof(val));
> + (const char *)&loop, sizeof(loop));
> if (ret < 0) {
> perror("setsockopt(SOL_IP, IP_MULTICAST_LOOP)");
> goto fail;
> --
> 1.7.6
>
>
> --
> This message has been scanned for viruses and
> dangerous content by MailScanner, and is
> believed to be clean.
>
>
>
Re: [Qemu-devel] [PATCH 0/2] SPARC64: implement %fprs register dirty bits
Thanks, applied all. On Sat, Jul 23, 2011 at 5:20 AM, Tsuneo Saito wrote: > Implement %fprs.DU/DL bits. > The FPU sets %fprs.DL and %fprs.DU when values are assigned to %f0-31 > and %f32-63 respectively. > The first patch fixes problems that are found when adding the lines > for fprs and would be better to be fixed before adding the fprs thing. > > Tsuneo Saito (2): > SPARC64: fix fnor* and fnand* > SPARC64: implement %fprs dirty bits > > target-sparc/translate.c | 130 +++-- > 1 files changed, 124 insertions(+), 6 deletions(-) > > -- > 1.7.5.4 > > >
Re: [Qemu-devel] [PATCH] fix disabling interrupts in sun4u
On Mon, Jul 25, 2011 at 8:22 PM, Artyom Tarasenko wrote:
> clear interrupt request if the interrupt priority < CPU pil
> clear hardware interrupt request if interrupts are disabled
>
> Signed-off-by: Artyom Tarasenko
> ---
> hw/sun4u.c | 6 --
> 1 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/hw/sun4u.c b/hw/sun4u.c
> index d7dcaf0..7f95aeb 100644
> --- a/hw/sun4u.c
> +++ b/hw/sun4u.c
> @@ -255,7 +255,7 @@ void cpu_check_irqs(CPUState *env)
> pil |= 1 << 14;
> }
>
> - if (!pil) {
> + if (pil < (2 << env->psrpil)){
Sorry, I don't understand the patch. Where is this '2' coming from?
> if (env->interrupt_request & CPU_INTERRUPT_HARD) {
> CPUIRQ_DPRINTF("Reset CPU IRQ (current interrupt %x)\n",
> env->interrupt_index);
> @@ -287,10 +287,12 @@ void cpu_check_irqs(CPUState *env)
> break;
> }
> }
> - } else {
> + } else if (env->interrupt_request & CPU_INTERRUPT_HARD) {
> CPUIRQ_DPRINTF("Interrupts disabled, pil=%08x pil_in=%08x
> softint=%08x "
> "current interrupt %x\n",
> pil, env->pil_in, env->softint, env->interrupt_index);
> + env->interrupt_index = 0;
> + cpu_reset_interrupt(env, CPU_INTERRUPT_HARD);
Why reset the index? The idea is that the interrupt is left pending a
change to PIL etc.
> }
> }
>
> --
> 1.7.3.4
>
>
Re: [Qemu-devel] [PATCH V2 1/2] [SLIRP] Simple ARP table
On 2011-07-29 19:34, Jan Kiszka wrote:
> On 2011-07-29 18:25, Fabien Chouteau wrote:
>> This patch adds a simple ARP table in Slirp and also adds handling of
>> gratuitous ARP requests.
>>
>> Signed-off-by: Fabien Chouteau
>> ---
>> Makefile.objs |2 +-
>> slirp/arp_table.c | 50 ++
>> slirp/bootp.c | 23 --
>> slirp/slirp.c | 63
>> +---
>> slirp/slirp.h | 50 +++--
>> 5 files changed, 129 insertions(+), 59 deletions(-)
>> create mode 100644 slirp/arp_table.c
>>
>> diff --git a/Makefile.objs b/Makefile.objs
>> index 6991a9f..0c10557 100644
>> --- a/Makefile.objs
>> +++ b/Makefile.objs
>> @@ -151,7 +151,7 @@ common-obj-y += qemu-timer.o qemu-timer-common.o
>>
>> slirp-obj-y = cksum.o if.o ip_icmp.o ip_input.o ip_output.o
>> slirp-obj-y += slirp.o mbuf.o misc.o sbuf.o socket.o tcp_input.o
>> tcp_output.o
>> -slirp-obj-y += tcp_subr.o tcp_timer.o udp.o bootp.o tftp.o
>> +slirp-obj-y += tcp_subr.o tcp_timer.o udp.o bootp.o tftp.o arp_table.o
>> common-obj-$(CONFIG_SLIRP) += $(addprefix slirp/, $(slirp-obj-y))
>>
>> # xen backend driver support
>> diff --git a/slirp/arp_table.c b/slirp/arp_table.c
>> new file mode 100644
>> index 000..5d7404b
>> --- /dev/null
>> +++ b/slirp/arp_table.c
>> @@ -0,0 +1,50 @@
>> +#include "slirp.h"
>> +
>> +void arp_table_add(ArpTable *arptbl,
>> + int ip_addr,
>> + uint8_t ethaddr[ETH_ALEN])
>
> I still prefer the condensed formatting, but that's a minor nit.
>
>> +{
>> +int i;
>> +
>> +DEBUG_CALL("arp_table_add");
>> +DEBUG_ARG("ip = 0x%x", ip_addr);
>> +DEBUG_ARGS((dfd, " hw addr = %02x:%02x:%02x:%02x:%02x:%02x\n",
>> +ethaddr[0], ethaddr[1], ethaddr[2],
>> +ethaddr[3], ethaddr[4], ethaddr[5]));
>> +
>> +/* Search for an entry */
>> +for (i = 0; i < ARP_TABLE_SIZE && arptbl->table[i].ar_sip != 0; i++) {
>
> Missed that on round #1: Why treating 0.0.0.0 special? If ip_addr can be
> zero, the current logic will append every "update" of that address as a
> new entry.
>
>> +if (arptbl->table[i].ar_sip == ip_addr) {
>> +/* Update the entry */
>> +memcpy(arptbl->table[i].ar_sha, ethaddr, ETH_ALEN);
>> +return;
>> +}
>> +}
>> +
>> +/* No entry found, create a new one */
>> +arptbl->table[arptbl->next_victim].ar_sip = ip_addr;
>> +memcpy(arptbl->table[arptbl->next_victim].ar_sha, ethaddr, ETH_ALEN);
>> +arptbl->next_victim = (arptbl->next_victim + 1) % ARP_TABLE_SIZE;
>> +}
>> +
>> +bool arp_table_search(ArpTable *arptbl,
>> + int in_ip_addr,
>> + uint8_t out_ethaddr[ETH_ALEN])
>> +{
>> +int i;
>> +
>> +DEBUG_CALL("arp_table_search");
>> +DEBUG_ARG("ip = 0x%x", in_ip_addr);
>> +
>> +for (i = 0; i < ARP_TABLE_SIZE; i++) {
>> +if (arptbl->table[i].ar_sip == in_ip_addr) {
>> +memcpy(out_ethaddr, arptbl->table[i].ar_sha, ETH_ALEN);
>> +DEBUG_ARGS((dfd, " found hw addr =
>> %02x:%02x:%02x:%02x:%02x:%02x\n",
>> +out_ethaddr[0], out_ethaddr[1], out_ethaddr[2],
>> +out_ethaddr[3], out_ethaddr[4], out_ethaddr[5]));
>> +return 1;
>> +}
>> +}
>> +
>> +return 0;
>> +}
>> diff --git a/slirp/bootp.c b/slirp/bootp.c
>> index 1eb2ed1..07cbb80 100644
>> --- a/slirp/bootp.c
>> +++ b/slirp/bootp.c
>> @@ -149,6 +149,7 @@ static void bootp_reply(Slirp *slirp, const struct
>> bootp_t *bp)
>> struct in_addr preq_addr;
>> int dhcp_msg_type, val;
>> uint8_t *q;
>> +uint8_t client_ethaddr[ETH_ALEN];
>>
>> /* extract exact DHCP msg type */
>> dhcp_decode(bp, &dhcp_msg_type, &preq_addr);
>> @@ -164,8 +165,9 @@ static void bootp_reply(Slirp *slirp, const struct
>> bootp_t *bp)
>> if (dhcp_msg_type != DHCPDISCOVER &&
>> dhcp_msg_type != DHCPREQUEST)
>> return;
>> -/* XXX: this is a hack to get the client mac address */
>> -memcpy(slirp->client_ethaddr, bp->bp_hwaddr, 6);
>> +
>> +/* Get client's hardware address from bootp request */
>> +memcpy(client_ethaddr, bp->bp_hwaddr, ETH_ALEN);
>>
>> m = m_get(slirp);
>> if (!m) {
>> @@ -178,25 +180,25 @@ static void bootp_reply(Slirp *slirp, const struct
>> bootp_t *bp)
>>
>> if (dhcp_msg_type == DHCPDISCOVER) {
>> if (preq_addr.s_addr != htonl(0L)) {
>> -bc = request_addr(slirp, &preq_addr, slirp->client_ethaddr);
>> +bc = request_addr(slirp, &preq_addr, client_ethaddr);
>> if (bc) {
>> daddr.sin_addr = preq_addr;
>> }
>> }
>> if (!bc) {
>> new_addr:
>> -bc = get_new_addr(slirp, &daddr.sin_addr,
>> slirp->client_ethaddr);
>> +bc = get_ne
Re: [Qemu-devel] [PATCH V2 1/2] [SLIRP] Simple ARP table
On 2011-07-30 11:09, Jan Kiszka wrote:
> On 2011-07-29 19:34, Jan Kiszka wrote:
>> On 2011-07-29 18:25, Fabien Chouteau wrote:
>>> This patch adds a simple ARP table in Slirp and also adds handling of
>>> gratuitous ARP requests.
>>>
>>> Signed-off-by: Fabien Chouteau
>>> ---
>>> Makefile.objs |2 +-
>>> slirp/arp_table.c | 50 ++
>>> slirp/bootp.c | 23 --
>>> slirp/slirp.c | 63
>>> +---
>>> slirp/slirp.h | 50 +++--
>>> 5 files changed, 129 insertions(+), 59 deletions(-)
>>> create mode 100644 slirp/arp_table.c
>>>
>>> diff --git a/Makefile.objs b/Makefile.objs
>>> index 6991a9f..0c10557 100644
>>> --- a/Makefile.objs
>>> +++ b/Makefile.objs
>>> @@ -151,7 +151,7 @@ common-obj-y += qemu-timer.o qemu-timer-common.o
>>>
>>> slirp-obj-y = cksum.o if.o ip_icmp.o ip_input.o ip_output.o
>>> slirp-obj-y += slirp.o mbuf.o misc.o sbuf.o socket.o tcp_input.o
>>> tcp_output.o
>>> -slirp-obj-y += tcp_subr.o tcp_timer.o udp.o bootp.o tftp.o
>>> +slirp-obj-y += tcp_subr.o tcp_timer.o udp.o bootp.o tftp.o arp_table.o
>>> common-obj-$(CONFIG_SLIRP) += $(addprefix slirp/, $(slirp-obj-y))
>>>
>>> # xen backend driver support
>>> diff --git a/slirp/arp_table.c b/slirp/arp_table.c
>>> new file mode 100644
>>> index 000..5d7404b
>>> --- /dev/null
>>> +++ b/slirp/arp_table.c
>>> @@ -0,0 +1,50 @@
>>> +#include "slirp.h"
>>> +
>>> +void arp_table_add(ArpTable *arptbl,
>>> + int ip_addr,
>>> + uint8_t ethaddr[ETH_ALEN])
>>
>> I still prefer the condensed formatting, but that's a minor nit.
>>
>>> +{
>>> +int i;
>>> +
>>> +DEBUG_CALL("arp_table_add");
>>> +DEBUG_ARG("ip = 0x%x", ip_addr);
>>> +DEBUG_ARGS((dfd, " hw addr = %02x:%02x:%02x:%02x:%02x:%02x\n",
>>> +ethaddr[0], ethaddr[1], ethaddr[2],
>>> +ethaddr[3], ethaddr[4], ethaddr[5]));
>>> +
>>> +/* Search for an entry */
>>> +for (i = 0; i < ARP_TABLE_SIZE && arptbl->table[i].ar_sip != 0; i++) {
>>
>> Missed that on round #1: Why treating 0.0.0.0 special? If ip_addr can be
>> zero, the current logic will append every "update" of that address as a
>> new entry.
>>
>>> +if (arptbl->table[i].ar_sip == ip_addr) {
>>> +/* Update the entry */
>>> +memcpy(arptbl->table[i].ar_sha, ethaddr, ETH_ALEN);
>>> +return;
>>> +}
>>> +}
>>> +
>>> +/* No entry found, create a new one */
>>> +arptbl->table[arptbl->next_victim].ar_sip = ip_addr;
>>> +memcpy(arptbl->table[arptbl->next_victim].ar_sha, ethaddr, ETH_ALEN);
>>> +arptbl->next_victim = (arptbl->next_victim + 1) % ARP_TABLE_SIZE;
>>> +}
>>> +
>>> +bool arp_table_search(ArpTable *arptbl,
>>> + int in_ip_addr,
>>> + uint8_t out_ethaddr[ETH_ALEN])
>>> +{
>>> +int i;
>>> +
>>> +DEBUG_CALL("arp_table_search");
>>> +DEBUG_ARG("ip = 0x%x", in_ip_addr);
>>> +
>>> +for (i = 0; i < ARP_TABLE_SIZE; i++) {
>>> +if (arptbl->table[i].ar_sip == in_ip_addr) {
>>> +memcpy(out_ethaddr, arptbl->table[i].ar_sha, ETH_ALEN);
>>> +DEBUG_ARGS((dfd, " found hw addr =
>>> %02x:%02x:%02x:%02x:%02x:%02x\n",
>>> +out_ethaddr[0], out_ethaddr[1], out_ethaddr[2],
>>> +out_ethaddr[3], out_ethaddr[4], out_ethaddr[5]));
>>> +return 1;
>>> +}
>>> +}
>>> +
>>> +return 0;
>>> +}
>>> diff --git a/slirp/bootp.c b/slirp/bootp.c
>>> index 1eb2ed1..07cbb80 100644
>>> --- a/slirp/bootp.c
>>> +++ b/slirp/bootp.c
>>> @@ -149,6 +149,7 @@ static void bootp_reply(Slirp *slirp, const struct
>>> bootp_t *bp)
>>> struct in_addr preq_addr;
>>> int dhcp_msg_type, val;
>>> uint8_t *q;
>>> +uint8_t client_ethaddr[ETH_ALEN];
>>>
>>> /* extract exact DHCP msg type */
>>> dhcp_decode(bp, &dhcp_msg_type, &preq_addr);
>>> @@ -164,8 +165,9 @@ static void bootp_reply(Slirp *slirp, const struct
>>> bootp_t *bp)
>>> if (dhcp_msg_type != DHCPDISCOVER &&
>>> dhcp_msg_type != DHCPREQUEST)
>>> return;
>>> -/* XXX: this is a hack to get the client mac address */
>>> -memcpy(slirp->client_ethaddr, bp->bp_hwaddr, 6);
>>> +
>>> +/* Get client's hardware address from bootp request */
>>> +memcpy(client_ethaddr, bp->bp_hwaddr, ETH_ALEN);
>>>
>>> m = m_get(slirp);
>>> if (!m) {
>>> @@ -178,25 +180,25 @@ static void bootp_reply(Slirp *slirp, const struct
>>> bootp_t *bp)
>>>
>>> if (dhcp_msg_type == DHCPDISCOVER) {
>>> if (preq_addr.s_addr != htonl(0L)) {
>>> -bc = request_addr(slirp, &preq_addr, slirp->client_ethaddr);
>>> +bc = request_addr(slirp, &preq_addr, client_ethaddr);
>>> if (bc) {
>>> daddr.sin_addr = preq_addr;
>>> }
>>>
Re: [Qemu-devel] [PATCH] fix disabling interrupts in sun4u
On Thu, Jul 28, 2011 at 5:44 PM, Artyom Tarasenko wrote: > On Thu, Jul 28, 2011 at 3:40 PM, wrote: >> At Thu, 28 Jul 2011 14:50:57 +0200, >> Artyom Tarasenko wrote: >>> On Thu, Jul 28, 2011 at 2:03 PM, wrote: >>> > At Thu, 28 Jul 2011 13:51:08 +0200, >>> > Artyom Tarasenko wrote: >>> >> On Thu, Jul 28, 2011 at 12:31 PM, wrote: >>> >> > Hi, >>> >> > >>> >> > At Mon, 25 Jul 2011 19:22:38 +0200, >>> >> > Artyom Tarasenko wrote: >>> >> > >>> >> >> clear interrupt request if the interrupt priority < CPU pil >>> >> >> clear hardware interrupt request if interrupts are disabled >>> >> > >>> >> > Not directly related to the fix, but I'd like to note a problem >>> >> > of hw/sun4u.c interrupt code: >>> >> > >>> >> > The interrupt code probably mixes hardware interrupts and >>> >> > software interrupts. >>> >> > %pil is for software interrupts (interrupt_level_n traps). >>> >> > %pil can not mask hardware interrupts (interrupt_vector traps); >>> >> > the CPU raises interrupt_vector traps even on %pil=15. >>> >> > But in cpu_check_irqs() and cpu_set_irq(), hardware interrupts >>> >> > seem to be masked by %pil. >>> >> >>> >> The interrupt_vector traps are currently not implemented, are they? >>> >> So it's hard to tell whether they are masked. >>> > >>> > Yes, interrupt_vector is not implemented yet. >>> > I failed to explain the problem. >>> > The problem is that cpu_set_irqs() should raise interrupt_vector >>> > traps but it raises interrupt_level_n traps actually. >>> > sun4uv_init() calls qemu_allocate_irqs() with cpu_set_irq as >>> > the 1st argument. The allocated irqs (the irq variable) are >>> > passed to pci_apb_init(). APB should generate interrupt_vector >>> > traps (hardware interrupts), not the interrupt_vector_n traps. >>> >>> Yes, this is true. But it's more complicated than this: cpu_check_irqs >>> also checks tick/stick/hstick interrupts. They should produce the >>> interrupt_level_n traps as they currently do. >> >> That's right. >> tick/stick/hstick must raise interrupt_level_n traps. >> >>> The patch merely fixes the problem of hanging on a interrupt_vector_n >>> trap if the trap handler uses pil for interrupt masking. The problem >>> exists independently from interrupt_vector trap generation (as you >>> pointed out). >> >> I understand what is the problem that your patch is going to fix. >> Thanks for the explanation. >> > > Sorry, I haven't implied that you don't. Meant to say "As you pointed > out, the problem exists independently...". > >>> Do you have objections to this patch in its current form? >> >> No, I don't have any objections. >> >>> > The interrupts from APB would be reported by cpu_set_irq(), >>> > but cpu_set_irq() seems to generate interrupt_vector_n traps. >>> >>> For me it's not obvious. The interrupt vector not just one line, but >>> the vector, which is written in the corresponding CPU register (also >>> missing in the current qemu implementation). On the real hardware the >>> vector is created by the IOMMU (PBM/APB/...). If qemu intends to >>> support multiple chipsets, we should keep it the way it's done on the >>> real hardware (for instance the interrupt vectors for on-board devices >>> on Ultra-1 and E6500 are not the same). >> >> Sorry, I can't keep up with this vector thing... >> Does the CPU receive hardware interrupts as interrupt_vector traps >> (trap type=0x60) regardless of the kind of the interrupt controller, >> doesn't it? > > It does indeed, but it also stores the interrupt vector identifying > the initiator device, in a CPU register readable with asi 0x7f . > What would APB pass to the cpu_set_irq? I see the three following variants: > > a) it passes the PCI interrupt id, which is translated to the > interrupt vector in cpu_set_irq() > b) it passes the vector. This implies that 2048 (0-0x7ff) CPU > interrupts have to be allocated. > c) hack combining a+b: allocate only the interrupts known to be used > and translate an internal interrupt id to a vector. > > The variant "a" is bad because it doesn't allow support for different > chipsets. The variant "b" is bad because qemu has to allocate way too > many interrupts. Only few of them will be used actually. The variant > "c" is bad, well, because it's a hack. > > That's why I suggest using another interface between APB and CPU. > > Have I overlooked something? d) use interrupts as a bit field, allocate 11 interrupts for each bit in the vector e) use a single interrupt but store the vector in the interrupt controller where CPU can read it when ASI 0x7f is used f) implement interrupts with message passing capabilities as discussed earlier with APIC cpu_receive_ivec(CPUState *env, int vector) would work, but this means that the interrupt controller needs a pointer to CPUState which is ugly. Doing this the other way around like in e) option, 0x7f ASI access would fetch the vector from controller. This would mean that CPUState would include a pointer to the interrupt controller. This is not so clean either
[Qemu-devel] [PATCH 00/15] sdl: Usability improvements
As SDL is my preferred way of working ad-hoc with guests, I had a closer look at oddities and shortcomings that this GUI exposed, at least here on Linux hosts. The result is a series of patches I've now finally polished and completed. Highlights: - fix termination in -no-shutdown mode - fix various issues when switching to/from full screen mode - polish mouse grabbing in full screen mode, under text console and when in absolute mouse mode - dynamically grab keyboard input in absolute mouse mode, enabling e.g. ALT+TAB in the guest - add zoom hot keys to make window scaling more attractive - refactor some ugly functions Please review/merge. CC: Stefano Stabellini Jan Kiszka (15): sdl: Fix termination in -no-shutdown mode sdl: Do not make full screen mode resizable sdl: Avoid redundant scaling deactivation sdl: Properly mark modifier+u as hotkey sdl: Fix full screen toggling from scaled mode sdl: Restore scaling mode on return from full screen sdl: Drop bogus gui_fullscreen_initial_grab sdl: Initialize gui_fullscreen earlier during setup sdl: Consistently avoid grabbing input for text consoles sdl: Never release input while in full screen mode sdl: Fix cursor handling when switching consoles in absolute mouse mode sdl: Dynamically grab input in absolute mouse mode sdl: Add zoom hot keys sdl: Factor out event handlers from sdl_refresh sdl: Refactor sdl_send_mouse_event qemu-doc.texi |8 + ui/sdl.c | 547 - 2 files changed, 355 insertions(+), 200 deletions(-) -- 1.7.3.4
[Qemu-devel] [PATCH 01/15] sdl: Fix termination in -no-shutdown mode
From: Jan Kiszka
Just like the monitor does, we need to clear no_shutdown before calling
qemu_system_shutdown_request on quit requests. Otherwise, QEMU just
stops the VM.
Signed-off-by: Jan Kiszka
---
ui/sdl.c |4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/ui/sdl.c b/ui/sdl.c
index 6dbc5cb..9efcda5 100644
--- a/ui/sdl.c
+++ b/ui/sdl.c
@@ -672,8 +672,10 @@ static void sdl_refresh(DisplayState *ds)
sdl_process_key(&ev->key);
break;
case SDL_QUIT:
-if (!no_quit)
+if (!no_quit) {
+no_shutdown = 0;
qemu_system_shutdown_request();
+}
break;
case SDL_MOUSEMOTION:
if (gui_grab || kbd_mouse_is_absolute() ||
--
1.7.3.4
Re: [Qemu-devel] [PATCH] v6 revamp acpitable parsing and allow to specify complete (headerful) table
Thanks, applied.
On Fri, Jul 29, 2011 at 4:49 AM, Isaku Yamahata wrote:
> On Fri, Jul 15, 2011 at 07:51:43PM +0300, Blue Swirl wrote:
>> On Fri, Jul 15, 2011 at 6:18 PM, John Baboval
>> wrote:
>> > Is there something I can do to help take this patch the rest of the way?
>> >
>> > I'd hate to see it die because of a style issue and a compiler warning.
>>
>> There's also suspicious missing 'break' statement. How about fixing
>> the issues and submitting the patch?
>
> I fixed the compile error.
> I think the missing break is intentional, so added an comment there. Michael?
> Blue, can you please take a look of it?
>
>
> From 9a5e4158074ea251ab064a946927bcaf861f5c1e Mon Sep 17 00:00:00 2001
> Message-Id:
> <9a5e4158074ea251ab064a946927bcaf861f5c1e.1311903724.git.yamah...@valinux.co.jp>
> From: Michael Tokarev
> Date: Thu, 12 May 2011 18:44:17 +0400
> Subject: [PATCH] revamp acpitable parsing and allow to specify complete
> (headerful) table
>
> From Michael Tokarev
>
> This patch almost rewrites acpi_table_add() function
> (but still leaves it using old get_param_value() interface).
> The result is that it's now possible to specify whole table
> (together with a header) in an external file, instead of just
> data portion, with a new file= parameter, but at the same time
> it's still possible to specify header fields as before.
>
> Now with the checkpatch.pl formatting fixes, thanks to
> Stefan Hajnoczi for suggestions, with changes from
> Isaku Yamahata, and with my further refinements.
>
> Signed-off-by: Michael Tokarev
> Cc:: Isaku Yamahata
> Cc: John Baboval
> Cc: Blue Swirl
>
> ---
> v5: rediffed against current qemu/master.
> v6: fix one "} else {" coding style defect (noted by Blue Swirl)
> v7: style fix and added an comment for suspicious break
> I think that the missing break of case 0 is intentional.
> I added the fallthrough comment there.
> ---
> hw/acpi.c | 298
> ---
> qemu-options.hx | 7 +-
> 2 files changed, 178 insertions(+), 127 deletions(-)
>
> diff --git a/hw/acpi.c b/hw/acpi.c
> index ad40fb4..79ec66c 100644
> --- a/hw/acpi.c
> +++ b/hw/acpi.c
> @@ -20,19 +20,30 @@
> #include "pc.h"
> #include "acpi.h"
>
> -struct acpi_table_header
> -{
> - char signature [4]; /* ACPI signature (4 ASCII characters) */
> +struct acpi_table_header {
> + uint16_t _length; /* our length, not actual part of the hdr */
> + /* XXX why we have 2 length fields here? */
> + char sig[4]; /* ACPI signature (4 ASCII characters) */
> uint32_t length; /* Length of table, in bytes, including header
> */
> uint8_t revision; /* ACPI Specification minor version # */
> uint8_t checksum; /* To make sum of entire table == 0 */
> - char oem_id [6]; /* OEM identification */
> - char oem_table_id [8]; /* OEM table identification */
> + char oem_id[6]; /* OEM identification */
> + char oem_table_id[8]; /* OEM table identification */
> uint32_t oem_revision; /* OEM revision number */
> - char asl_compiler_id [4]; /* ASL compiler vendor ID */
> + char asl_compiler_id[4]; /* ASL compiler vendor ID */
> uint32_t asl_compiler_revision; /* ASL compiler revision number */
> } __attribute__((packed));
>
> +#define ACPI_TABLE_HDR_SIZE sizeof(struct acpi_table_header)
> +#define ACPI_TABLE_PFX_SIZE sizeof(uint16_t) /* size of the extra prefix */
> +
> +static const char dfl_hdr[ACPI_TABLE_HDR_SIZE] =
> + "\0\0" /* fake _length (2) */
> + "QEMU\0\0\0\0\1\0" /* sig (4), len(4), revno (1), csum (1) */
> + "QEMUQEQEMUQEMU\1\0\0\0" /* OEM id (6), table (8), revno (4) */
> + "QEMU\1\0\0\0" /* ASL compiler ID (4), version (4) */
> + ;
> +
> char *acpi_tables;
> size_t acpi_tables_len;
>
> @@ -40,163 +51,198 @@ static int acpi_checksum(const uint8_t *data, int len)
> {
> int sum, i;
> sum = 0;
> - for(i = 0; i < len; i++)
> + for (i = 0; i < len; i++) {
> sum += data[i];
> + }
> return (-sum) & 0xff;
> }
>
> +/* like strncpy() but zero-fills the tail of destination */
> +static void strzcpy(char *dst, const char *src, size_t size)
> +{
> + size_t len = strlen(src);
> + if (len >= size) {
> + len = size;
> + } else {
> + memset(dst + len, 0, size - len);
> + }
> + memcpy(dst, src, len);
> +}
> +
> +/* XXX fixme: this function uses obsolete argument parsing interface */
> int acpi_table_add(const char *t)
> {
> - static const char *dfl_id = "QEMUQEMU";
> char buf[1024], *p, *f;
> - struct acpi_table_header acpi_hdr;
> unsigned long val;
> - uint32_t length;
> - struct acpi_table_header *acpi_hdr_p;
> - size_t off;
> + size_t len, start, allen;
> + bool has_header;
> + int changed;
> + int r;
> + struct acpi_table_header hdr;
> +
> + r = 0;
> + r |= get_param_value(buf
[Qemu-devel] [PATCH 12/15] sdl: Dynamically grab input in absolute mouse mode
From: Jan Kiszka
Not grabbing the input means that special keys like ALT+TAB are still
handled by the host. Improve the usability by grabbing input once the
mouse is inside the guest screen, provided the SDL window has the input
focus. Release it again when the mouse is moved to any border. Also grab
the input when we gain the input focus and the mouse is within the
screen limits.
Signed-off-by: Jan Kiszka
---
ui/sdl.c | 43 +--
1 files changed, 37 insertions(+), 6 deletions(-)
diff --git a/ui/sdl.c b/ui/sdl.c
index 5ad38d5..e8ac3bb 100644
--- a/ui/sdl.c
+++ b/ui/sdl.c
@@ -490,15 +490,12 @@ static void sdl_mouse_mode_change(Notifier *notify, void
*data)
{
if (kbd_mouse_is_absolute()) {
if (!absolute_enabled) {
-sdl_hide_cursor();
-if (gui_grab) {
-sdl_grab_end();
-}
+sdl_grab_start();
absolute_enabled = 1;
}
} else if (absolute_enabled) {
- sdl_show_cursor();
- absolute_enabled = 0;
+sdl_grab_end();
+absolute_enabled = 0;
}
}
@@ -572,6 +569,19 @@ static void toggle_full_screen(DisplayState *ds)
vga_hw_update();
}
+static void absolute_mouse_grab(void)
+{
+int mouse_x, mouse_y;
+
+if (SDL_GetAppState() & SDL_APPINPUTFOCUS) {
+SDL_GetMouseState(&mouse_x, &mouse_y);
+if (mouse_x > 0 && mouse_x < real_screen->w - 1 &&
+mouse_y > 0 && mouse_y < real_screen->h - 1) {
+sdl_grab_start();
+}
+}
+}
+
static void sdl_refresh(DisplayState *ds)
{
SDL_Event ev1, *ev = &ev1;
@@ -638,6 +648,7 @@ static void sdl_refresh(DisplayState *ds)
}
} else if (absolute_enabled) {
sdl_hide_cursor();
+absolute_mouse_grab();
}
break;
default:
@@ -724,6 +735,22 @@ static void sdl_refresh(DisplayState *ds)
}
break;
case SDL_MOUSEMOTION:
+if (is_graphic_console() &&
+(kbd_mouse_is_absolute() || absolute_enabled)) {
+int max_x = real_screen->w - 1;
+int max_y = real_screen->h - 1;
+
+if (gui_grab &&
+(ev->motion.x == 0 || ev->motion.y == 0 ||
+ ev->motion.x == max_x || ev->motion.y == max_y)) {
+sdl_grab_end();
+}
+if (!gui_grab && SDL_GetAppState() & SDL_APPINPUTFOCUS &&
+(ev->motion.x > 0 && ev->motion.x < max_x &&
+ ev->motion.y > 0 && ev->motion.y < max_y)) {
+sdl_grab_start();
+}
+}
if (gui_grab || kbd_mouse_is_absolute() ||
absolute_enabled) {
sdl_send_mouse_event(ev->motion.xrel, ev->motion.yrel, 0,
@@ -764,6 +791,10 @@ static void sdl_refresh(DisplayState *ds)
!ev->active.gain && !gui_fullscreen) {
sdl_grab_end();
}
+if (!gui_grab && ev->active.gain && is_graphic_console() &&
+(kbd_mouse_is_absolute() || absolute_enabled)) {
+absolute_mouse_grab();
+}
if (ev->active.state & SDL_APPACTIVE) {
if (ev->active.gain) {
/* Back to default interval */
--
1.7.3.4
[Qemu-devel] [PATCH 10/15] sdl: Never release input while in full screen mode
From: Jan Kiszka
It's confusing to suddenly find two mice in full screen mode when
switching consoles or accidentally hitting the grab hot keys.
Signed-off-by: Jan Kiszka
---
ui/sdl.c |7 ---
1 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/ui/sdl.c b/ui/sdl.c
index 27465b2..662ffef 100644
--- a/ui/sdl.c
+++ b/ui/sdl.c
@@ -626,9 +626,10 @@ static void sdl_refresh(DisplayState *ds)
reset_keys();
console_select(keycode - 0x02);
if (!is_graphic_console()) {
-/* display grab if going to a text console */
-if (gui_grab)
+/* release grab if going to a text console */
+if (gui_grab && !gui_fullscreen) {
sdl_grab_end();
+}
}
gui_keysym = 1;
break;
@@ -694,7 +695,7 @@ static void sdl_refresh(DisplayState *ds)
SDL_GetAppState() & SDL_APPACTIVE) {
sdl_grab_start();
}
-} else {
+} else if (!gui_fullscreen) {
sdl_grab_end();
}
/* SDL does not send back all the
--
1.7.3.4
[Qemu-devel] [PATCH 03/15] sdl: Avoid redundant scaling deactivation
From: Jan Kiszka
Prevents screen flickering.
Signed-off-by: Jan Kiszka
---
ui/sdl.c | 10 ++
1 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/ui/sdl.c b/ui/sdl.c
index 9a92b47..1563ee1 100644
--- a/ui/sdl.c
+++ b/ui/sdl.c
@@ -583,10 +583,12 @@ static void sdl_refresh(DisplayState *ds)
gui_keysym = 1;
break;
case 0x16: /* 'u' key on US keyboard */
-scaling_active = 0;
-sdl_resize(ds);
-vga_hw_invalidate();
-vga_hw_update();
+if (scaling_active) {
+scaling_active = 0;
+sdl_resize(ds);
+vga_hw_invalidate();
+vga_hw_update();
+}
break;
case 0x02 ... 0x0a: /* '1' to '9' keys */
/* Reset the modifiers sent to the current console */
--
1.7.3.4
[Qemu-devel] [PATCH 09/15] sdl: Consistently avoid grabbing input for text consoles
From: Jan Kiszka
There were some preexisting bits that released the input when switching
to text console. This patch spreads this logic consistently and also
avoids grabbing the input while a text console is active.
Signed-off-by: Jan Kiszka
---
ui/sdl.c |9 ++---
1 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/ui/sdl.c b/ui/sdl.c
index f19bae2..27465b2 100644
--- a/ui/sdl.c
+++ b/ui/sdl.c
@@ -564,8 +564,9 @@ static void toggle_full_screen(DisplayState *ds)
} else {
do_sdl_resize(ds_get_width(ds), ds_get_height(ds), 0);
}
-if (!gui_saved_grab)
+if (!gui_saved_grab || !is_graphic_console()) {
sdl_grab_end();
+}
}
vga_hw_invalidate();
vga_hw_update();
@@ -689,8 +690,10 @@ static void sdl_refresh(DisplayState *ds)
'SDL_WM_GrabInput(SDL_GRAB_ON)'
from blocking all the application
(SDL bug). */
-if (SDL_GetAppState() & SDL_APPACTIVE)
+if (is_graphic_console() &&
+SDL_GetAppState() & SDL_APPACTIVE) {
sdl_grab_start();
+}
} else {
sdl_grab_end();
}
@@ -721,7 +724,7 @@ static void sdl_refresh(DisplayState *ds)
break;
case SDL_MOUSEBUTTONDOWN:
case SDL_MOUSEBUTTONUP:
-{
+if (is_graphic_console()) {
SDL_MouseButtonEvent *bev = &ev->button;
if (!gui_grab && !kbd_mouse_is_absolute()) {
if (ev->type == SDL_MOUSEBUTTONDOWN &&
--
1.7.3.4
[Qemu-devel] [PATCH 02/15] sdl: Do not make full screen mode resizable
From: Jan Kiszka
This prevents continuous resizing events and improper screen setups when
going full screen.
CC: Stefano Stabellini
Signed-off-by: Jan Kiszka
---
ui/sdl.c |7 +--
1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/ui/sdl.c b/ui/sdl.c
index 9efcda5..9a92b47 100644
--- a/ui/sdl.c
+++ b/ui/sdl.c
@@ -97,9 +97,12 @@ static void do_sdl_resize(int new_width, int new_height, int
bpp)
//printf("resizing to %d %d\n", w, h);
-flags = SDL_HWSURFACE|SDL_ASYNCBLIT|SDL_HWACCEL|SDL_RESIZABLE;
-if (gui_fullscreen)
+flags = SDL_HWSURFACE | SDL_ASYNCBLIT | SDL_HWACCEL;
+if (gui_fullscreen) {
flags |= SDL_FULLSCREEN;
+} else {
+flags |= SDL_RESIZABLE;
+}
if (gui_noframe)
flags |= SDL_NOFRAME;
--
1.7.3.4
[Qemu-devel] [PATCH 07/15] sdl: Drop bogus gui_fullscreen_initial_grab
From: Jan Kiszka
There must be no difference between initial -full-screen and switching
to this mode via the hot key.
Signed-off-by: Jan Kiszka
---
ui/sdl.c |4 +---
1 files changed, 1 insertions(+), 3 deletions(-)
diff --git a/ui/sdl.c b/ui/sdl.c
index e18c59a..80bf776 100644
--- a/ui/sdl.c
+++ b/ui/sdl.c
@@ -47,7 +47,6 @@ static int gui_fullscreen;
static int gui_noframe;
static int gui_key_modifier_pressed;
static int gui_keysym;
-static int gui_fullscreen_initial_grab;
static int gui_grab_code = KMOD_LALT | KMOD_LCTRL;
static uint8_t modifiers_state[256];
static int width, height;
@@ -751,7 +750,7 @@ static void sdl_refresh(DisplayState *ds)
break;
case SDL_ACTIVEEVENT:
if (gui_grab && ev->active.state == SDL_APPINPUTFOCUS &&
-!ev->active.gain && !gui_fullscreen_initial_grab) {
+!ev->active.gain && !gui_fullscreen) {
sdl_grab_end();
}
if (ev->active.state & SDL_APPACTIVE) {
@@ -923,7 +922,6 @@ void sdl_display_init(DisplayState *ds, int full_screen,
int no_frame)
atexit(sdl_cleanup);
if (full_screen) {
gui_fullscreen = 1;
-gui_fullscreen_initial_grab = 1;
sdl_grab_start();
}
}
--
1.7.3.4
[Qemu-devel] [PATCH 04/15] sdl: Properly mark modifier+u as hotkey
From: Jan Kiszka Signed-off-by: Jan Kiszka --- ui/sdl.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/ui/sdl.c b/ui/sdl.c index 1563ee1..e6c9597 100644 --- a/ui/sdl.c +++ b/ui/sdl.c @@ -589,6 +589,7 @@ static void sdl_refresh(DisplayState *ds) vga_hw_invalidate(); vga_hw_update(); } +gui_keysym = 1; break; case 0x02 ... 0x0a: /* '1' to '9' keys */ /* Reset the modifiers sent to the current console */ -- 1.7.3.4
[Qemu-devel] [PATCH 13/15] sdl: Add zoom hot keys
From: Jan Kiszka
Allow to enlarge or shrink the screen via CTRL-ALT-+/-. In contrast to
scaling the window, these controls always preserve the aspect ratio of
the current console.
CC: Stefano Stabellini
Signed-off-by: Jan Kiszka
---
qemu-doc.texi |8
ui/sdl.c | 13 +
2 files changed, 21 insertions(+), 0 deletions(-)
diff --git a/qemu-doc.texi b/qemu-doc.texi
index 47e1991..31199f6 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -288,6 +288,14 @@ then the modifier is Ctrl-Alt-Shift (instead of Ctrl-Alt)
and if you use
@kindex Ctrl-Alt-f
Toggle full screen
+@item Ctrl-Alt-+
+@kindex Ctrl-Alt-+
+Enlarge the screen
+
+@item Ctrl-Alt--
+@kindex Ctrl-Alt--
+Shrink the screen
+
@item Ctrl-Alt-u
@kindex Ctrl-Alt-u
Restore the screen's un-scaled dimensions
diff --git a/ui/sdl.c b/ui/sdl.c
index e8ac3bb..fc63c8e 100644
--- a/ui/sdl.c
+++ b/ui/sdl.c
@@ -651,6 +651,19 @@ static void sdl_refresh(DisplayState *ds)
absolute_mouse_grab();
}
break;
+case 0x1b: /* '+' */
+case 0x35: /* '-' */
+if (!gui_fullscreen) {
+int width = MAX(real_screen->w +
+(keycode == 0x1b ? 50 : -50), 160);
+int height = (ds_get_height(ds) * width) /
+ ds_get_width(ds);
+
+sdl_scale(ds, width, height);
+vga_hw_invalidate();
+vga_hw_update();
+gui_keysym = 1;
+}
default:
break;
}
--
1.7.3.4
[Qemu-devel] [PATCH 06/15] sdl: Restore scaling mode on return from full screen
From: Jan Kiszka
Save the scaling mode and its geometry when going full screen, restore
it when returning to windowed mode.
CC: Stefano Stabellini
Signed-off-by: Jan Kiszka
---
ui/sdl.c | 48 +---
1 files changed, 33 insertions(+), 15 deletions(-)
diff --git a/ui/sdl.c b/ui/sdl.c
index 4acfe81..e18c59a 100644
--- a/ui/sdl.c
+++ b/ui/sdl.c
@@ -39,6 +39,9 @@ static SDL_Surface *real_screen;
static SDL_Surface *guest_screen = NULL;
static int gui_grab; /* if true, all keyboard/mouse events are grabbed */
static int last_vm_running;
+static bool gui_saved_scaling;
+static int gui_saved_width;
+static int gui_saved_height;
static int gui_saved_grab;
static int gui_fullscreen;
static int gui_noframe;
@@ -526,16 +529,42 @@ static void sdl_send_mouse_event(int dx, int dy, int dz,
int x, int y, int state
kbd_mouse_event(dx, dy, dz, buttons);
}
+static void sdl_scale(DisplayState *ds, int width, int height)
+{
+int bpp = real_screen->format->BitsPerPixel;
+
+if (bpp != 16 && bpp != 32) {
+bpp = 32;
+}
+do_sdl_resize(width, height, bpp);
+scaling_active = 1;
+if (!is_buffer_shared(ds->surface)) {
+ds->surface = qemu_resize_displaysurface(ds, ds_get_width(ds),
+ ds_get_height(ds));
+dpy_resize(ds);
+}
+}
+
static void toggle_full_screen(DisplayState *ds)
{
gui_fullscreen = !gui_fullscreen;
-do_sdl_resize(ds_get_width(ds), ds_get_height(ds),
- ds_get_bits_per_pixel(ds));
if (gui_fullscreen) {
+gui_saved_width = real_screen->w;
+gui_saved_height = real_screen->h;
+gui_saved_scaling = scaling_active;
+
+do_sdl_resize(ds_get_width(ds), ds_get_height(ds),
+ ds_get_bits_per_pixel(ds));
scaling_active = 0;
+
gui_saved_grab = gui_grab;
sdl_grab_start();
} else {
+if (gui_saved_scaling) {
+sdl_scale(ds, gui_saved_width, gui_saved_height);
+} else {
+do_sdl_resize(ds_get_width(ds), ds_get_height(ds), 0);
+}
if (!gui_saved_grab)
sdl_grab_end();
}
@@ -737,22 +766,11 @@ static void sdl_refresh(DisplayState *ds)
}
}
break;
- case SDL_VIDEORESIZE:
-{
- SDL_ResizeEvent *rev = &ev->resize;
-int bpp = real_screen->format->BitsPerPixel;
-if (bpp != 16 && bpp != 32)
-bpp = 32;
-do_sdl_resize(rev->w, rev->h, bpp);
-scaling_active = 1;
-if (!is_buffer_shared(ds->surface)) {
-ds->surface = qemu_resize_displaysurface(ds, ds_get_width(ds),
ds_get_height(ds));
-dpy_resize(ds);
-}
+case SDL_VIDEORESIZE:
+sdl_scale(ds, ev->resize.w, ev->resize.h);
vga_hw_invalidate();
vga_hw_update();
break;
-}
default:
break;
}
--
1.7.3.4
[Qemu-devel] [PATCH 05/15] sdl: Fix full screen toggling from scaled mode
From: Jan Kiszka
When switching to full screen mode from a scaled window, we need to
resize to DisplayState's dimension, not the scaled "real" screen size.
Moreover, scaling mode may have manipulated the bpp. So we need to
restore it from the DisplayState as well.
CC: Stefano Stabellini
Signed-off-by: Jan Kiszka
---
ui/sdl.c |3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/ui/sdl.c b/ui/sdl.c
index e6c9597..4acfe81 100644
--- a/ui/sdl.c
+++ b/ui/sdl.c
@@ -529,7 +529,8 @@ static void sdl_send_mouse_event(int dx, int dy, int dz,
int x, int y, int state
static void toggle_full_screen(DisplayState *ds)
{
gui_fullscreen = !gui_fullscreen;
-do_sdl_resize(real_screen->w, real_screen->h,
real_screen->format->BitsPerPixel);
+do_sdl_resize(ds_get_width(ds), ds_get_height(ds),
+ ds_get_bits_per_pixel(ds));
if (gui_fullscreen) {
scaling_active = 0;
gui_saved_grab = gui_grab;
--
1.7.3.4
[Qemu-devel] [PATCH 11/15] sdl: Fix cursor handling when switching consoles in absolute mouse mode
From: Jan Kiszka
Restore the cursor when switching from graphic to text console while the
mouse is in absolute mode. Disable it again when returning.
Signed-off-by: Jan Kiszka
---
ui/sdl.c | 13 ++---
1 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/ui/sdl.c b/ui/sdl.c
index 662ffef..5ad38d5 100644
--- a/ui/sdl.c
+++ b/ui/sdl.c
@@ -452,7 +452,7 @@ static void sdl_show_cursor(void)
if (!cursor_hide)
return;
-if (!kbd_mouse_is_absolute()) {
+if (!kbd_mouse_is_absolute() || !is_graphic_console()) {
SDL_ShowCursor(1);
if (guest_cursor &&
(gui_grab || kbd_mouse_is_absolute() || absolute_enabled))
@@ -625,13 +625,20 @@ static void sdl_refresh(DisplayState *ds)
/* Reset the modifiers sent to the current console */
reset_keys();
console_select(keycode - 0x02);
+gui_keysym = 1;
+if (gui_fullscreen) {
+break;
+}
if (!is_graphic_console()) {
/* release grab if going to a text console */
-if (gui_grab && !gui_fullscreen) {
+if (gui_grab) {
sdl_grab_end();
+} else if (absolute_enabled) {
+sdl_show_cursor();
}
+} else if (absolute_enabled) {
+sdl_hide_cursor();
}
-gui_keysym = 1;
break;
default:
break;
--
1.7.3.4
[Qemu-devel] [PATCH 15/15] sdl: Refactor sdl_send_mouse_event
From: Jan Kiszka
Replace width/height globals with the identical values from real_screen,
refactor the function according to our coding style.
Signed-off-by: Jan Kiszka
---
ui/sdl.c | 22 +++---
1 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/ui/sdl.c b/ui/sdl.c
index e5b8a5f..30cde86 100644
--- a/ui/sdl.c
+++ b/ui/sdl.c
@@ -49,7 +49,6 @@ static int gui_key_modifier_pressed;
static int gui_keysym;
static int gui_grab_code = KMOD_LALT | KMOD_LCTRL;
static uint8_t modifiers_state[256];
-static int width, height;
static SDL_Cursor *sdl_cursor_normal;
static SDL_Cursor *sdl_cursor_hidden;
static int absolute_enabled = 0;
@@ -93,7 +92,7 @@ static void sdl_setdata(DisplayState *ds)
ds->surface->pf.bmask,
ds->surface->pf.amask);
}
-static void do_sdl_resize(int new_width, int new_height, int bpp)
+static void do_sdl_resize(int width, int height, int bpp)
{
int flags;
@@ -108,8 +107,6 @@ static void do_sdl_resize(int new_width, int new_height,
int bpp)
if (gui_noframe)
flags |= SDL_NOFRAME;
-width = new_width;
-height = new_height;
real_screen = SDL_SetVideoMode(width, height, bpp, flags);
if (!real_screen) {
fprintf(stderr, "Could not open SDL display (%dx%dx%d): %s\n", width,
@@ -501,18 +498,21 @@ static void sdl_mouse_mode_change(Notifier *notify, void
*data)
static void sdl_send_mouse_event(int dx, int dy, int dz, int x, int y, int
state)
{
-int buttons;
-buttons = 0;
-if (state & SDL_BUTTON(SDL_BUTTON_LEFT))
+int buttons = 0;
+
+if (state & SDL_BUTTON(SDL_BUTTON_LEFT)) {
buttons |= MOUSE_EVENT_LBUTTON;
-if (state & SDL_BUTTON(SDL_BUTTON_RIGHT))
+}
+if (state & SDL_BUTTON(SDL_BUTTON_RIGHT)) {
buttons |= MOUSE_EVENT_RBUTTON;
-if (state & SDL_BUTTON(SDL_BUTTON_MIDDLE))
+}
+if (state & SDL_BUTTON(SDL_BUTTON_MIDDLE)) {
buttons |= MOUSE_EVENT_MBUTTON;
+}
if (kbd_mouse_is_absolute()) {
- dx = x * 0x7FFF / (width - 1);
- dy = y * 0x7FFF / (height - 1);
+dx = x * 0x7FFF / (real_screen->w - 1);
+dy = y * 0x7FFF / (real_screen->h - 1);
} else if (guest_cursor) {
x -= guest_x;
y -= guest_y;
--
1.7.3.4
[Qemu-devel] [PATCH 08/15] sdl: Initialize gui_fullscreen earlier during setup
From: Jan Kiszka
This ensures that we actually enter full screen on startup when e.g.
'-vga none -full-screen' was specified.
Signed-off-by: Jan Kiszka
---
ui/sdl.c |9 +
1 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/ui/sdl.c b/ui/sdl.c
index 80bf776..f19bae2 100644
--- a/ui/sdl.c
+++ b/ui/sdl.c
@@ -891,6 +891,11 @@ void sdl_display_init(DisplayState *ds, int full_screen,
int no_frame)
qemu_free(filename);
}
+if (full_screen) {
+gui_fullscreen = 1;
+sdl_grab_start();
+}
+
dcl = qemu_mallocz(sizeof(DisplayChangeListener));
dcl->dpy_update = sdl_update;
dcl->dpy_resize = sdl_resize;
@@ -920,8 +925,4 @@ void sdl_display_init(DisplayState *ds, int full_screen,
int no_frame)
sdl_cursor_normal = SDL_GetCursor();
atexit(sdl_cleanup);
-if (full_screen) {
-gui_fullscreen = 1;
-sdl_grab_start();
-}
}
--
1.7.3.4
[Qemu-devel] [PATCH 14/15] sdl: Factor out event handlers from sdl_refresh
From: Jan Kiszka
No functional changes.
Signed-off-by: Jan Kiszka
---
ui/sdl.c | 476 +++---
1 files changed, 271 insertions(+), 205 deletions(-)
diff --git a/ui/sdl.c b/ui/sdl.c
index fc63c8e..e5b8a5f 100644
--- a/ui/sdl.c
+++ b/ui/sdl.c
@@ -582,11 +582,274 @@ static void absolute_mouse_grab(void)
}
}
-static void sdl_refresh(DisplayState *ds)
+static void handle_keydown(DisplayState *ds, SDL_Event *ev)
{
-SDL_Event ev1, *ev = &ev1;
int mod_state;
+int keycode;
+
+if (alt_grab) {
+mod_state = (SDL_GetModState() & (gui_grab_code | KMOD_LSHIFT)) ==
+(gui_grab_code | KMOD_LSHIFT);
+} else if (ctrl_grab) {
+mod_state = (SDL_GetModState() & KMOD_RCTRL) == KMOD_RCTRL;
+} else {
+mod_state = (SDL_GetModState() & gui_grab_code) == gui_grab_code;
+}
+gui_key_modifier_pressed = mod_state;
+
+if (gui_key_modifier_pressed) {
+keycode = sdl_keyevent_to_keycode(&ev->key);
+switch (keycode) {
+case 0x21: /* 'f' key on US keyboard */
+toggle_full_screen(ds);
+gui_keysym = 1;
+break;
+case 0x16: /* 'u' key on US keyboard */
+if (scaling_active) {
+scaling_active = 0;
+sdl_resize(ds);
+vga_hw_invalidate();
+vga_hw_update();
+}
+gui_keysym = 1;
+break;
+case 0x02 ... 0x0a: /* '1' to '9' keys */
+/* Reset the modifiers sent to the current console */
+reset_keys();
+console_select(keycode - 0x02);
+gui_keysym = 1;
+if (gui_fullscreen) {
+break;
+}
+if (!is_graphic_console()) {
+/* release grab if going to a text console */
+if (gui_grab) {
+sdl_grab_end();
+} else if (absolute_enabled) {
+sdl_show_cursor();
+}
+} else if (absolute_enabled) {
+sdl_hide_cursor();
+absolute_mouse_grab();
+}
+break;
+case 0x1b: /* '+' */
+case 0x35: /* '-' */
+if (!gui_fullscreen) {
+int width = MAX(real_screen->w + (keycode == 0x1b ? 50 : -50),
+160);
+int height = (ds_get_height(ds) * width) / ds_get_width(ds);
+
+sdl_scale(ds, width, height);
+vga_hw_invalidate();
+vga_hw_update();
+gui_keysym = 1;
+}
+default:
+break;
+}
+} else if (!is_graphic_console()) {
+int keysym = 0;
+
+if (ev->key.keysym.mod & (KMOD_LCTRL | KMOD_RCTRL)) {
+switch (ev->key.keysym.sym) {
+case SDLK_UP:
+keysym = QEMU_KEY_CTRL_UP;
+break;
+case SDLK_DOWN:
+keysym = QEMU_KEY_CTRL_DOWN;
+break;
+case SDLK_LEFT:
+keysym = QEMU_KEY_CTRL_LEFT;
+break;
+case SDLK_RIGHT:
+keysym = QEMU_KEY_CTRL_RIGHT;
+break;
+case SDLK_HOME:
+keysym = QEMU_KEY_CTRL_HOME;
+break;
+case SDLK_END:
+keysym = QEMU_KEY_CTRL_END;
+break;
+case SDLK_PAGEUP:
+keysym = QEMU_KEY_CTRL_PAGEUP;
+break;
+case SDLK_PAGEDOWN:
+keysym = QEMU_KEY_CTRL_PAGEDOWN;
+break;
+default:
+break;
+}
+} else {
+switch (ev->key.keysym.sym) {
+case SDLK_UP:
+keysym = QEMU_KEY_UP;
+break;
+case SDLK_DOWN:
+keysym = QEMU_KEY_DOWN;
+break;
+case SDLK_LEFT:
+keysym = QEMU_KEY_LEFT;
+break;
+case SDLK_RIGHT:
+keysym = QEMU_KEY_RIGHT;
+break;
+case SDLK_HOME:
+keysym = QEMU_KEY_HOME;
+break;
+case SDLK_END:
+keysym = QEMU_KEY_END;
+break;
+case SDLK_PAGEUP:
+keysym = QEMU_KEY_PAGEUP;
+break;
+case SDLK_PAGEDOWN:
+keysym = QEMU_KEY_PAGEDOWN;
+break;
+case SDLK_BACKSPACE:
+keysym = QEMU_KEY_BACKSPACE;
+break;
+case SDLK_DELETE:
+keysym = QEMU_KEY_DELETE;
+break;
+default:
+break;
+}
+}
+if (keysym) {
+kbd_put_keysym(keysym);
+} else if (ev->key.keysym.unicode != 0) {
+kbd_put_keysym(ev-
Re: [Qemu-devel] [PATCH v3 0/6] Implement constant folding and copy propagation in TCG
Thanks, applied all. On Thu, Jul 7, 2011 at 3:37 PM, Kirill Batuzov wrote: > This series implements some basic machine-independent optimizations. They > simplify code and allow liveness analysis do it's work better. > > Suppose we have following ARM code: > > movw r12, #0xb6db > movt r12, #0xdb6d > > In TCG before optimizations we'll have: > > movi_i32 tmp8,$0xb6db > mov_i32 r12,tmp8 > mov_i32 tmp8,r12 > ext16u_i32 tmp8,tmp8 > movi_i32 tmp9,$0xdb6d > or_i32 tmp8,tmp8,tmp9 > mov_i32 r12,tmp8 > > And after optimizations we'll have this: > > movi_i32 r12,$0xdb6db6db > > Here are performance evaluation results on SPEC CPU2000 integer tests in > user-mode emulation on x86_64 host. There were 5 runs of each test on > reference data set. The tables below show runtime in seconds for all these > runs. > > ARM guest without optimizations: > Test name #1 #2 #3 #4 #5 Median > 164.gzip 1408.891 1402.323 1407.623 1404.955 1405.396 1405.396 > 175.vpr 1245.31 1248.758 1247.936 1248.534 1247.534 1247.936 > 176.gcc 912.561 809.481 847.057 912.636 912.544 912.544 > 181.mcf 198.384 197.841 199.127 197.976 197.29 197.976 > 186.crafty 1545.881 1546.051 1546.002 1545.927 1545.945 1545.945 > 197.parser 3779.954 3779.878 3779.79 3779.94 3779.88 3779.88 > 252.eon 2563.168 2776.152 2776.395 2776.577 2776.202 2776.202 > 253.perlbmk 2591.781 2504.078 2507.07 2591.337 2463.401 2507.07 > 256.bzip2 1306.197 1304.639 1184.853 1305.141 1305.606 1305.141 > 300.twolf 2918.984 2918.926 2918.93 2918.97 2918.914 2918.93 > > ARM guest with optimizations: > Test name #1 #2 #3 #4 #5 Median Gain > 164.gzip 1401.198 1376.337 1401.117 1401.23 1401.246 1401.198 0.30% > 175.vpr 1247.964 1151.468 1247.76 1154.419 1242.017 1242.017 0.47% > 176.gcc 896.882 918.546 918.297 851.465 918.39 918.297 -0.63% > 181.mcf 198.19 197.399 198.421 198.663 198.312 198.312 -0.17% > 186.crafty 1520.425 1520.362 1520.477 1520.445 1520.957 1520.445 1.65% > 197.parser 3770.943 3770.927 3770.578 3771.048 3770.904 3770.927 0.24% > 252.eon 2752.371 2752.111 2752.005 2752.214 2752.109 2752.111 0.87% > 253.perlbmk 2577.462 2578.588 2493.567 2578.571 2578.318 2578.318 -2.84% > 256.bzip2 1296.198 1271.128 1296.044 1296.321 1296.147 1296.147 0.69% > 300.twolf 2888.984 2889.023 2889.225 2889.039 2889.05 2889.039 1.02% > > > x86_64 guest without optimizations: > Test name #1 #2 #3 #4 #5 Median > 164.gzip 857.654 857.646 857.678 798.119 857.675 857.654 > 175.vpr 959.265 959.207 959.185 959.461 959.332 959.265 > 176.gcc 625.722 637.257 646.638 646.614 646.56 646.56 > 181.mcf 221.666 220.194 220.079 219.868 221.5 220.194 > 186.crafty 1129.531 1129.739 1129.573 1129.588 1129.624 1129.588 > 197.parser 1809.517 1809.516 1809.386 1809.477 1809.427 1809.477 > 253.perlbmk 1774.944 1776.046 1769.865 1774.052 1775.236 1774.944 > 254.gap 1061.033 1061.158 1061.064 1061.047 1061.01 1061.047 > 255.vortex 1871.261 1914.144 1914.057 1914.086 1914.127 1914.086 > 256.bzip2 918.916 1011.828 1011.819 1012.11 1011.932 1011.828 > 300.twolf 1332.797 1330.56 1330.687 1330.917 1330.602 1330.687 > > x86_64 guest with optimizations: > Test name #1 #2 #3 #4 #5 Median Gain > 164.gzip 806.198 854.159 854.184 854.168 854.187 854.168 0.41% > 175.vpr 955.905 950.86 955.876 876.397 955.957 955.876 1.82% > 176.gcc 641.663 640.189 641.57 641.552 641.514 641.552 0.03% > 181.mcf 217.619 218.627 218.699 217.977 216.955 217.977 1.18% > 186.crafty 1123.909 1123.852 1123.917 1123.781 1123.805 1123.852 0.51% > 197.parser 1813.94 1814.643 1815.286 1814.445 1813.72 1814.445 -0.27% > 253.perlbmk 1791.536 1795.642 1793.0 1797.486 1791.401 1793.0 -1.02% > 254.gap 1070.605 1070.216 1070.637 1070.168 1070.491 1070.491 -0.89% > 255.vortex 1918.764 1918.573 1917.411 1918.287 1918.735 1918.573 -0.23% > 256.bzip2 1017.179 1017.083 1017.283 1016.913 1017.189 1017.179 -0.53% > 300.twolf 1321.072 1321.109 1321.019 1321.072 1321.004 1321.072 0.72% > > ARM guests for 254.gap and 255.vortex and x86_64 guest for 252.eon does not > work under QEMU for some unrelated reason. > > Changes: > v1 -> v2 > - State and Vals arrays merged to an array of structures. > - Added reference counting of temp's copies. This helps to reset temp's state > faster in most cases. > - Do not make copy propagation through operations with TCG_OPF_CALL_CLOBBER > or > TCG_OPF_SIDE_EFFECTS flag. > - Split some expression simplifications into independent switch. > - Let compiler handle signed shifts and sign/zero extends in it's > implementation defined way. > > v2 -> v3 > - Elements of equiv class are placed in a double-linked circular list so it's > easier to choose
Re: [Qemu-devel] [PATCH 5/7] target-alpha: Implement WAIT IPR.
On Sat, Jul 23, 2011 at 10:17 PM, Richard Henderson wrote:
> ---
Misses SoB and I don't know how to add it while pulling. Please fix or educate.
> target-alpha/translate.c | 31 +--
> 1 files changed, 21 insertions(+), 10 deletions(-)
>
> diff --git a/target-alpha/translate.c b/target-alpha/translate.c
> index c61906a..fddaff8 100644
> --- a/target-alpha/translate.c
> +++ b/target-alpha/translate.c
> @@ -1617,9 +1617,10 @@ static void gen_mfpr(int ra, int regno)
> }
> }
>
> -static void gen_mtpr(int rb, int regno)
> +static ExitStatus gen_mtpr(DisasContext *ctx, int rb, int regno)
> {
> TCGv tmp;
> + int data;
>
> if (rb == 31) {
> tmp = tcg_const_i64(0);
> @@ -1627,19 +1628,27 @@ static void gen_mtpr(int rb, int regno)
> tmp = cpu_ir[rb];
> }
>
> - /* These two register numbers perform a TLB cache flush. Thankfully we
> - can only do this inside PALmode, which means that the current basic
> - block cannot be affected by the change in mappings. */
> - if (regno == 255) {
> + switch (regno) {
> + case 255:
> /* TBIA */
> gen_helper_tbia();
> - } else if (regno == 254) {
> + break;
> +
> + case 254:
> /* TBIS */
> gen_helper_tbis(tmp);
> - } else {
> + break;
> +
> + case 253:
> + /* WAIT */
> + tmp = tcg_const_i64(1);
> + tcg_gen_st32_i64(tmp, cpu_env, offsetof(CPUState, halted));
> + return gen_excp(ctx, EXCP_HLT, 0);
> +
> + default:
> /* The basic registers are data only, and unknown registers
> are read-zero, write-ignore. */
> - int data = cpu_pr_data(regno);
> + data = cpu_pr_data(regno);
> if (data != 0) {
> if (data & PR_BYTE) {
> tcg_gen_st8_i64(tmp, cpu_env, data & ~PR_BYTE);
> @@ -1649,11 +1658,14 @@ static void gen_mtpr(int rb, int regno)
> tcg_gen_st_i64(tmp, cpu_env, data);
> }
> }
> + break;
> }
>
> if (rb == 31) {
> tcg_temp_free(tmp);
> }
> +
> + return NO_EXIT;
> }
> #endif /* !USER_ONLY*/
>
> @@ -3053,8 +3065,7 @@ static ExitStatus translate_one(DisasContext *ctx,
> uint32_t insn)
> /* HW_MTPR (PALcode) */
> #ifndef CONFIG_USER_ONLY
> if (ctx->tb->flags & TB_FLAGS_PAL_MODE) {
> - gen_mtpr(rb, insn & 0x);
> - break;
> + return gen_mtpr(ctx, rb, insn & 0x);
> }
> #endif
> goto invalid_opc;
> --
> 1.7.4.4
>
>
>
[Qemu-devel] [PATCH][STABLE] user: Restore debug usage message for '-d ?' in user mode emulation
The code which prints the debug usage message on '-d ?' for *-user
has to come before the check for "not enough arguments", so that
"qemu-foo -d ?" prints the list of possible debug log items rather than
the generic usage message. (This was inadvertently broken in commit
c235d73.)
Signed-off-by: Peter Maydell
---
This is just a retransmit of this patch (applied in master as 4b5dfd824)
with the appropriate magic markers to say "please apply to 0.15 as well",
since it's a regression from 0.14.
bsd-user/main.c|8 +---
darwin-user/main.c |8 +---
linux-user/main.c | 11 ++-
3 files changed, 16 insertions(+), 11 deletions(-)
diff --git a/bsd-user/main.c b/bsd-user/main.c
index 6018a41..a63b877 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -856,9 +856,6 @@ int main(int argc, char **argv)
usage();
}
}
-if (optind >= argc)
-usage();
-filename = argv[optind];
/* init debug */
cpu_set_log_filename(log_file);
@@ -877,6 +874,11 @@ int main(int argc, char **argv)
cpu_set_log(mask);
}
+if (optind >= argc) {
+usage();
+}
+filename = argv[optind];
+
/* Zero out regs */
memset(regs, 0, sizeof(struct target_pt_regs));
diff --git a/darwin-user/main.c b/darwin-user/main.c
index 35196a1..72307ad 100644
--- a/darwin-user/main.c
+++ b/darwin-user/main.c
@@ -809,9 +809,6 @@ int main(int argc, char **argv)
usage();
}
}
-if (optind >= argc)
-usage();
-filename = argv[optind];
/* init debug */
cpu_set_log_filename(log_file);
@@ -830,6 +827,11 @@ int main(int argc, char **argv)
cpu_set_log(mask);
}
+if (optind >= argc) {
+usage();
+}
+filename = argv[optind];
+
/* Zero out regs */
memset(regs, 0, sizeof(struct target_pt_regs));
diff --git a/linux-user/main.c b/linux-user/main.c
index 2135b9c..6a8f4bd 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -3048,11 +3048,6 @@ int main(int argc, char **argv, char **envp)
usage();
}
}
-if (optind >= argc)
-usage();
-filename = argv[optind];
-exec_path = argv[optind];
-
/* init debug */
cpu_set_log_filename(log_file);
if (log_mask) {
@@ -3070,6 +3065,12 @@ int main(int argc, char **argv, char **envp)
cpu_set_log(mask);
}
+if (optind >= argc) {
+usage();
+}
+filename = argv[optind];
+exec_path = argv[optind];
+
/* Zero out regs */
memset(regs, 0, sizeof(struct target_pt_regs));
--
1.7.4.1
Re: [Qemu-devel] [PATCH v3 5/6] Do constant folding for shift operations.
On Thu, Jul 7, 2011 at 3:37 PM, Kirill Batuzov wrote:
> Perform constant forlding for SHR, SHL, SAR, ROTR, ROTL operations.
This patch broke build on targets (Sparc, MIPS) which don't implement
rotation ops, the next patch likewise. I committed a fix.
> Signed-off-by: Kirill Batuzov
> ---
> tcg/optimize.c | 72
>
> 1 files changed, 72 insertions(+), 0 deletions(-)
>
> diff --git a/tcg/optimize.c b/tcg/optimize.c
> index c469952..a1bb287 100644
> --- a/tcg/optimize.c
> +++ b/tcg/optimize.c
> @@ -102,6 +102,11 @@ static int op_bits(int op)
> case INDEX_op_and_i32:
> case INDEX_op_or_i32:
> case INDEX_op_xor_i32:
> + case INDEX_op_shl_i32:
> + case INDEX_op_shr_i32:
> + case INDEX_op_sar_i32:
> + case INDEX_op_rotl_i32:
> + case INDEX_op_rotr_i32:
> return 32;
> #if TCG_TARGET_REG_BITS == 64
> case INDEX_op_mov_i64:
> @@ -111,6 +116,11 @@ static int op_bits(int op)
> case INDEX_op_and_i64:
> case INDEX_op_or_i64:
> case INDEX_op_xor_i64:
> + case INDEX_op_shl_i64:
> + case INDEX_op_shr_i64:
> + case INDEX_op_sar_i64:
> + case INDEX_op_rotl_i64:
> + case INDEX_op_rotr_i64:
> return 64;
> #endif
> default:
> @@ -205,6 +215,58 @@ static TCGArg do_constant_folding_2(int op, TCGArg x,
> TCGArg y)
> CASE_OP_32_64(xor):
> return x ^ y;
>
> + case INDEX_op_shl_i32:
> + return (uint32_t)x << (uint32_t)y;
> +
> +#if TCG_TARGET_REG_BITS == 64
> + case INDEX_op_shl_i64:
> + return (uint64_t)x << (uint64_t)y;
> +#endif
> +
> + case INDEX_op_shr_i32:
> + return (uint32_t)x >> (uint32_t)y;
> +
> +#if TCG_TARGET_REG_BITS == 64
> + case INDEX_op_shr_i64:
> + return (uint64_t)x >> (uint64_t)y;
> +#endif
> +
> + case INDEX_op_sar_i32:
> + return (int32_t)x >> (int32_t)y;
> +
> +#if TCG_TARGET_REG_BITS == 64
> + case INDEX_op_sar_i64:
> + return (int64_t)x >> (int64_t)y;
> +#endif
> +
> + case INDEX_op_rotr_i32:
> +#if TCG_TARGET_REG_BITS == 64
> + x &= 0x;
> + y &= 0x;
> +#endif
> + x = (x << (32 - y)) | (x >> y);
> + return x;
> +
> +#if TCG_TARGET_REG_BITS == 64
> + case INDEX_op_rotr_i64:
> + x = (x << (64 - y)) | (x >> y);
> + return x;
> +#endif
> +
> + case INDEX_op_rotl_i32:
> +#if TCG_TARGET_REG_BITS == 64
> + x &= 0x;
> + y &= 0x;
> +#endif
> + x = (x << y) | (x >> (32 - y));
> + return x;
> +
> +#if TCG_TARGET_REG_BITS == 64
> + case INDEX_op_rotl_i64:
> + x = (x << y) | (x >> (64 - y));
> + return x;
> +#endif
> +
> default:
> fprintf(stderr,
> "Unrecognized operation %d in do_constant_folding.\n", op);
> @@ -278,6 +340,11 @@ static TCGArg *tcg_constant_folding(TCGContext *s,
> uint16_t *tcg_opc_ptr,
> switch (op) {
> CASE_OP_32_64(add):
> CASE_OP_32_64(sub):
> + CASE_OP_32_64(shl):
> + CASE_OP_32_64(shr):
> + CASE_OP_32_64(sar):
> + CASE_OP_32_64(rotl):
> + CASE_OP_32_64(rotr):
> if (temps[args[1]].state == TCG_TEMP_CONST) {
> /* Proceed with possible constant folding. */
> break;
> @@ -363,6 +430,11 @@ static TCGArg *tcg_constant_folding(TCGContext *s,
> uint16_t *tcg_opc_ptr,
> CASE_OP_32_64(or):
> CASE_OP_32_64(and):
> CASE_OP_32_64(xor):
> + CASE_OP_32_64(shl):
> + CASE_OP_32_64(shr):
> + CASE_OP_32_64(sar):
> + CASE_OP_32_64(rotl):
> + CASE_OP_32_64(rotr):
> if (temps[args[1]].state == TCG_TEMP_CONST
> && temps[args[2]].state == TCG_TEMP_CONST) {
> gen_opc_buf[op_index] = op_to_movi(op);
> --
> 1.7.4.1
>
>
>
[Qemu-devel] Bug#636035: qemu-user: init_paths() doesnt guard against recursive link cycles
Package: qemu-user Version: 0.14.1+dfsg-3 Severity: normal Hi, when a symbolic or hardlink which creates a recursive cycle exists in the directory specified as the elf interpreter prefix (default=/etc/qemu-binfmt//), qemu-user will recursively traverse this path and never finish (eating tons of memory in the process). The easiest way to reproduce is to do: ln -s '.' /etc/qemu-binfmt//foobar (or wherever directory you pointed qemu to with the -L option) This is mostly annoying because there exists the /usr/bin/X11 symlink which points to '.' in debian systems with x11-common installed. Other problems occur with /proc/self/fd/ (and /dev/fd/ which links to it) when /proc is mounted in the elf interpreter prefix path. The problem occurs when init_paths() is called on interp_prefix (the elf interpreter prefix path). init_paths() doesnt protect from infinitely traversing cyclic symlinks. find(1) solves this problem by maintaining a hashtable of (to be) traversed directories and skipping directories that are found to be already part of it. I can prepare a patch for path.c (which defines init_paths()) which guards against this behaviour in the same way that find(1) does if this problem is desired to be solved that way. cheers, josch
Re: [Qemu-devel] [PATCH] fix disabling interrupts in sun4u
On Sat, Jul 30, 2011 at 11:09 AM, Blue Swirl wrote:
> On Mon, Jul 25, 2011 at 8:22 PM, Artyom Tarasenko wrote:
>> clear interrupt request if the interrupt priority < CPU pil
>> clear hardware interrupt request if interrupts are disabled
>>
>> Signed-off-by: Artyom Tarasenko
>> ---
>> hw/sun4u.c | 6 --
>> 1 files changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/sun4u.c b/hw/sun4u.c
>> index d7dcaf0..7f95aeb 100644
>> --- a/hw/sun4u.c
>> +++ b/hw/sun4u.c
>> @@ -255,7 +255,7 @@ void cpu_check_irqs(CPUState *env)
>> pil |= 1 << 14;
>> }
>>
>> - if (!pil) {
>> + if (pil < (2 << env->psrpil)){
>
> Sorry, I don't understand the patch. Where is this '2' coming from?
We shouldn't interrupt at levels <= psrpil. The bit corresponding to
psrpil is (1<< psrpil),
the next bit is (2 << psrpil).
>
>> if (env->interrupt_request & CPU_INTERRUPT_HARD) {
>> CPUIRQ_DPRINTF("Reset CPU IRQ (current interrupt %x)\n",
>> env->interrupt_index);
>> @@ -287,10 +287,12 @@ void cpu_check_irqs(CPUState *env)
>> break;
>> }
>> }
>> - } else {
>> + } else if (env->interrupt_request & CPU_INTERRUPT_HARD) {
>> CPUIRQ_DPRINTF("Interrupts disabled, pil=%08x pil_in=%08x
>> softint=%08x "
>> "current interrupt %x\n",
>> pil, env->pil_in, env->softint, env->interrupt_index);
>> + env->interrupt_index = 0;
>> + cpu_reset_interrupt(env, CPU_INTERRUPT_HARD);
>
> Why reset the index? The idea is that the interrupt is left pending a
> change to PIL etc.
But it is kept in env->pil_in and env->softint . Am I missing some
scenario where it's not enough?
>> }
>> }
>>
>> --
>> 1.7.3.4
>>
>>
>
--
Regards,
Artyom Tarasenko
solaris/sparc under qemu blog: http://tyom.blogspot.com/
Re: [Qemu-devel] [PATCH 02/15] hw/omap_gpio.c: Don't complain about some writes to r/o registers
On 30 July 2011 05:53, andrzej zaborowski wrote:
> I went ahead and pushed the series
Thanks.
> with the exception of this patch
> and 14/15 because I think these are the types of patches that should
> remain in downstream as a reminder, is there an argument for not
> fixing these things in Linux?
I think the only arguments are "it's too hard" and "I'd like to
run legacy images without having qemu constantly complaining and
making '-serial stdio' unusable" :-) [have you seen how much chatter
you get out of n810 running a meego image?] I think I've said before
that really we ought to have this kind of message more consistently
and conveniently enablable/disablable.
I'll leave this sort of change out of future patches; when I get
down to the point where they're the only thing left in my patch
stack (a) I'll be very happy and (b) I might look at a nicer way
to handle them.
> In patch 04 I renamed omap2_gpio_module_set to omap2_gpio_set because
> the parameter is no longer the module pointer. By the way I think we
> should also pass the target agent pointer on creation the same way
> clocks are passed and use omap_l4_attach.
The clock passing thing is (as per comments) a bit nasty; at
the moment sysbus is the closest we have to a nice way of exposing
"I have some MMIO regions" so I'm a bit reluctant to hide it behind
another magic pointer property. I'll have a think about this, though.
> In patch 07 I bumped the vmstate version because the structure seems
> to have changed.
Oops, yes. I had a note at some point to look more carefully at
the vmstate change but I must have forgotten about it :-(
I agree we can just bump the version.
> In patch 12 I removed the
> else {
> s->bdrv_cur = s->bdrv;
> } part because there seemed to be no reason to add it, please check
> that I haven't broken something.
That looks like it should be OK, we'll always call onenand_reset()
which will set bdrv_cur to bdrv.
-- PMM
Re: [Qemu-devel] [PATCH 01/15] sdl: Fix termination in -no-shutdown mode
On 07/30/2011 04:39 AM, Jan Kiszka wrote:
From: Jan Kiszka
Just like the monitor does, we need to clear no_shutdown before calling
qemu_system_shutdown_request on quit requests. Otherwise, QEMU just
stops the VM.
Signed-off-by: Jan Kiszka
Reviewed-by: Anthony Liguori
Regards,
Anthony Liguori
---
ui/sdl.c |4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/ui/sdl.c b/ui/sdl.c
index 6dbc5cb..9efcda5 100644
--- a/ui/sdl.c
+++ b/ui/sdl.c
@@ -672,8 +672,10 @@ static void sdl_refresh(DisplayState *ds)
sdl_process_key(&ev->key);
break;
case SDL_QUIT:
-if (!no_quit)
+if (!no_quit) {
+no_shutdown = 0;
qemu_system_shutdown_request();
+}
break;
case SDL_MOUSEMOTION:
if (gui_grab || kbd_mouse_is_absolute() ||
[Qemu-devel] qemu-kvm idle guest using 10% cpu and causes significant increase of read_hpet calls on host
Hi, I've started running a Centos6 minimal install virtual machine on a Centost6 host and noticed that even when the guest is completely idle the qemu-kvm process on the host shows up with 10% cpu usage on in "top". Using perf and oprofile I noticed that once the guest is running the amount of read_hpet calls increases by a factor of 10. So I went and booted the host with "hpet=disable" which resulted in the qemu-kvm process now showing up with 15% cpu usage instead of 10%. So this seems to have an impact but certainly not the intended one. I'm not well versed in these matters so I'm looking for any pointers in terms of for example kernel options I could test on the guest/host to find out what the cause of this is. I opened a bug here: http://bugs.centos.org/view.php?id=5028 According to one developer this is also seen by users on RHEL6 and SL6 which is why I decided to ask for help here since it doesn't seem to be a distribution specific problem. Regards, Dennis
Re: [Qemu-devel] [PATCH] fix disabling interrupts in sun4u
On Sat, Jul 30, 2011 at 3:31 PM, Artyom Tarasenko wrote:
> On Sat, Jul 30, 2011 at 11:09 AM, Blue Swirl wrote:
>> On Mon, Jul 25, 2011 at 8:22 PM, Artyom Tarasenko
>> wrote:
>>> clear interrupt request if the interrupt priority < CPU pil
>>> clear hardware interrupt request if interrupts are disabled
>>>
>>> Signed-off-by: Artyom Tarasenko
>>> ---
>>> hw/sun4u.c | 6 --
>>> 1 files changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/hw/sun4u.c b/hw/sun4u.c
>>> index d7dcaf0..7f95aeb 100644
>>> --- a/hw/sun4u.c
>>> +++ b/hw/sun4u.c
>>> @@ -255,7 +255,7 @@ void cpu_check_irqs(CPUState *env)
>>> pil |= 1 << 14;
>>> }
>>>
>>> - if (!pil) {
>>> + if (pil < (2 << env->psrpil)){
>>
>> Sorry, I don't understand the patch. Where is this '2' coming from?
>
> We shouldn't interrupt at levels <= psrpil. The bit corresponding to
> psrpil is (1<< psrpil),
> the next bit is (2 << psrpil).
Now I see. The check below "i > env->psrpil" does something similar
but it doesn't reset interrupt.
How about pil < (1 << (env->psrpil + 1))? I think that makes the
purpose clearer.
>>
>>> if (env->interrupt_request & CPU_INTERRUPT_HARD) {
>>> CPUIRQ_DPRINTF("Reset CPU IRQ (current interrupt %x)\n",
>>> env->interrupt_index);
>>> @@ -287,10 +287,12 @@ void cpu_check_irqs(CPUState *env)
>>> break;
>>> }
>>> }
>>> - } else {
>>> + } else if (env->interrupt_request & CPU_INTERRUPT_HARD) {
>>> CPUIRQ_DPRINTF("Interrupts disabled, pil=%08x pil_in=%08x
>>> softint=%08x "
>>> "current interrupt %x\n",
>>> pil, env->pil_in, env->softint,
>>> env->interrupt_index);
>>> + env->interrupt_index = 0;
>>> + cpu_reset_interrupt(env, CPU_INTERRUPT_HARD);
>>
>> Why reset the index? The idea is that the interrupt is left pending a
>> change to PIL etc.
>
> But it is kept in env->pil_in and env->softint . Am I missing some
> scenario where it's not enough?
cpu-exec.c:378 checks interrupt_index, not pil_in etc.
Re: [Qemu-devel] Bug#632192: [PATCH] add QEMU_LD_PREFIX environment variable
On Fri, Jul 29, 2011 at 05:21:59PM +0200, Johannes Schauer wrote: > So if we agree on using environment variables to pass options to > qemu-user we next need to agree on how to name the options. > The following commandline arguments exist (in order as they are checked > in linux-user/main.c) and I shortly described and proposed a name for > the environment variable in the same line. > -d (activate log) - QEMU_LOG > -D (logfile) - QEMU_LOGFILE > -E (set target env variabe) - QEMU_SET_ENV > -U (unset target env variabe) - QEMU_UNSET_ENV > -0 (set target argv[0]) - QEMU_ARGV0 > -s (stack size) - QEMU_STACK_SIZE > -L (elf interpreter prefix) - QEMU_LD_PREFIX > -p (page size)- QEMU_PAGESIZE > -g (listen for gdb on port) - QEMU_GDB > -r (uname)- QEMU_UNAME > -cpu (cpu model) - QEMU_CPU > -B (guest base) - QEMU_GUEST_BASE > -R (reserved virtual address) - QEMU_RESERVED_VA > -drop-ld-preload - QEMU_DROP_LD_PRELOAD This is a legacy option that could be removed already. The -U LD_PRELOAD replaces this option. The only known user of this option (scratchbox) has migrated -U LD_PRELOAD years ago. > -singlestep - QEMU_SINGLESTEP > -strace - QEMU_STRACE > also, there already is the QEMU_STRACE environment variable which could > be incorporated into the solution? Else names look good to me. > the -E and -U options can be specified several times so the environment > variables should be able to receive a list - maybe in the getsubopt(3) > style? getsubopt would mean that passing enviroment variable contents with commas wouldn't work. Perhaps that would still be an acceptable limitation. Riku
[Qemu-devel] Is there any qemu imag
Hi, Is there any qemu image (small OS which support , gcc, ssh, and networking ) , like linux-0.2.img.bz2 . I want this for KVM hyeprvisor ..
Re: [Qemu-devel] [PATCH] v6 revamp acpitable parsing and allow to specify complete (headerful) table
Thanks.
Somehow completely missed Blue's response back on the 15th. Glad to see this
in.
When using this for SLIC I had to patch the OEM ID fields in the other tables
to match at runtime in order to make windows licensing happy. But thats a BIOS
change, not something in QEMU
On Jul 30, 2011, at 2:41 AM, "Blue Swirl" wrote:
> Thanks, applied.
>
> On Fri, Jul 29, 2011 at 4:49 AM, Isaku Yamahata
> wrote:
>> On Fri, Jul 15, 2011 at 07:51:43PM +0300, Blue Swirl wrote:
>>> On Fri, Jul 15, 2011 at 6:18 PM, John Baboval
>>> wrote:
Is there something I can do to help take this patch the rest of the way?
I'd hate to see it die because of a style issue and a compiler warning.
>>>
>>> There's also suspicious missing 'break' statement. How about fixing
>>> the issues and submitting the patch?
>>
>> I fixed the compile error.
>> I think the missing break is intentional, so added an comment there. Michael?
>> Blue, can you please take a look of it?
>>
>>
>> From 9a5e4158074ea251ab064a946927bcaf861f5c1e Mon Sep 17 00:00:00 2001
>> Message-Id:
>> <9a5e4158074ea251ab064a946927bcaf861f5c1e.1311903724.git.yamah...@valinux.co.jp>
>> From: Michael Tokarev
>> Date: Thu, 12 May 2011 18:44:17 +0400
>> Subject: [PATCH] revamp acpitable parsing and allow to specify complete
>> (headerful) table
>>
>> From Michael Tokarev
>>
>> This patch almost rewrites acpi_table_add() function
>> (but still leaves it using old get_param_value() interface).
>> The result is that it's now possible to specify whole table
>> (together with a header) in an external file, instead of just
>> data portion, with a new file= parameter, but at the same time
>> it's still possible to specify header fields as before.
>>
>> Now with the checkpatch.pl formatting fixes, thanks to
>> Stefan Hajnoczi for suggestions, with changes from
>> Isaku Yamahata, and with my further refinements.
>>
>> Signed-off-by: Michael Tokarev
>> Cc:: Isaku Yamahata
>> Cc: John Baboval
>> Cc: Blue Swirl
>>
>> ---
>> v5: rediffed against current qemu/master.
>> v6: fix one "} else {" coding style defect (noted by Blue Swirl)
>> v7: style fix and added an comment for suspicious break
>>I think that the missing break of case 0 is intentional.
>>I added the fallthrough comment there.
>> ---
>> hw/acpi.c | 298
>> ---
>> qemu-options.hx |7 +-
>> 2 files changed, 178 insertions(+), 127 deletions(-)
>>
>> diff --git a/hw/acpi.c b/hw/acpi.c
>> index ad40fb4..79ec66c 100644
>> --- a/hw/acpi.c
>> +++ b/hw/acpi.c
>> @@ -20,19 +20,30 @@
>> #include "pc.h"
>> #include "acpi.h"
>>
>> -struct acpi_table_header
>> -{
>> -char signature [4];/* ACPI signature (4 ASCII characters) */
>> +struct acpi_table_header {
>> +uint16_t _length; /* our length, not actual part of the hdr */
>> + /* XXX why we have 2 length fields here? */
>> +char sig[4]; /* ACPI signature (4 ASCII characters) */
>> uint32_t length; /* Length of table, in bytes, including header
>> */
>> uint8_t revision; /* ACPI Specification minor version # */
>> uint8_t checksum; /* To make sum of entire table == 0 */
>> -char oem_id [6]; /* OEM identification */
>> -char oem_table_id [8]; /* OEM table identification */
>> +char oem_id[6]; /* OEM identification */
>> +char oem_table_id[8]; /* OEM table identification */
>> uint32_t oem_revision;/* OEM revision number */
>> -char asl_compiler_id [4]; /* ASL compiler vendor ID */
>> +char asl_compiler_id[4]; /* ASL compiler vendor ID */
>> uint32_t asl_compiler_revision; /* ASL compiler revision number */
>> } __attribute__((packed));
>>
>> +#define ACPI_TABLE_HDR_SIZE sizeof(struct acpi_table_header)
>> +#define ACPI_TABLE_PFX_SIZE sizeof(uint16_t) /* size of the extra prefix */
>> +
>> +static const char dfl_hdr[ACPI_TABLE_HDR_SIZE] =
>> +"\0\0" /* fake _length (2) */
>> +"QEMU\0\0\0\0\1\0" /* sig (4), len(4), revno (1), csum (1) */
>> +"QEMUQEQEMUQEMU\1\0\0\0" /* OEM id (6), table (8), revno (4) */
>> +"QEMU\1\0\0\0" /* ASL compiler ID (4), version (4) */
>> +;
>> +
>> char *acpi_tables;
>> size_t acpi_tables_len;
>>
>> @@ -40,163 +51,198 @@ static int acpi_checksum(const uint8_t *data, int len)
>> {
>> int sum, i;
>> sum = 0;
>> -for(i = 0; i < len; i++)
>> +for (i = 0; i < len; i++) {
>> sum += data[i];
>> +}
>> return (-sum) & 0xff;
>> }
>>
>> +/* like strncpy() but zero-fills the tail of destination */
>> +static void strzcpy(char *dst, const char *src, size_t size)
>> +{
>> +size_t len = strlen(src);
>> +if (len >= size) {
>> +len = size;
>> +} else {
>> + memset(dst + len, 0, size - len);
>> +}
>> +memcpy(dst, src, len);
>> +}
>> +
>> +/* XXX fixme: this function uses obsolete ar
Re: [Qemu-devel] kvm PCI assignment & VFIO ramblings
On Sat, 2011-07-30 at 09:58 +1000, Benjamin Herrenschmidt wrote: > Hi folks ! > > So I promised Anthony I would try to summarize some of the comments & > issues we have vs. VFIO after we've tried to use it for PCI pass-through > on POWER. It's pretty long, there are various items with more or less > impact, some of it is easily fixable, some are API issues, and we'll > probably want to discuss them separately, but for now here's a brain > dump. Thanks Ben. For those wondering what happened to VFIO and where it lives now, Tom Lyon turned it over to me. I've been continuing to hack and bug fix and prep it for upstream. My trees are here: git://github.com/awilliam/linux-vfio.git vfio git://github.com/awilliam/qemu-vfio.git vfio I was hoping we were close to being ready for an upstream push, but we obviously need to work through the issues Ben and company have been hitting. > David, Alexei, please make sure I haven't missed anything :-) > > * Granularity of pass-through > > So let's first start with what is probably the main issue and the most > contentious, which is the problem of dealing with the various > constraints which define the granularity of pass-through, along with > exploiting features like the VTd iommu domains. > > For the sake of clarity, let me first talk a bit about the "granularity" > issue I've mentioned above. > > There are various constraints that can/will force several devices to be > "owned" by the same guest and on the same side of the host/guest > boundary. This is generally because some kind of HW resource is shared > and thus not doing so would break the isolation barrier and enable a > guest to disrupt the operations of the host and/or another guest. > > Some of those constraints are well know, such as shared interrupts. Some > are more subtle, for example, if a PCIe->PCI bridge exist in the system, > there is no way for the iommu to identify transactions from devices > coming from the PCI segment of that bridge with a granularity other than > "behind the bridge". So typically a EHCI/OHCI/OHCI combo (a classic) > behind such a bridge must be treated as a single "entity" for > pass-trough purposes. On x86, the USB controllers don't typically live behind a PCIe-to-PCI bridge, so don't suffer the source identifier problem, but they do often share an interrupt. But even then, we can count on most modern devices supporting PCI2.3, and thus the DisINTx feature, which allows us to share interrupts. In any case, yes, it's more rare but we need to know how to handle devices behind PCI bridges. However I disagree that we need to assign all the devices behind such a bridge to the guest. There's a difference between removing the device from the host and exposing the device to the guest. If I have a NIC and HBA behind a bridge, it's perfectly reasonable that I might only assign the NIC to the guest, but as you describe, we then need to prevent the host, or any other guest from making use of the HBA. > In IBM POWER land, we call this a "partitionable endpoint" (the term > "endpoint" here is historic, such a PE can be made of several PCIe > "endpoints"). I think "partitionable" is a pretty good name tho to > represent the constraints, so I'll call this a "partitionable group" > from now on. > > Other examples of such HW imposed constraints can be a shared iommu with > no filtering capability (some older POWER hardware which we might want > to support fall into that category, each PCI host bridge is its own > domain but doesn't have a finer granularity... however those machines > tend to have a lot of host bridges :) > > If we are ever going to consider applying some of this to non-PCI > devices (see the ongoing discussions here), then we will be faced with > the crazyness of embedded designers which probably means all sort of new > constraints we can't even begin to think about > > This leads me to those initial conclusions: > > - The -minimum- granularity of pass-through is not always a single > device and not always under SW control But IMHO, we need to preserve the granularity of exposing a device to a guest as a single device. That might mean some devices are held hostage by an agent on the host. > - Having a magic heuristic in libvirt to figure out those constraints is > WRONG. This reeks of XFree 4 PCI layer trying to duplicate the kernel > knowledge of PCI resource management and getting it wrong in many many > cases, something that took years to fix essentially by ripping it all > out. This is kernel knowledge and thus we need the kernel to expose in a > way or another what those constraints are, what those "partitionable > groups" are. > > - That does -not- mean that we cannot specify for each individual device > within such a group where we want to put it in qemu (what devfn etc...). > As long as there is a clear understanding that the "ownership" of the > device goes with the group, this is somewhat orthogonal to how they are > represented in qemu. (Not compl
Re: [Qemu-devel] [PATCH v3 5/6] Do constant folding for shift operations.
On Sat, Jul 30, 2011 at 3:25 PM, Blue Swirl wrote:
> On Thu, Jul 7, 2011 at 3:37 PM, Kirill Batuzov wrote:
>> Perform constant forlding for SHR, SHL, SAR, ROTR, ROTL operations.
>
> This patch broke build on targets (Sparc, MIPS) which don't implement
> rotation ops, the next patch likewise. I committed a fix.
Unfortunately my patch which fixed Sparc build broke i386 build, so I
committed another fix.
>> Signed-off-by: Kirill Batuzov
>> ---
>> tcg/optimize.c | 72
>>
>> 1 files changed, 72 insertions(+), 0 deletions(-)
>>
>> diff --git a/tcg/optimize.c b/tcg/optimize.c
>> index c469952..a1bb287 100644
>> --- a/tcg/optimize.c
>> +++ b/tcg/optimize.c
>> @@ -102,6 +102,11 @@ static int op_bits(int op)
>> case INDEX_op_and_i32:
>> case INDEX_op_or_i32:
>> case INDEX_op_xor_i32:
>> + case INDEX_op_shl_i32:
>> + case INDEX_op_shr_i32:
>> + case INDEX_op_sar_i32:
>> + case INDEX_op_rotl_i32:
>> + case INDEX_op_rotr_i32:
>> return 32;
>> #if TCG_TARGET_REG_BITS == 64
>> case INDEX_op_mov_i64:
>> @@ -111,6 +116,11 @@ static int op_bits(int op)
>> case INDEX_op_and_i64:
>> case INDEX_op_or_i64:
>> case INDEX_op_xor_i64:
>> + case INDEX_op_shl_i64:
>> + case INDEX_op_shr_i64:
>> + case INDEX_op_sar_i64:
>> + case INDEX_op_rotl_i64:
>> + case INDEX_op_rotr_i64:
>> return 64;
>> #endif
>> default:
>> @@ -205,6 +215,58 @@ static TCGArg do_constant_folding_2(int op, TCGArg x,
>> TCGArg y)
>> CASE_OP_32_64(xor):
>> return x ^ y;
>>
>> + case INDEX_op_shl_i32:
>> + return (uint32_t)x << (uint32_t)y;
>> +
>> +#if TCG_TARGET_REG_BITS == 64
>> + case INDEX_op_shl_i64:
>> + return (uint64_t)x << (uint64_t)y;
>> +#endif
>> +
>> + case INDEX_op_shr_i32:
>> + return (uint32_t)x >> (uint32_t)y;
>> +
>> +#if TCG_TARGET_REG_BITS == 64
>> + case INDEX_op_shr_i64:
>> + return (uint64_t)x >> (uint64_t)y;
>> +#endif
>> +
>> + case INDEX_op_sar_i32:
>> + return (int32_t)x >> (int32_t)y;
>> +
>> +#if TCG_TARGET_REG_BITS == 64
>> + case INDEX_op_sar_i64:
>> + return (int64_t)x >> (int64_t)y;
>> +#endif
>> +
>> + case INDEX_op_rotr_i32:
>> +#if TCG_TARGET_REG_BITS == 64
>> + x &= 0x;
>> + y &= 0x;
>> +#endif
>> + x = (x << (32 - y)) | (x >> y);
>> + return x;
>> +
>> +#if TCG_TARGET_REG_BITS == 64
>> + case INDEX_op_rotr_i64:
>> + x = (x << (64 - y)) | (x >> y);
>> + return x;
>> +#endif
>> +
>> + case INDEX_op_rotl_i32:
>> +#if TCG_TARGET_REG_BITS == 64
>> + x &= 0x;
>> + y &= 0x;
>> +#endif
>> + x = (x << y) | (x >> (32 - y));
>> + return x;
>> +
>> +#if TCG_TARGET_REG_BITS == 64
>> + case INDEX_op_rotl_i64:
>> + x = (x << y) | (x >> (64 - y));
>> + return x;
>> +#endif
>> +
>> default:
>> fprintf(stderr,
>> "Unrecognized operation %d in do_constant_folding.\n", op);
>> @@ -278,6 +340,11 @@ static TCGArg *tcg_constant_folding(TCGContext *s,
>> uint16_t *tcg_opc_ptr,
>> switch (op) {
>> CASE_OP_32_64(add):
>> CASE_OP_32_64(sub):
>> + CASE_OP_32_64(shl):
>> + CASE_OP_32_64(shr):
>> + CASE_OP_32_64(sar):
>> + CASE_OP_32_64(rotl):
>> + CASE_OP_32_64(rotr):
>> if (temps[args[1]].state == TCG_TEMP_CONST) {
>> /* Proceed with possible constant folding. */
>> break;
>> @@ -363,6 +430,11 @@ static TCGArg *tcg_constant_folding(TCGContext *s,
>> uint16_t *tcg_opc_ptr,
>> CASE_OP_32_64(or):
>> CASE_OP_32_64(and):
>> CASE_OP_32_64(xor):
>> + CASE_OP_32_64(shl):
>> + CASE_OP_32_64(shr):
>> + CASE_OP_32_64(sar):
>> + CASE_OP_32_64(rotl):
>> + CASE_OP_32_64(rotr):
>> if (temps[args[1]].state == TCG_TEMP_CONST
>> && temps[args[2]].state == TCG_TEMP_CONST) {
>> gen_opc_buf[op_index] = op_to_movi(op);
>> --
>> 1.7.4.1
>>
>>
>>
>
[Qemu-devel] [PATCH] TCG: improve optimizer debugging
Use enum TCGOpcode instead of plain old int so that the name of
current op can be seen in GDB. Add a default case to switch
so that GCC does not complain about unhandled enum cases.
Signed-off-by: Blue Swirl
---
tcg/optimize.c | 15 +--
1 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/tcg/optimize.c b/tcg/optimize.c
index a3bfa5e..23e69f6 100644
--- a/tcg/optimize.c
+++ b/tcg/optimize.c
@@ -92,7 +92,7 @@ static void reset_temp(TCGArg temp, int nb_temps,
int nb_globals)
}
}
-static int op_bits(int op)
+static int op_bits(TCGOpcode op)
{
switch (op) {
case INDEX_op_mov_i32:
@@ -169,7 +169,7 @@ static int op_bits(int op)
}
}
-static int op_to_movi(int op)
+static TCGOpcode op_to_movi(TCGOpcode op)
{
switch (op_bits(op)) {
case 32:
@@ -218,7 +218,7 @@ static void tcg_opt_gen_movi(TCGArg *gen_args,
TCGArg dst, TCGArg val,
gen_args[1] = val;
}
-static int op_to_mov(int op)
+static TCGOpcode op_to_mov(TCGOpcode op)
{
switch (op_bits(op)) {
case 32:
@@ -234,7 +234,7 @@ static int op_to_mov(int op)
}
}
-static TCGArg do_constant_folding_2(int op, TCGArg x, TCGArg y)
+static TCGArg do_constant_folding_2(TCGOpcode op, TCGArg x, TCGArg y)
{
switch (op) {
CASE_OP_32_64(add):
@@ -384,7 +384,7 @@ static TCGArg do_constant_folding_2(int op, TCGArg
x, TCGArg y)
}
}
-static TCGArg do_constant_folding(int op, TCGArg x, TCGArg y)
+static TCGArg do_constant_folding(TCGOpcode op, TCGArg x, TCGArg y)
{
TCGArg res = do_constant_folding_2(op, x, y);
#if TCG_TARGET_REG_BITS == 64
@@ -399,7 +399,8 @@ static TCGArg do_constant_folding(int op, TCGArg
x, TCGArg y)
static TCGArg *tcg_constant_folding(TCGContext *s, uint16_t *tcg_opc_ptr,
TCGArg *args, TCGOpDef *tcg_op_defs)
{
-int i, nb_ops, op_index, op, nb_temps, nb_globals, nb_call_args;
+int i, nb_ops, op_index, nb_temps, nb_globals, nb_call_args;
+TCGOpcode op;
const TCGOpDef *def;
TCGArg *gen_args;
TCGArg tmp;
@@ -508,6 +509,8 @@ static TCGArg *tcg_constant_folding(TCGContext *s,
uint16_t *tcg_opc_ptr,
continue;
}
break;
+default:
+break;
}
/* Propagate constants through copy operations and do constant
--
1.6.2.4
From 257a33ee8b83070f826f3e8a932e12acfba5ba8c Mon Sep 17 00:00:00 2001
Message-Id: <257a33ee8b83070f826f3e8a932e12acfba5ba8c.1312053638.git.blauwir...@gmail.com>
From: Blue Swirl
Date: Sat, 30 Jul 2011 19:18:32 +
Subject: [PATCH] TCG: improve optimizer debugging
Use enum TCGOpcode instead of plain old int so that the name of
current op can be seen in GDB. Add a default case to switch
so that GCC does not complain about unhandled enum cases.
Signed-off-by: Blue Swirl
---
tcg/optimize.c | 15 +--
1 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/tcg/optimize.c b/tcg/optimize.c
index a3bfa5e..23e69f6 100644
--- a/tcg/optimize.c
+++ b/tcg/optimize.c
@@ -92,7 +92,7 @@ static void reset_temp(TCGArg temp, int nb_temps, int nb_globals)
}
}
-static int op_bits(int op)
+static int op_bits(TCGOpcode op)
{
switch (op) {
case INDEX_op_mov_i32:
@@ -169,7 +169,7 @@ static int op_bits(int op)
}
}
-static int op_to_movi(int op)
+static TCGOpcode op_to_movi(TCGOpcode op)
{
switch (op_bits(op)) {
case 32:
@@ -218,7 +218,7 @@ static void tcg_opt_gen_movi(TCGArg *gen_args, TCGArg dst, TCGArg val,
gen_args[1] = val;
}
-static int op_to_mov(int op)
+static TCGOpcode op_to_mov(TCGOpcode op)
{
switch (op_bits(op)) {
case 32:
@@ -234,7 +234,7 @@ static int op_to_mov(int op)
}
}
-static TCGArg do_constant_folding_2(int op, TCGArg x, TCGArg y)
+static TCGArg do_constant_folding_2(TCGOpcode op, TCGArg x, TCGArg y)
{
switch (op) {
CASE_OP_32_64(add):
@@ -384,7 +384,7 @@ static TCGArg do_constant_folding_2(int op, TCGArg x, TCGArg y)
}
}
-static TCGArg do_constant_folding(int op, TCGArg x, TCGArg y)
+static TCGArg do_constant_folding(TCGOpcode op, TCGArg x, TCGArg y)
{
TCGArg res = do_constant_folding_2(op, x, y);
#if TCG_TARGET_REG_BITS == 64
@@ -399,7 +399,8 @@ static TCGArg do_constant_folding(int op, TCGArg x, TCGArg y)
static TCGArg *tcg_constant_folding(TCGContext *s, uint16_t *tcg_opc_ptr,
TCGArg *args, TCGOpDef *tcg_op_defs)
{
-int i, nb_ops, op_index, op, nb_temps, nb_globals, nb_call_args;
+int i, nb_ops, op_index, nb_temps, nb_globals, nb_call_args;
+TCGOpcode op;
const TCGOpDef *def;
TCGArg *gen_args;
TCGArg tmp;
@@ -508,6 +509,8 @@ static TCGArg *tcg_constant_folding(TCGContext *s, uint16_t *tcg_opc_ptr,
continue;
}
break;
+default:
+break;
}
/* Propagate constants through copy operations and do constant
--
1.7.2.5
[Qemu-devel] [PATCH] dyngen-exec.h: cleanup
Remove unused or otherwise available stuff. Signed-off-by: Blue Swirl --- dyngen-exec.h | 14 -- 1 files changed, 0 insertions(+), 14 deletions(-) diff --git a/dyngen-exec.h b/dyngen-exec.h index cc1e4fb..8beb7f3 100644 --- a/dyngen-exec.h +++ b/dyngen-exec.h @@ -19,15 +19,6 @@ #if !defined(__DYNGEN_EXEC_H__) #define __DYNGEN_EXEC_H__ -#include "qemu-common.h" - -#ifdef __OpenBSD__ -#include -#endif - -/* XXX: This may be wrong for 64-bit ILP32 hosts. */ -typedef void * host_reg_t; - #if defined(__i386__) #define AREG0 "ebp" #elif defined(__x86_64__) @@ -66,11 +57,6 @@ typedef void * host_reg_t; register CPUState *env asm(AREG0); -#define xglue(x, y) x ## y -#define glue(x, y) xglue(x, y) -#define stringify(s) tostring(s) -#define tostring(s)#s - /* The return address may point to the start of the next instruction. Subtracting one gets us the call instruction itself. */ #if defined(__s390__) && !defined(__s390x__) -- 1.6.2.4 From 474aab1da10b49190493a65c073407510e129a8f Mon Sep 17 00:00:00 2001 Message-Id: <474aab1da10b49190493a65c073407510e129a8f.1312053733.git.blauwir...@gmail.com> From: Blue Swirl Date: Sat, 30 Jul 2011 15:50:22 + Subject: [PATCH] dyngen-exec.h: cleanup Remove unused or otherwise available stuff. Signed-off-by: Blue Swirl --- dyngen-exec.h | 14 -- 1 files changed, 0 insertions(+), 14 deletions(-) diff --git a/dyngen-exec.h b/dyngen-exec.h index cc1e4fb..8beb7f3 100644 --- a/dyngen-exec.h +++ b/dyngen-exec.h @@ -19,15 +19,6 @@ #if !defined(__DYNGEN_EXEC_H__) #define __DYNGEN_EXEC_H__ -#include "qemu-common.h" - -#ifdef __OpenBSD__ -#include -#endif - -/* XXX: This may be wrong for 64-bit ILP32 hosts. */ -typedef void * host_reg_t; - #if defined(__i386__) #define AREG0 "ebp" #elif defined(__x86_64__) @@ -66,11 +57,6 @@ typedef void * host_reg_t; register CPUState *env asm(AREG0); -#define xglue(x, y) x ## y -#define glue(x, y) xglue(x, y) -#define stringify(s) tostring(s) -#define tostring(s) #s - /* The return address may point to the start of the next instruction. Subtracting one gets us the call instruction itself. */ #if defined(__s390__) && !defined(__s390x__) -- 1.7.2.5
Re: [Qemu-devel] [PATCH] fix disabling interrupts in sun4u
On Sat, Jul 30, 2011 at 3:25 PM, Blue Swirl wrote:
> On Sat, Jul 30, 2011 at 3:31 PM, Artyom Tarasenko wrote:
>> On Sat, Jul 30, 2011 at 11:09 AM, Blue Swirl wrote:
>>> On Mon, Jul 25, 2011 at 8:22 PM, Artyom Tarasenko
>>> wrote:
clear interrupt request if the interrupt priority < CPU pil
clear hardware interrupt request if interrupts are disabled
Signed-off-by: Artyom Tarasenko
---
hw/sun4u.c | 6 --
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/hw/sun4u.c b/hw/sun4u.c
index d7dcaf0..7f95aeb 100644
--- a/hw/sun4u.c
+++ b/hw/sun4u.c
@@ -255,7 +255,7 @@ void cpu_check_irqs(CPUState *env)
pil |= 1 << 14;
}
- if (!pil) {
+ if (pil < (2 << env->psrpil)){
>>>
>>> Sorry, I don't understand the patch. Where is this '2' coming from?
>>
>> We shouldn't interrupt at levels <= psrpil. The bit corresponding to
>> psrpil is (1<< psrpil),
>> the next bit is (2 << psrpil).
>
> Now I see. The check below "i > env->psrpil" does something similar
> but it doesn't reset interrupt.
>
> How about pil < (1 << (env->psrpil + 1))? I think that makes the
> purpose clearer.
But it's also one operation more. Shall I just add a comment?
>>>
if (env->interrupt_request & CPU_INTERRUPT_HARD) {
CPUIRQ_DPRINTF("Reset CPU IRQ (current interrupt %x)\n",
env->interrupt_index);
@@ -287,10 +287,12 @@ void cpu_check_irqs(CPUState *env)
break;
}
}
- } else {
+ } else if (env->interrupt_request & CPU_INTERRUPT_HARD) {
CPUIRQ_DPRINTF("Interrupts disabled, pil=%08x pil_in=%08x
softint=%08x "
"current interrupt %x\n",
pil, env->pil_in, env->softint,
env->interrupt_index);
+ env->interrupt_index = 0;
+ cpu_reset_interrupt(env, CPU_INTERRUPT_HARD);
>>>
>>> Why reset the index? The idea is that the interrupt is left pending a
>>> change to PIL etc.
>>
>> But it is kept in env->pil_in and env->softint . Am I missing some
>> scenario where it's not enough?
>
> cpu-exec.c:378 checks interrupt_index, not pil_in etc.
The scenario this patch is trying to fix:
There comes an interrupt N, the PIL is small enough to proceed,
proceeding disables the interrupts.
Then the interrupt handler deasserts N and enables the interrupts. The
interrupt N comes again but is not processed because of old_interrupt
!= new_interrupt check.
--
Regards,
Artyom Tarasenko
solaris/sparc under qemu blog: http://tyom.blogspot.com/
Re: [Qemu-devel] [PATCH] fix disabling interrupts in sun4u
On Sat, Jul 30, 2011 at 11:19 PM, Artyom Tarasenko wrote:
> On Sat, Jul 30, 2011 at 3:25 PM, Blue Swirl wrote:
>> On Sat, Jul 30, 2011 at 3:31 PM, Artyom Tarasenko
>> wrote:
>>> On Sat, Jul 30, 2011 at 11:09 AM, Blue Swirl wrote:
On Mon, Jul 25, 2011 at 8:22 PM, Artyom Tarasenko
wrote:
> clear interrupt request if the interrupt priority < CPU pil
> clear hardware interrupt request if interrupts are disabled
>
> Signed-off-by: Artyom Tarasenko
> ---
> hw/sun4u.c | 6 --
> 1 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/hw/sun4u.c b/hw/sun4u.c
> index d7dcaf0..7f95aeb 100644
> --- a/hw/sun4u.c
> +++ b/hw/sun4u.c
> @@ -255,7 +255,7 @@ void cpu_check_irqs(CPUState *env)
> pil |= 1 << 14;
> }
>
> - if (!pil) {
> + if (pil < (2 << env->psrpil)){
Sorry, I don't understand the patch. Where is this '2' coming from?
>>>
>>> We shouldn't interrupt at levels <= psrpil. The bit corresponding to
>>> psrpil is (1<< psrpil),
>>> the next bit is (2 << psrpil).
>>
>> Now I see. The check below "i > env->psrpil" does something similar
>> but it doesn't reset interrupt.
>>
>> How about pil < (1 << (env->psrpil + 1))? I think that makes the
>> purpose clearer.
>
> But it's also one operation more. Shall I just add a comment?
I think the compiler should be able to calculate that 1 << (x + 1) == 2 << x.
> if (env->interrupt_request & CPU_INTERRUPT_HARD) {
> CPUIRQ_DPRINTF("Reset CPU IRQ (current interrupt %x)\n",
> env->interrupt_index);
> @@ -287,10 +287,12 @@ void cpu_check_irqs(CPUState *env)
> break;
> }
> }
> - } else {
> + } else if (env->interrupt_request & CPU_INTERRUPT_HARD) {
> CPUIRQ_DPRINTF("Interrupts disabled, pil=%08x pil_in=%08x
> softint=%08x "
> "current interrupt %x\n",
> pil, env->pil_in, env->softint,
> env->interrupt_index);
> + env->interrupt_index = 0;
> + cpu_reset_interrupt(env, CPU_INTERRUPT_HARD);
Why reset the index? The idea is that the interrupt is left pending a
change to PIL etc.
>>>
>>> But it is kept in env->pil_in and env->softint . Am I missing some
>>> scenario where it's not enough?
>>
>> cpu-exec.c:378 checks interrupt_index, not pil_in etc.
>
> The scenario this patch is trying to fix:
>
> There comes an interrupt N, the PIL is small enough to proceed,
> proceeding disables the interrupts.
> Then the interrupt handler deasserts N and enables the interrupts. The
> interrupt N comes again but is not processed because of old_interrupt
> != new_interrupt check.
Deasserting the interrupt should clear pil_in which should flow down
to interrupt_index. It's a bug if this does not happen.
Re: [Qemu-devel] [PATCH] fix disabling interrupts in sun4u
On Sat, Jul 30, 2011 at 10:32 PM, Blue Swirl wrote:
> On Sat, Jul 30, 2011 at 11:19 PM, Artyom Tarasenko
> wrote:
>> On Sat, Jul 30, 2011 at 3:25 PM, Blue Swirl wrote:
>>> On Sat, Jul 30, 2011 at 3:31 PM, Artyom Tarasenko
>>> wrote:
On Sat, Jul 30, 2011 at 11:09 AM, Blue Swirl wrote:
> On Mon, Jul 25, 2011 at 8:22 PM, Artyom Tarasenko
> wrote:
>> clear interrupt request if the interrupt priority < CPU pil
>> clear hardware interrupt request if interrupts are disabled
>>
>> Signed-off-by: Artyom Tarasenko
>> ---
>> hw/sun4u.c | 6 --
>> 1 files changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/sun4u.c b/hw/sun4u.c
>> index d7dcaf0..7f95aeb 100644
>> --- a/hw/sun4u.c
>> +++ b/hw/sun4u.c
>> @@ -255,7 +255,7 @@ void cpu_check_irqs(CPUState *env)
>> pil |= 1 << 14;
>> }
>>
>> - if (!pil) {
>> + if (pil < (2 << env->psrpil)){
>
> Sorry, I don't understand the patch. Where is this '2' coming from?
We shouldn't interrupt at levels <= psrpil. The bit corresponding to
psrpil is (1<< psrpil),
the next bit is (2 << psrpil).
>>>
>>> Now I see. The check below "i > env->psrpil" does something similar
>>> but it doesn't reset interrupt.
>>>
>>> How about pil < (1 << (env->psrpil + 1))? I think that makes the
>>> purpose clearer.
>>
>> But it's also one operation more. Shall I just add a comment?
>
> I think the compiler should be able to calculate that 1 << (x + 1) == 2 << x.
>
Are you sure?
$ cat test_smart_shift.c
int main (int argc, char *argv[]) {
return 1 << (1+argc);
}
$ sparc64-linux-gnu-gcc-4.4.5 -O2 -S test_smart_shift.c
cat test_smart_shift.s
.file "test_smart_shift.c"
.section".text"
.align 4
.align 32
.global main
.type main, #function
.proc 04
main:
mov 1, %g1
add %o0, 1, %o0
sll %g1, %o0, %o0
jmp %o7+8
sra%o0, 0, %o0
.size main, .-main
.ident "GCC: (GNU) 4.4.5"
.section.note.GNU-stack,"",@progbits
>
>> if (env->interrupt_request & CPU_INTERRUPT_HARD) {
>> CPUIRQ_DPRINTF("Reset CPU IRQ (current interrupt %x)\n",
>> env->interrupt_index);
>> @@ -287,10 +287,12 @@ void cpu_check_irqs(CPUState *env)
>> break;
>> }
>> }
>> - } else {
>> + } else if (env->interrupt_request & CPU_INTERRUPT_HARD) {
>> CPUIRQ_DPRINTF("Interrupts disabled, pil=%08x pil_in=%08x
>> softint=%08x "
>> "current interrupt %x\n",
>> pil, env->pil_in, env->softint,
>> env->interrupt_index);
>> + env->interrupt_index = 0;
>> + cpu_reset_interrupt(env, CPU_INTERRUPT_HARD);
>
> Why reset the index? The idea is that the interrupt is left pending a
> change to PIL etc.
But it is kept in env->pil_in and env->softint . Am I missing some
scenario where it's not enough?
>>>
>>> cpu-exec.c:378 checks interrupt_index, not pil_in etc.
>>
>> The scenario this patch is trying to fix:
>>
>> There comes an interrupt N, the PIL is small enough to proceed,
>> proceeding disables the interrupts.
>> Then the interrupt handler deasserts N and enables the interrupts. The
>> interrupt N comes again but is not processed because of old_interrupt
>> != new_interrupt check.
>
> Deasserting the interrupt should clear pil_in which should flow down
> to interrupt_index. It's a bug if this does not happen.
Ok, I'll drop the second part in v2 then.
--
Regards,
Artyom Tarasenko
solaris/sparc under qemu blog: http://tyom.blogspot.com/
[Qemu-devel] [Bug 818645] [NEW] Unhandled OF service in FreeBSD loader - "unimplemented service milliseconds"
Public bug reported: wkoszek@wkoszek:~/bin/qemu/qemu$ git log | head -1 commit c886edfb851c0c590d4e77f058f2ec8ed95ad1b5 built with default settings. Run like that: /home/wkoszek/bin/qemu-dynamic/bin/qemu-system-sparc64 -m 1024 -cdrom ~/Pulpit/iso/FreeBSD-7.4-RELEASE-sparc64-bootonly.iso -hda ~/Pulpit/iso/freebsd_sparc64.qcow2 -nographic -boot d Configuration device id QEMU version 1 machine id 0 kernel cmdline CPUs: 1 x SUNW,UltraSPARC-IIi UUID: ---- Welcome to OpenBIOS v1.0 built on Jul 20 2011 21:17 Type 'help' for detailed information Trying cdrom:f... Not a bootable ELF image Loading a.out image... Loaded 7680 bytes entry point is 0x4000 Jumping to entry point 4000 for type 0005... switching to new context: entry point 0x4000 stack 0xffe86b49 >> FreeBSD/sparc64 boot block Boot path: cdrom:f Boot loader: /boot/loader Consoles: Open Firmware console Booting with sun4u support. Boot path set to cdrom:a FreeBSD/sparc64 bootstrap loader, Revision 1.0 (r...@warner.cse.buffalo.edu, Fri Feb 18 05:38:31 UTC 2011) bootpath="cdrom:a" Loading /boot/defaults/loader.conf /boot/kernel/kernel data=0x8d1f48+0x82f88 syms=[0x8+0x88ec0+0x8+0x76966] | Unimplemented service milliseconds ([0] -- [1]) Hit [Enter] to boot immediately, or any other key for command prompt. Unimplemented service milliseconds ([0] -- [1]) Unimplemented service milliseconds ([0] -- [1]) Unimplemented service milliseconds ([0] -- [1]) Unimplemented service milliseconds ([0] -- [1]) Unimplemented service milliseconds ([0] -- [1]) This basically loops here unless I press CTRL+C. Was there any OpenBIOS release that didn't have this problem? ** Affects: qemu Importance: Undecided Status: New ** Tags: freebsd -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/818645 Title: Unhandled OF service in FreeBSD loader - "unimplemented service milliseconds" Status in QEMU: New Bug description: wkoszek@wkoszek:~/bin/qemu/qemu$ git log | head -1 commit c886edfb851c0c590d4e77f058f2ec8ed95ad1b5 built with default settings. Run like that: /home/wkoszek/bin/qemu-dynamic/bin/qemu-system-sparc64 -m 1024 -cdrom ~/Pulpit/iso/FreeBSD-7.4-RELEASE-sparc64-bootonly.iso -hda ~/Pulpit/iso/freebsd_sparc64.qcow2 -nographic -boot d Configuration device id QEMU version 1 machine id 0 kernel cmdline CPUs: 1 x SUNW,UltraSPARC-IIi UUID: ---- Welcome to OpenBIOS v1.0 built on Jul 20 2011 21:17 Type 'help' for detailed information Trying cdrom:f... Not a bootable ELF image Loading a.out image... Loaded 7680 bytes entry point is 0x4000 Jumping to entry point 4000 for type 0005... switching to new context: entry point 0x4000 stack 0xffe86b49 >> FreeBSD/sparc64 boot block Boot path: cdrom:f Boot loader: /boot/loader Consoles: Open Firmware console Booting with sun4u support. Boot path set to cdrom:a FreeBSD/sparc64 bootstrap loader, Revision 1.0 (r...@warner.cse.buffalo.edu, Fri Feb 18 05:38:31 UTC 2011) bootpath="cdrom:a" Loading /boot/defaults/loader.conf /boot/kernel/kernel data=0x8d1f48+0x82f88 syms=[0x8+0x88ec0+0x8+0x76966] | Unimplemented service milliseconds ([0] -- [1]) Hit [Enter] to boot immediately, or any other key for command prompt. Unimplemented service milliseconds ([0] -- [1]) Unimplemented service milliseconds ([0] -- [1]) Unimplemented service milliseconds ([0] -- [1]) Unimplemented service milliseconds ([0] -- [1]) Unimplemented service milliseconds ([0] -- [1]) This basically loops here unless I press CTRL+C. Was there any OpenBIOS release that didn't have this problem? To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/818645/+subscriptions
Re: [Qemu-devel] [PATCH] fix disabling interrupts in sun4u
On Sat, Jul 30, 2011 at 8:58 PM, Artyom Tarasenko wrote:
> On Sat, Jul 30, 2011 at 10:32 PM, Blue Swirl wrote:
>> On Sat, Jul 30, 2011 at 11:19 PM, Artyom Tarasenko
>> wrote:
>>> On Sat, Jul 30, 2011 at 3:25 PM, Blue Swirl wrote:
On Sat, Jul 30, 2011 at 3:31 PM, Artyom Tarasenko
wrote:
> On Sat, Jul 30, 2011 at 11:09 AM, Blue Swirl wrote:
>> On Mon, Jul 25, 2011 at 8:22 PM, Artyom Tarasenko
>> wrote:
>>> clear interrupt request if the interrupt priority < CPU pil
>>> clear hardware interrupt request if interrupts are disabled
>>>
>>> Signed-off-by: Artyom Tarasenko
>>> ---
>>> hw/sun4u.c | 6 --
>>> 1 files changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/hw/sun4u.c b/hw/sun4u.c
>>> index d7dcaf0..7f95aeb 100644
>>> --- a/hw/sun4u.c
>>> +++ b/hw/sun4u.c
>>> @@ -255,7 +255,7 @@ void cpu_check_irqs(CPUState *env)
>>> pil |= 1 << 14;
>>> }
>>>
>>> - if (!pil) {
>>> + if (pil < (2 << env->psrpil)){
>>
>> Sorry, I don't understand the patch. Where is this '2' coming from?
>
> We shouldn't interrupt at levels <= psrpil. The bit corresponding to
> psrpil is (1<< psrpil),
> the next bit is (2 << psrpil).
Now I see. The check below "i > env->psrpil" does something similar
but it doesn't reset interrupt.
How about pil < (1 << (env->psrpil + 1))? I think that makes the
purpose clearer.
>>>
>>> But it's also one operation more. Shall I just add a comment?
>>
>> I think the compiler should be able to calculate that 1 << (x + 1) == 2 << x.
>>
> Are you sure?
>
> $ cat test_smart_shift.c
> int main (int argc, char *argv[]) {
> return 1 << (1+argc);
> }
>
> $ sparc64-linux-gnu-gcc-4.4.5 -O2 -S test_smart_shift.c
> cat test_smart_shift.s
> .file "test_smart_shift.c"
> .section ".text"
> .align 4
> .align 32
> .global main
> .type main, #function
> .proc 04
> main:
> mov 1, %g1
> add %o0, 1, %o0
> sll %g1, %o0, %o0
> jmp %o7+8
> sra %o0, 0, %o0
> .size main, .-main
> .ident "GCC: (GNU) 4.4.5"
> .section .note.GNU-stack,"",@progbits
Same happens on amd64. Well, the compiler isn't so smart as I thought.
Then please use the 2 << version with a comment.
>>
>>> if (env->interrupt_request & CPU_INTERRUPT_HARD) {
>>> CPUIRQ_DPRINTF("Reset CPU IRQ (current interrupt %x)\n",
>>> env->interrupt_index);
>>> @@ -287,10 +287,12 @@ void cpu_check_irqs(CPUState *env)
>>> break;
>>> }
>>> }
>>> - } else {
>>> + } else if (env->interrupt_request & CPU_INTERRUPT_HARD) {
>>> CPUIRQ_DPRINTF("Interrupts disabled, pil=%08x pil_in=%08x
>>> softint=%08x "
>>> "current interrupt %x\n",
>>> pil, env->pil_in, env->softint,
>>> env->interrupt_index);
>>> + env->interrupt_index = 0;
>>> + cpu_reset_interrupt(env, CPU_INTERRUPT_HARD);
>>
>> Why reset the index? The idea is that the interrupt is left pending a
>> change to PIL etc.
>
> But it is kept in env->pil_in and env->softint . Am I missing some
> scenario where it's not enough?
cpu-exec.c:378 checks interrupt_index, not pil_in etc.
>>>
>>> The scenario this patch is trying to fix:
>>>
>>> There comes an interrupt N, the PIL is small enough to proceed,
>>> proceeding disables the interrupts.
>>> Then the interrupt handler deasserts N and enables the interrupts. The
>>> interrupt N comes again but is not processed because of old_interrupt
>>> != new_interrupt check.
>>
>> Deasserting the interrupt should clear pil_in which should flow down
>> to interrupt_index. It's a bug if this does not happen.
>
> Ok, I'll drop the second part in v2 then.
>
>
> --
> Regards,
> Artyom Tarasenko
>
> solaris/sparc under qemu blog: http://tyom.blogspot.com/
>
[Qemu-devel] [Bug 818647] [NEW] Getting segmentation fault when trying to boot FreeBSD
Public bug reported: wkoszek@wkoszek:~/bin/qemu/qemu$ git log | head -1 commit c886edfb851c0c590d4e77f058f2ec8ed95ad1b5 wkoszek@wkoszek:~/o/freebsd/sys/boot/i386$ qemu-system-sparc64 --version QEMU emulator version 0.15.50, Copyright (c) 2003-2008 Fabrice Bellard wkoszek@wkoszek:~/o/freebsd/sys/boot/i386$ uname -a Linux wkoszek 2.6.38-10-generic #46-Ubuntu SMP Tue Jun 28 15:05:41 UTC 2011 i686 i686 i386 GNU/Linux Qemu built with default settings (./configure --prefix= && make && make install) I run FreeBSD ISO image: /home/wkoszek/bin/qemu-dynamic/bin/qemu-system-sparc64 -m 1024 -cdrom ~/Pulpit/iso/FreeBSD-7.4-RELEASE-sparc64-bootonly.iso -hda ~/Pulpit/iso/freebsd_sparc64.qcow2 -nographic -boot d Configuration device id QEMU version 1 machine id 0 kernel cmdline CPUs: 1 x SUNW,UltraSPARC-IIi UUID: ---- Welcome to OpenBIOS v1.0 built on Jul 20 2011 21:17 Type 'help' for detailed information Trying cdrom:f... Not a bootable ELF image Loading a.out image... Loaded 7680 bytes entry point is 0x4000 Jumping to entry point 4000 for type 0005... switching to new context: entry point 0x4000 stack 0xffe86b49 >> FreeBSD/sparc64 boot block Boot path: cdrom:f Boot loader: /boot/loader Consoles: Open Firmware console Booting with sun4u support. Boot path set to cdrom:a FreeBSD/sparc64 bootstrap loader, Revision 1.0 (r...@warner.cse.buffalo.edu, Fri Feb 18 05:38:31 UTC 2011) bootpath="cdrom:a" Loading /boot/defaults/loader.conf /boot/kernel/kernel data=0x8d1f48+0x82f88 syms=[0x8+0x88ec0+0x8+0x76966] | Unimplemented service milliseconds ([0] -- [1]) Hit [Enter] to boot immediately, or any other key for command prompt. Unimplemented service milliseconds ([0] -- [1]) Unimplemented service milliseconds ([0] -- [1]) Unimplemented service milliseconds ([0] -- [1]) Unimplemented service milliseconds ([0] -- [1]) Unimplemented service milliseconds ([0] -- [1]) Unimplemented service milliseconds ([0] -- [1]) I press CTRL + C and I get out of the looped warning about "unimplemented service". Then I see: Type '?' for a list of commands, 'help' for more detailed help. OK boot jumping to kernel entry at 0xc0078000. BOOTUnhandled Exception 0x0034 PC = 0xc0637454 NPC = 0xc0637458 I wanted to start FreeBSD debugging here - I pressed 'CTRL+A c', I was dropped to the monitor. FRom the monitor I typed: Stopping execution QEMU 0.15.50 monitor - type 'help' for more information (qemu) x 0xc0078000 c0078000: Cannot access memory (qemu) x 0xc0637454 c0637454: Cannot access memory (qemu) x 0xc0637458 c0637458: Cannot access memory (qemu) xp 0xc0078000 Segmentation fault IMO it shouldn't have crashed. ** Affects: qemu Importance: Undecided Status: New ** Tags: freebsd -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/818647 Title: Getting segmentation fault when trying to boot FreeBSD Status in QEMU: New Bug description: wkoszek@wkoszek:~/bin/qemu/qemu$ git log | head -1 commit c886edfb851c0c590d4e77f058f2ec8ed95ad1b5 wkoszek@wkoszek:~/o/freebsd/sys/boot/i386$ qemu-system-sparc64 --version QEMU emulator version 0.15.50, Copyright (c) 2003-2008 Fabrice Bellard wkoszek@wkoszek:~/o/freebsd/sys/boot/i386$ uname -a Linux wkoszek 2.6.38-10-generic #46-Ubuntu SMP Tue Jun 28 15:05:41 UTC 2011 i686 i686 i386 GNU/Linux Qemu built with default settings (./configure --prefix= && make && make install) I run FreeBSD ISO image: /home/wkoszek/bin/qemu-dynamic/bin/qemu-system-sparc64 -m 1024 -cdrom ~/Pulpit/iso/FreeBSD-7.4-RELEASE-sparc64-bootonly.iso -hda ~/Pulpit/iso/freebsd_sparc64.qcow2 -nographic -boot d Configuration device id QEMU version 1 machine id 0 kernel cmdline CPUs: 1 x SUNW,UltraSPARC-IIi UUID: ---- Welcome to OpenBIOS v1.0 built on Jul 20 2011 21:17 Type 'help' for detailed information Trying cdrom:f... Not a bootable ELF image Loading a.out image... Loaded 7680 bytes entry point is 0x4000 Jumping to entry point 4000 for type 0005... switching to new context: entry point 0x4000 stack 0xffe86b49 >> FreeBSD/sparc64 boot block Boot path: cdrom:f Boot loader: /boot/loader Consoles: Open Firmware console Booting with sun4u support. Boot path set to cdrom:a FreeBSD/sparc64 bootstrap loader, Revision 1.0 (r...@warner.cse.buffalo.edu, Fri Feb 18 05:38:31 UTC 2011) bootpath="cdrom:a" Loading /boot/defaults/loader.conf /boot/kernel/kernel data=0x8d1f48+0x82f88 syms=[0x8+0x88ec0+0x8+0x76966] | Unimplemented service milliseconds ([0] -- [1]) Hit [Enter] to boot immediately, or any other key for command prompt. Unimplemented service milliseconds ([0] -- [1]) Unimplemented service milliseconds ([0] -- [1]) Un
[Qemu-devel] [Bug 818673] [NEW] virtio: trying to map MMIO memory
Public bug reported: Qemu host is Core i7, running Linux. Guest is Windows XP sp3. Often, qemu will crash shortly after starting (1-5 minutes) with a statement "qemu-system-x86_64: virtio: trying to map MMIO memory" This has occured with qemu-kvm 0.14, qemu-kvm 0.14.1, qemu-0.15.0-rc0 and qemu 0.15.0-rc1. Qemu is started as such: qemu-system-x86_64 -cpu host -enable-kvm -pidfile /home/rick/qemu/hds/wxp.pid -drive file=/home/rick/qemu/hds/wxp.raw,if=virtio -m 768 -name WinXP -net nic,model=virtio -net user -localtime -usb -vga qxl -device virtio-serial -chardev spicevmc,name=vdagent,id=vdagent -device virtserialport,chardev=vdagent,name=com.redhat.spice.0 -spice port=1234,disable-ticketing -daemonize -monitor telnet:localhost:12341,server,nowait The WXP guest has virtio 1.1.16 drivers for net and scsi, and the most current spice binaries from spice-space.org. ** 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/818673 Title: virtio: trying to map MMIO memory Status in QEMU: New Bug description: Qemu host is Core i7, running Linux. Guest is Windows XP sp3. Often, qemu will crash shortly after starting (1-5 minutes) with a statement "qemu-system-x86_64: virtio: trying to map MMIO memory" This has occured with qemu-kvm 0.14, qemu-kvm 0.14.1, qemu-0.15.0-rc0 and qemu 0.15.0-rc1. Qemu is started as such: qemu-system-x86_64 -cpu host -enable-kvm -pidfile /home/rick/qemu/hds/wxp.pid -drive file=/home/rick/qemu/hds/wxp.raw,if=virtio -m 768 -name WinXP -net nic,model=virtio -net user -localtime -usb -vga qxl -device virtio-serial -chardev spicevmc,name=vdagent,id=vdagent -device virtserialport,chardev=vdagent,name=com.redhat.spice.0 -spice port=1234,disable-ticketing -daemonize -monitor telnet:localhost:12341,server,nowait The WXP guest has virtio 1.1.16 drivers for net and scsi, and the most current spice binaries from spice-space.org. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/818673/+subscriptions
Re: [Qemu-devel] [PATCH] Fix forcing multicast msgs to loopback on OpenBSD.
On Fri, Jul 29, 2011 at 07:15:11PM -0400, Brad wrote:
> Fix forcing multicast msgs to loopback on OpenBSD.
>
> e.g.
> $ sudo qemu -m 128 -no-fd-bootchk \
> -hda virtual.img -boot n -nographic \
> -net nic,vlan=0,model=rtl8139,macaddr=52:54:00:12:34:03 \
> -net user -tftp /usr/src/sys/arch/i386/compile/TEST -bootp pxeboot \
> -net nic,vlan=1,model=rtl8139,macaddr=52:54:00:23:03:01 \
> -net tap,vlan=1,script=no \
> -net nic,vlan=3,model=rtl8139,macaddr=52:54:00:23:03:03 \
> -net socket,vlan=3,mcast=230.0.0.1:10003
> setsockopt(SOL_IP, IP_MULTICAST_LOOP): Invalid argument
> qemu: -net socket,vlan=3,mcast=230.0.0.1:10003: Device 'socket' could not be
> initialized
>
>
> Signed-off-by: Brad Smith
An updated diff taking Blue Swirl's comment into consideration.
---
net/socket.c | 10 --
1 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/net/socket.c b/net/socket.c
index 11fe5f3..5cd0b9a 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -154,6 +154,12 @@ static int net_socket_mcast_create(struct sockaddr_in
*mcastaddr, struct in_addr
struct ip_mreq imr;
int fd;
int val, ret;
+#ifdef __OpenBSD__
+unsigned char loop;
+#else
+int loop;
+#endif
+
if (!IN_MULTICAST(ntohl(mcastaddr->sin_addr.s_addr))) {
fprintf(stderr, "qemu: error: specified mcastaddr \"%s\" (0x%08x) does
not contain a multicast address\n",
inet_ntoa(mcastaddr->sin_addr),
@@ -197,9 +203,9 @@ static int net_socket_mcast_create(struct sockaddr_in
*mcastaddr, struct in_addr
}
/* Force mcast msgs to loopback (eg. several QEMUs in same host */
-val = 1;
+loop = 1;
ret=setsockopt(fd, IPPROTO_IP, IP_MULTICAST_LOOP,
- (const char *)&val, sizeof(val));
+ (const char *)&loop, sizeof(loop));
if (ret < 0) {
perror("setsockopt(SOL_IP, IP_MULTICAST_LOOP)");
goto fail;
--
1.7.6
--
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.
Re: [Qemu-devel] kvm PCI assignment & VFIO ramblings
On Sat, 2011-07-30 at 12:20 -0600, Alex Williamson wrote: > On x86, the USB controllers don't typically live behind a PCIe-to-PCI > bridge, so don't suffer the source identifier problem, but they do often > share an interrupt. But even then, we can count on most modern devices > supporting PCI2.3, and thus the DisINTx feature, which allows us to > share interrupts. In any case, yes, it's more rare but we need to know > how to handle devices behind PCI bridges. However I disagree that we > need to assign all the devices behind such a bridge to the guest. Well, ok so let's dig a bit more here :-) First, yes I agree they don't all need to appear to the guest. My point is really that we must prevent them to be "used" by somebody else, either host or another guest. Now once you get there, I personally prefer having a clear "group" ownership rather than having devices stay in some "limbo" under vfio control but it's an implementation detail. Regarding DisINTx, well, it's a bit like putting separate PCIe functions into separate guests, it looks good ... but you are taking a chance. Note that I do intend to do some of that for power ... well I think, I haven't completely made my mind. pHyp for has a stricter requirement, PEs essentially are everything behind a bridge. If you have a slot, you have some kind of bridge above this slot and everything on it will be a PE. The problem I see is that with your filtering of config space, BAR emulation, DisINTx etc... you essentially assume that you can reasonably reliably isolate devices. But in practice, it's chancy. Some devices for example have "backdoors" into their own config space via MMIO. If I have such a device in a guest, I can completely override your DisINTx and thus DOS your host or another guest with a shared interrupt. I can move my MMIO around and DOS another function by overlapping the addresses. You can really only be protect yourself against a device if you have it behind a bridge (in addition to having a filtering iommu), which limits the MMIO span (and thus letting the guest whack the BARs randomly will only allow that guest to shoot itself in the foot). Some bridges also provide a way to block INTx below them which comes in handy but it's bridge specific. Some devices can be coerced to send the INTx "assert" message and never de-assert it (for example by doing a soft-reset while it's asserted, which can be done with some devices with an MMIO). Anything below a PCIe -> PCI/PCI-X needs to also be "grouped" due to simple lack of proper filtering by the iommu (PCI-X in theory has RIDs and fowards them up, but this isn't very reliable, for example it fails over with split transactions). Fortunately in PCIe land, we most have bridges above everything. The problem somewhat remains with functions of a device, how can you be sure that there isn't a way via some MMIO to create side effects on the other functions of the device ? (For example by checkstopping the whole thing). You can't really :-) So it boils down of the "level" of safety/isolation you want to provide, and I suppose to some extent it's a user decision but the user needs to be informed to some extent. A hard problem :-) > There's a difference between removing the device from the host and > exposing the device to the guest. If I have a NIC and HBA behind a > bridge, it's perfectly reasonable that I might only assign the NIC to > the guest, but as you describe, we then need to prevent the host, or any > other guest from making use of the HBA. Yes. However the other device is in "limbo" and it may be not clear to the user why it can't be used anymore :-) The question is more, the user needs to "know" (or libvirt does, or somebody ... ) that in order to pass-through device A, it must also "remove" device B from the host. How can you even provide a meaningful error message to the user if all VFIO does is give you something like -EBUSY ? So the information about the grouping constraint must trickle down somewhat. Look at it from a GUI perspective for example. Imagine a front-end showing you devices in your system and allowing you to "Drag & drop" them to your guest. How do you represent that need for grouping ? First how do you expose it from kernel/libvirt to the GUI tool and how do you represent it to the user ? By grouping the devices in logical groups which end up being the "objects" you can drag around, at least you provide some amount of clarity. Now if you follow that path down to how the GUI app, libvirt and possibly qemu need to know / resolve the dependency, being given the "groups" as the primary information of what can be used for pass-through makes everything a lot simpler. > > - The -minimum- granularity of pass-through is not always a single > > device and not always under SW control > > But IMHO, we need to preserve the granularity of exposing a device to a > guest as a single device. That might mean some devices are held hostage > by an agent on the host. Maybe
Re: [Qemu-devel] kvm PCI assignment & VFIO ramblings
On Sat, 2011-07-30 at 12:20 -0600, Alex Williamson wrote: > On x86, the USB controllers don't typically live behind a PCIe-to-PCI > bridge, so don't suffer the source identifier problem, but they do often > share an interrupt. But even then, we can count on most modern devices > supporting PCI2.3, and thus the DisINTx feature, which allows us to > share interrupts. In any case, yes, it's more rare but we need to know > how to handle devices behind PCI bridges. However I disagree that we > need to assign all the devices behind such a bridge to the guest. Well, ok so let's dig a bit more here :-) First, yes I agree they don't all need to appear to the guest. My point is really that we must prevent them to be "used" by somebody else, either host or another guest. Now once you get there, I personally prefer having a clear "group" ownership rather than having devices stay in some "limbo" under vfio control but it's an implementation detail. Regarding DisINTx, well, it's a bit like putting separate PCIe functions into separate guests, it looks good ... but you are taking a chance. Note that I do intend to do some of that for power ... well I think, I haven't completely made my mind. pHyp for has a stricter requirement, PEs essentially are everything behind a bridge. If you have a slot, you have some kind of bridge above this slot and everything on it will be a PE. The problem I see is that with your filtering of config space, BAR emulation, DisINTx etc... you essentially assume that you can reasonably reliably isolate devices. But in practice, it's chancy. Some devices for example have "backdoors" into their own config space via MMIO. If I have such a device in a guest, I can completely override your DisINTx and thus DOS your host or another guest with a shared interrupt. I can move my MMIO around and DOS another function by overlapping the addresses. You can really only be protect yourself against a device if you have it behind a bridge (in addition to having a filtering iommu), which limits the MMIO span (and thus letting the guest whack the BARs randomly will only allow that guest to shoot itself in the foot). Some bridges also provide a way to block INTx below them which comes in handy but it's bridge specific. Some devices can be coerced to send the INTx "assert" message and never de-assert it (for example by doing a soft-reset while it's asserted, which can be done with some devices with an MMIO). Anything below a PCIe -> PCI/PCI-X needs to also be "grouped" due to simple lack of proper filtering by the iommu (PCI-X in theory has RIDs and fowards them up, but this isn't very reliable, for example it fails over with split transactions). Fortunately in PCIe land, we most have bridges above everything. The problem somewhat remains with functions of a device, how can you be sure that there isn't a way via some MMIO to create side effects on the other functions of the device ? (For example by checkstopping the whole thing). You can't really :-) So it boils down of the "level" of safety/isolation you want to provide, and I suppose to some extent it's a user decision but the user needs to be informed to some extent. A hard problem :-) > There's a difference between removing the device from the host and > exposing the device to the guest. If I have a NIC and HBA behind a > bridge, it's perfectly reasonable that I might only assign the NIC to > the guest, but as you describe, we then need to prevent the host, or any > other guest from making use of the HBA. Yes. However the other device is in "limbo" and it may be not clear to the user why it can't be used anymore :-) The question is more, the user needs to "know" (or libvirt does, or somebody ... ) that in order to pass-through device A, it must also "remove" device B from the host. How can you even provide a meaningful error message to the user if all VFIO does is give you something like -EBUSY ? So the information about the grouping constraint must trickle down somewhat. Look at it from a GUI perspective for example. Imagine a front-end showing you devices in your system and allowing you to "Drag & drop" them to your guest. How do you represent that need for grouping ? First how do you expose it from kernel/libvirt to the GUI tool and how do you represent it to the user ? By grouping the devices in logical groups which end up being the "objects" you can drag around, at least you provide some amount of clarity. Now if you follow that path down to how the GUI app, libvirt and possibly qemu need to know / resolve the dependency, being given the "groups" as the primary information of what can be used for pass-through makes everything a lot simpler. > > - The -minimum- granularity of pass-through is not always a single > > device and not always under SW control > > But IMHO, we need to preserve the granularity of exposing a device to a > guest as a single device. That might mean some devices are held hostage > by an agent on the host. Maybe
[Qemu-devel] buildbot failure in qemu on block_x86_64_debian_5_0
The Buildbot has detected a new failure on builder block_x86_64_debian_5_0 while building qemu. Full details are available at: http://buildbot.b1-systems.de/qemu/builders/block_x86_64_debian_5_0/builds/50 Buildbot URL: http://buildbot.b1-systems.de/qemu/ Buildslave for this Build: yuzuki Build Reason: The Nightly scheduler named 'nightly_block' triggered this build Build Source Stamp: [branch block] HEAD Blamelist: BUILD FAILED: failed git sincerely, -The Buildbot
[Qemu-devel] buildbot failure in qemu on block_i386_debian_5_0
The Buildbot has detected a new failure on builder block_i386_debian_5_0 while building qemu. Full details are available at: http://buildbot.b1-systems.de/qemu/builders/block_i386_debian_5_0/builds/50 Buildbot URL: http://buildbot.b1-systems.de/qemu/ Buildslave for this Build: yuzuki Build Reason: The Nightly scheduler named 'nightly_block' triggered this build Build Source Stamp: [branch block] HEAD Blamelist: BUILD FAILED: failed git sincerely, -The Buildbot
[Qemu-devel] buildbot failure in qemu on qmp_i386_debian_5_0
The Buildbot has detected a new failure on builder qmp_i386_debian_5_0 while building qemu. Full details are available at: http://buildbot.b1-systems.de/qemu/builders/qmp_i386_debian_5_0/builds/50 Buildbot URL: http://buildbot.b1-systems.de/qemu/ Buildslave for this Build: yuzuki Build Reason: The Nightly scheduler named 'nightly_qmp' triggered this build Build Source Stamp: [branch queue/qmp] HEAD Blamelist: BUILD FAILED: failed git sincerely, -The Buildbot
[Qemu-devel] buildbot failure in qemu on qmp_x86_64_debian_5_0
The Buildbot has detected a new failure on builder qmp_x86_64_debian_5_0 while building qemu. Full details are available at: http://buildbot.b1-systems.de/qemu/builders/qmp_x86_64_debian_5_0/builds/50 Buildbot URL: http://buildbot.b1-systems.de/qemu/ Buildslave for this Build: yuzuki Build Reason: The Nightly scheduler named 'nightly_qmp' triggered this build Build Source Stamp: [branch queue/qmp] HEAD Blamelist: BUILD FAILED: failed git sincerely, -The Buildbot
[Qemu-devel] buildbot failure in qemu on xen_i386_debian_5_0
The Buildbot has detected a new failure on builder xen_i386_debian_5_0 while building qemu. Full details are available at: http://buildbot.b1-systems.de/qemu/builders/xen_i386_debian_5_0/builds/50 Buildbot URL: http://buildbot.b1-systems.de/qemu/ Buildslave for this Build: yuzuki Build Reason: The Nightly scheduler named 'nightly_xen' triggered this build Build Source Stamp: [branch xen-next] HEAD Blamelist: BUILD FAILED: failed git sincerely, -The Buildbot
[Qemu-devel] buildbot failure in qemu on xen_x86_64_debian_5_0
The Buildbot has detected a new failure on builder xen_x86_64_debian_5_0 while building qemu. Full details are available at: http://buildbot.b1-systems.de/qemu/builders/xen_x86_64_debian_5_0/builds/50 Buildbot URL: http://buildbot.b1-systems.de/qemu/ Buildslave for this Build: yuzuki Build Reason: The Nightly scheduler named 'nightly_xen' triggered this build Build Source Stamp: [branch xen-next] HEAD Blamelist: BUILD FAILED: failed git sincerely, -The Buildbot
