Re: [Qemu-devel] [libfdt][PATCH v2] implement strnlen for systems that need it
On Fri, Oct 20, 2017 at 04:44:58PM -0700, Richard Henderson wrote:
> On 10/20/2017 10:55 AM, John Arbuckle wrote:
> > +static inline size_t strnlen(const char *string, size_t max_count)
> > +{
> > +size_t count;
> > +for (count = 0; count < max_count; count++) {
> > +if (string[count] == '\0') {
> > +break;
> > +}
> > +}
> > +return count;
>
> Not to nitpick, but
>
> const char *p = memchr(string, 0, max_count);
> return p ? max_count : p - string;
Richard's right, that's definitely a better implementation.
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
[Qemu-devel] [Qemu devel v3 PATCH] msf2: Remove dead code reported by Coverity
Fixed incorrect frame size mask, validated maximum frame
size in spi_write and removed dead code.
Signed-off-by: Subbaraya Sundeep
---
v3:
Added comment that [31:6] bits are reserved in R_SPI_DFSIZE
register and logged incorrect value too in guest error(suggested
by Darren).
v2:
else if -> else in set_fifodepth
log guest error when frame size is more than 32
hw/ssi/mss-spi.c | 18 ++
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/hw/ssi/mss-spi.c b/hw/ssi/mss-spi.c
index 5a8e308..e1b6227 100644
--- a/hw/ssi/mss-spi.c
+++ b/hw/ssi/mss-spi.c
@@ -76,9 +76,10 @@
#define C_BIGFIFO(1 << 29)
#define C_RESET (1 << 31)
-#define FRAMESZ_MASK 0x1F
+#define FRAMESZ_MASK 0x3F
#define FMCOUNT_MASK 0x0000
#define FMCOUNT_SHIFT8
+#define FRAMESZ_MAX 32
static void txfifo_reset(MSSSpiState *s)
{
@@ -104,10 +105,8 @@ static void set_fifodepth(MSSSpiState *s)
s->fifo_depth = 32;
} else if (size <= 16) {
s->fifo_depth = 16;
-} else if (size <= 32) {
-s->fifo_depth = 8;
} else {
-s->fifo_depth = 4;
+s->fifo_depth = 8;
}
}
@@ -301,6 +300,17 @@ static void spi_write(void *opaque, hwaddr addr,
if (s->enabled) {
break;
}
+/*
+ * [31:6] bits are reserved bits and for future use.
+ * [5:0] are for frame size. Only [5:0] bits are validated
+ * during write, [31:6] bits are untouched.
+ */
+if ((value & FRAMESZ_MASK) > FRAMESZ_MAX) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: Incorrect size %d provided."
+ "Maximum frame size is %d\n",
+ __func__, value & FRAMESZ_MASK, FRAMESZ_MAX);
+break;
+}
s->regs[R_SPI_DFSIZE] = value;
break;
--
2.5.0
Re: [Qemu-devel] [RFC] RISC-V Decoder generator
On 10/20/2017 06:57 PM, Richard Henderson wrote: > All that said, this looks very readable. Thank you. > Thanks for your great feedback. I think most of them are not hard to implement. However I might need some time for that :) Cheers, Bastian
Re: [Qemu-devel] [libfdt][PATCH v2] implement strnlen for systems that need it
On 21 October 2017 at 00:44, Richard Henderson
wrote:
> On 10/20/2017 10:55 AM, John Arbuckle wrote:
>> +static inline size_t strnlen(const char *string, size_t max_count)
>> +{
>> +size_t count;
>> +for (count = 0; count < max_count; count++) {
>> +if (string[count] == '\0') {
>> +break;
>> +}
>> +}
>> +return count;
>
> Not to nitpick, but
>
> const char *p = memchr(string, 0, max_count);
> return p ? max_count : p - string;
Am I misreading that, or do you have the ?: arms the wrong way
around there?
thanks
-- PMM
Re: [Qemu-devel] [sw-dev] [RFC] RISC-V Decoder generator
Hi Richard, On 10/21/2017 08:44 AM, Richard W.M. Jones wrote: > On Fri, Oct 20, 2017 at 03:46:54PM +0200, Bastian Koppelmann wrote: >> I asked you for feedback some while ago regarding a modular RISC-V QEMU >> target (see discussion [1]). I tried getting it to work with the good >> old C preprocessor and quickly realized that it is too limiting. Instead >> I created a data-driven decoder generator written in python (see the >> code on github [2]) using YAML as a description language. >> >> I'd like to get some feedback whether this is acceptable to be >> upstreamed to QEMU or if you have any suggestions for improvements. >> Right now only RV32I instruction are implemented in this scheme. > > My suggestion would be to reimplement (part of) the s390x decoder > using this scheme. That would give us a direct comparison of how your > scheme is better or worse than the existing macros. Yeah that would be a great test. However I'm not sure if it's worth the effort. AFAIK s390x will not be extended with new instructions, so there is no need for a new scheme unless it helps making the code better maintainable. But that's up to the s390x maintainers. > > Will you be at the KVM Forum next week? No, sorry. I'm at the verge of graduating from University, so my time is very limited right now :( Cheers, Bastian
[Qemu-devel] PCI Passthrough + AMD + NPT
Hi All, I have started to dig into why ntp seems to slow down graphics performance on AMD systems using PCI passthrough and figured I would report what I have so far discovered. I have noted the primary point of failure seems to be specifically with PhysX. This is why people only see a slow down in certain games, not everything uses PhysX. Using FluidMark[1] the problem is immediately obvious, showing extremely low FPS on light/medium workloads with ntp enabled, and extreme fluididy and high FPS with ntp disabled. Switching nVidia to use CPU makes no difference to the performance when ntp is enabled, which seems to indicate that PhysX is falling back to CPU due to a failure of some kind to initialize. With ntp turned off, and nVidia set to use the CPU for PhysX I see an identical performance drop off in FluidMark as I see when ntp is enabled, this would seem to confirm this suspicion. Since other features such as APIC is only available if ntp is enabled, it could be something down stream of ntp that is getting disabled as a consequence of turning off ntp. It might be interesting to see if we can get some diagnostics information out of PhysX to see what if any error or debugging information it might provide when it falls back to CPU. 1: http://www.geeks3d.com/20130308/fluidmark-1-5-1-physx-benchmark-fluid-sph-simulation-opengl-download/ Kind Regards, Geoffrey McRae
Re: [Qemu-devel] [libfdt][PATCH v2] implement strnlen for systems that need it
> On Oct 22, 2017, at 9:37 AM, Peter Maydell wrote:
>
> On 21 October 2017 at 00:44, Richard Henderson
> wrote:
>> On 10/20/2017 10:55 AM, John Arbuckle wrote:
>>> +static inline size_t strnlen(const char *string, size_t max_count)
>>> +{
>>> +size_t count;
>>> +for (count = 0; count < max_count; count++) {
>>> +if (string[count] == '\0') {
>>> +break;
>>> +}
>>> +}
>>> +return count;
>>
>> Not to nitpick, but
>>
>> const char *p = memchr(string, 0, max_count);
>> return p ? max_count : p - string;
>
> Am I misreading that, or do you have the ?: arms the wrong way
> around there?
>
> thanks
> -- PMM
Yes. It should read:
return p ? p - string : max_count;
Re: [Qemu-devel] [Qemu devel v2 PATCH] msf2: Remove dead code reported by Coverity
Hi Darren,
On Wed, Oct 18, 2017 at 4:04 PM, Darren Kenny
wrote:
> Hi Sundeep,
>
>
> On Wed, Oct 18, 2017 at 10:10:07AM +, sundeep subbaraya wrote:
>
>> Hi Darren,
>>
>> On Wed, Oct 18, 2017 at 2:24 PM, Darren Kenny
>> wrote:
>>
>> On Wed, Oct 18, 2017 at 03:40:38AM +, Subbaraya Sundeep wrote:
>>>
>>> Fixed incorrect frame size mask, validated maximum frame
size in spi_write and removed dead code.
Signed-off-by: Subbaraya Sundeep
---
v2:
else if -> else in set_fifodepth
log guest error when frame size is more than 32
hw/ssi/mss-spi.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/hw/ssi/mss-spi.c b/hw/ssi/mss-spi.c
index 5a8e308..7fef2c3 100644
--- a/hw/ssi/mss-spi.c
+++ b/hw/ssi/mss-spi.c
@@ -76,9 +76,10 @@
#define C_BIGFIFO(1 << 29)
#define C_RESET (1 << 31)
-#define FRAMESZ_MASK 0x1F
+#define FRAMESZ_MASK 0x3F
#define FMCOUNT_MASK 0x0000
#define FMCOUNT_SHIFT8
+#define FRAMESZ_MAX 32
static void txfifo_reset(MSSSpiState *s)
{
@@ -104,10 +105,8 @@ static void set_fifodepth(MSSSpiState *s)
s->fifo_depth = 32;
} else if (size <= 16) {
s->fifo_depth = 16;
-} else if (size <= 32) {
-s->fifo_depth = 8;
} else {
-s->fifo_depth = 4;
+s->fifo_depth = 8;
}
}
@@ -301,6 +300,11 @@ static void spi_write(void *opaque, hwaddr addr,
if (s->enabled) {
break;
}
+if ((value & FRAMESZ_MASK) > FRAMESZ_MAX) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: Maximum frame size is
%d\n",
+ __func__, FRAMESZ_MAX);
+break;
+}
s->regs[R_SPI_DFSIZE] = value;
break;
>>> This test, and subsequent use of value appear to be out of sorts -
>>> in that while it is testing for the value by ANDing it with
>>> FRAMESZ_MASK, it is subsequently using the value without that mask
>>> applied to it, which still has the potential to be larger than
>>> FRAMESZ_MASK if it contains a value larger than 0x3F.
>>>
>>> Is that the expected behaviour? If so, maybe include a comment on
>>> it?
>>>
>>>
>> As per docs regarding [31:6]:
>> Software should not rely on the value of a reserved bit. To provide
>> compatibility with future products, the value of a reserved bit should be
>> preserved across a read-modify-write operation.
>>
>> Hence we do not care about [31:6] and validate only [5:0] for size
>> during write. When reading size we AND with FRAMESZ_MASK. In other
>> words we let [31:6] bits like scratch bits where guest can read and
>> write. I am really not sure how hardware behaves if [5:0] is
>> greater than 32 hence guest error and write wont happen. If this is
>> not right we can discuss :)
>>
>
> That sounds fine then - definitely would suggest some sort of
> comment w.r.t. the fact that we are intentionally preserving these
> extra bits - in case anyone looks at this again in the future.
>
Sure.
Thank you,
Sundeep
>
>>
>>
>>> Also, it might be useful to include the incorrect value in the
>>> logged output too, not just what the maximum is.
>>>
>>> Ok I will change.
>>>
>>
> OK
>
> Thanks,
>
> Darren.
>
Re: [Qemu-devel] [libfdt][PATCH v2] implement strnlen for systems that need it
> On Oct 22, 2017, at 1:33 AM, David Gibson wrote:
>
> On Fri, Oct 20, 2017 at 04:44:58PM -0700, Richard Henderson wrote:
>> On 10/20/2017 10:55 AM, John Arbuckle wrote:
>>> +static inline size_t strnlen(const char *string, size_t max_count)
>>> +{
>>> +size_t count;
>>> +for (count = 0; count < max_count; count++) {
>>> +if (string[count] == '\0') {
>>> +break;
>>> +}
>>> +}
>>> +return count;
>>
>> Not to nitpick, but
>>
>> const char *p = memchr(string, 0, max_count);
>> return p ? max_count : p - string;
>
> Richard's right, that's definitely a better implementation.
His implementation is smaller, but this one is even smaller. Plus it uses the
familiar strlen() function:
size_t strnlen(const char *string, size_t max_count)
{
return strlen(string) < max_count ? strlen(string) : max_count;
}
Re: [Qemu-devel] [libfdt][PATCH v2] implement strnlen for systems that need it
... this one is even smaller. Plus it uses the familiar strlen() function:
size_t strnlen(const char *string, size_t max_count)
{
return strlen(string) < max_count ? strlen(string) : max_count;
}
Please do not use that implementation.
The major goal of strnlen is to avoid looking beyond &string[max_count].
strlen(string) looks all the way to the end, which may be very much longer than
max_count;
and which may cause SIGSEGV by running into a memory page that does not exist
before the terminating '\0' is found.
[Besides, some compilers do not recognize that "strlen(string)"
need not be evaluated twice.]
--
Re: [Qemu-devel] [Qemu devel v3 PATCH] msf2: Remove dead code reported by Coverity
Hi, This series failed automatic build test. Please find the testing commands and their output below. If you have docker installed, you can probably reproduce it locally. Type: series Message-id: 1508678882-4327-1-git-send-email-sundeep.l...@gmail.com Subject: [Qemu-devel] [Qemu devel v3 PATCH] msf2: Remove dead code reported by Coverity === TEST SCRIPT BEGIN === #!/bin/bash set -e git submodule update --init dtc # Let docker tests dump environment info export SHOW_ENV=1 export J=8 time make docker-test-quick@centos6 time make docker-test-build@min-glib time make docker-test-mingw@fedora time make docker-test-block@fedora === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 From https://github.com/patchew-project/qemu * [new tag] patchew/1508678882-4327-1-git-send-email-sundeep.l...@gmail.com -> patchew/1508678882-4327-1-git-send-email-sundeep.l...@gmail.com Switched to a new branch 'test' a1c66b66e7 msf2: Remove dead code reported by Coverity === OUTPUT BEGIN === Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc' Cloning into '/var/tmp/patchew-tester-tmp-egubaaoc/src/dtc'... fatal: Could not read from remote repository. Please make sure you have the correct access rights and the repository exists. fatal: clone of 'git://git.qemu-project.org/dtc.git' into submodule path '/var/tmp/patchew-tester-tmp-egubaaoc/src/dtc' failed Failed to clone 'dtc'. Retry scheduled Cloning into '/var/tmp/patchew-tester-tmp-egubaaoc/src/dtc'... fatal: Could not read from remote repository. Please make sure you have the correct access rights and the repository exists. fatal: clone of 'git://git.qemu-project.org/dtc.git' into submodule path '/var/tmp/patchew-tester-tmp-egubaaoc/src/dtc' failed Failed to clone 'dtc' a second time, aborting === OUTPUT END === Test command exited with code: 1 --- Email generated automatically by Patchew [http://patchew.org/]. Please send your feedback to patchew-de...@freelists.org
Re: [Qemu-devel] [PULL 00/10] linux-user update for 2.11
Hi, This series seems to have some coding style problems. See output below for more information: Type: series Message-id: cover.1508329282.git.riku.voi...@linaro.org Subject: [Qemu-devel] [PULL 00/10] linux-user update for 2.11 === TEST SCRIPT BEGIN === #!/bin/bash BASE=base n=1 total=$(git log --oneline $BASE.. | wc -l) failed=0 git config --local diff.renamelimit 0 git config --local diff.renames True commits="$(git log --format=%H --reverse $BASE..)" for c in $commits; do echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..." if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then failed=1 echo fi n=$((n+1)) done exit $failed === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 From https://github.com/patchew-project/qemu t [tag update]patchew/20171018140620.25790-1-berra...@redhat.com -> patchew/20171018140620.25790-1-berra...@redhat.com Switched to a new branch 'test' 9ba15e5f0c Merge remote-tracking branch 'remotes/riku/tags/pull-linux-user-20171018' into staging e7519f2151 linux-user: Fix TARGET_MTIOCTOP/MTIOCGET/MTIOCPOS values bc68f37ffa linux-user/main: support dfilter 1ef58810fb linux-user: Fix target FS_IOC_GETFLAGS and FS_IOC_SETFLAGS numbers 63e3b96579 linux-user/sh4: Reduce TARGET_VIRT_ADDR_SPACE_BITS to 31 f4fa750ede linux-user: Tidy and enforce reserved_va initialization 70a7fe548c tcg: Fix off-by-one in assert in page_set_flags 720cebab0f linux-user: Allow -R values up to 0x for 32-bit ARM guests 68bc7fd626 linux-user: remove duplicate break in syscall 01f2b8a4e4 target/m68k,linux-user: manage FP registers in ucontext 5aca4e3e12 linux-user: fix O_TMPFILE handling === OUTPUT BEGIN === Checking PATCH 1/11: linux-user: fix O_TMPFILE handling... Checking PATCH 2/11: target/m68k,linux-user: manage FP registers in ucontext... Checking PATCH 3/11: linux-user: remove duplicate break in syscall... Checking PATCH 4/11: linux-user: Allow -R values up to 0x for 32-bit ARM guests... Checking PATCH 5/11: tcg: Fix off-by-one in assert in page_set_flags... Checking PATCH 6/11: linux-user: Tidy and enforce reserved_va initialization... Checking PATCH 7/11: linux-user/sh4: Reduce TARGET_VIRT_ADDR_SPACE_BITS to 31... Checking PATCH 8/11: linux-user: Fix target FS_IOC_GETFLAGS and FS_IOC_SETFLAGS numbers... Checking PATCH 9/11: linux-user/main: support dfilter... ERROR: space required after that ',' (ctx:VxV) #44: FILE: linux-user/main.c:4075: + "range[,...]","filter logging based on address range"}, ^ total: 1 errors, 0 warnings, 19 lines checked Your patch has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. Checking PATCH 10/11: linux-user: Fix TARGET_MTIOCTOP/MTIOCGET/MTIOCPOS values... Checking PATCH 11/11: Merge remote-tracking branch 'remotes/riku/tags/pull-linux-user-20171018' into staging... === OUTPUT END === Test command exited with code: 1 --- Email generated automatically by Patchew [http://patchew.org/]. Please send your feedback to patchew-de...@freelists.org
[Qemu-devel] Unable to clone git repository from git.qemu.org
(please keep me CC'ed, I'm not subscribed) Hi, I apologize if this is not the right mailing list for this report. I'm unable to clone the git repository from git://git.qemu.org/qemu.git (which is listed on https://www.qemu.org/contribute/). I'm getting TCP RST's when I try to clone it, as indicated by the packet capture: dualbus@ubuntu:~/src/qemu$ git clone git://git.qemu.org/qemu.git Cloning into 'qemu'... fatal: read error: Connection reset by peer dualbus@ubuntu:~/src/qemu$ git clone git://git.qemu.org/qemu.git Cloning into 'qemu'... fatal: Could not read from remote repository. Please make sure you have the correct access rights and the repository exists. --- dualbus@ubuntu:~$ sudo tcpdump -n 'host git.qemu.org' tcpdump: verbose output suppressed, use -v or -vv for full protocol decode listening on eno1, link-type EN10MB (Ethernet), capture size 262144 bytes 10:29:34.459497 IP 192.168.1.108.43168 > 172.99.69.163.9418: Flags [S], seq 1870728944, win 29200, options [mss 1460,sackOK,TS val 500213983 ecr 0,nop,wscale 7], length 0 10:29:35.487129 IP 192.168.1.108.43168 > 172.99.69.163.9418: Flags [S], seq 1870728944, win 29200, options [mss 1460,sackOK,TS val 500214240 ecr 0,nop,wscale 7], length 0 10:29:35.487854 IP 172.99.69.163.9418 > 192.168.1.108.43168: Flags [S.], seq 1436706601, ack 1870728945, win 28960, options [mss 1380,sackOK,TS val 3511630652 ecr 500213983,nop,wscale 10], length 0 10:29:35.487949 IP 192.168.1.108.43168 > 172.99.69.163.9418: Flags [.], ack 1, win 229, options [nop,nop,TS val 500214240 ecr 3511630652], length 0 10:29:35.488077 IP 192.168.1.108.43168 > 172.99.69.163.9418: Flags [P.], seq 1:49, ack 1, win 229, options [nop,nop,TS val 500214240 ecr 3511630652], length 48 10:29:36.604320 IP 172.99.69.163.9418 > 192.168.1.108.43168: Flags [S.], seq 1436706601, ack 1870728945, win 28960, options [mss 1380,sackOK,TS val 3511631678 ecr 500213983,nop,wscale 10], length 0 10:29:36.604378 IP 192.168.1.108.43168 > 172.99.69.163.9418: Flags [.], ack 1, win 229, options [nop,nop,TS val 500214519 ecr 3511630652], length 0 10:29:36.608418 IP 172.99.69.163.9418 > 192.168.1.108.43168: Flags [.], ack 49, win 29, options [nop,nop,TS val 3511631682 ecr 500214240], length 0 10:29:36.608943 IP 172.99.69.163.9418 > 192.168.1.108.43168: Flags [R.], seq 1, ack 49, win 29, options [nop,nop,TS val 0 ecr 500214240], length 0 10:29:37.553793 IP 172.99.69.163.9418 > 192.168.1.108.43168: Flags [R.], seq 1, ack 49, win 229, length 0 10:29:40.037068 IP 192.168.1.108.43170 > 172.99.69.163.9418: Flags [S], seq 4085609936, win 29200, options [mss 1460,sackOK,TS val 500215377 ecr 0,nop,wscale 7], length 0 10:29:41.038109 IP 172.99.69.163.9418 > 192.168.1.108.43170: Flags [S.], seq 1040747289, ack 4085609937, win 28960, options [mss 1380,sackOK,TS val 3511636236 ecr 500215377,nop,wscale 10], length 0 10:29:41.038218 IP 192.168.1.108.43170 > 172.99.69.163.9418: Flags [.], ack 1, win 229, options [nop,nop,TS val 500215627 ecr 3511636236], length 0 10:29:41.038398 IP 192.168.1.108.43170 > 172.99.69.163.9418: Flags [P.], seq 1:49, ack 1, win 229, options [nop,nop,TS val 500215627 ecr 3511636236], length 48 10:29:42.111622 IP 172.99.69.163.9418 > 192.168.1.108.43170: Flags [F.], seq 1, ack 1, win 29, options [nop,nop,TS val 3511637236 ecr 500215627], length 0 10:29:42.113384 IP 192.168.1.108.43170 > 172.99.69.163.9418: Flags [F.], seq 49, ack 2, win 229, options [nop,nop,TS val 500215896 ecr 3511637236], length 0 10:29:42.121555 IP 172.99.69.163.9418 > 192.168.1.108.43170: Flags [R], seq 1040747290, win 0, length 0 10:29:43.121773 IP 172.99.69.163.9418 > 192.168.1.108.43170: Flags [R.], seq 2, ack 50, win 229, length 0 ^C 18 packets captured 27 packets received by filter 0 packets dropped by kernel I was able to clone it via HTTPS, but now I'm having issues updating the git submodules dualbus@ubuntu:~/src/qemu/qemu$ git submodule update ui/keycodemapdb Cloning into '/home/dualbus/src/qemu/qemu/ui/keycodemapdb'... fatal: Could not read from remote repository. Please make sure you have the correct access rights and the repository exists. fatal: clone of 'git://git.qemu.org/keycodemapdb.git' into submodule path '/home/dualbus/src/qemu/qemu/ui/keycodemapdb' failed Failed to clone 'ui/keycodemapdb'. Retry scheduled Cloning into '/home/dualbus/src/qemu/qemu/ui/keycodemapdb'... fatal: Could not read from remote repository. Please make sure you have the correct access rights and the repository exists. fatal: clone of 'git://git.qemu.org/keycodemapdb.git' into submodule path '/home/dualbus/src/qemu/qemu/ui/keycodemapdb' failed Failed to clone 'ui/keycodemapdb' a second time, aborting signature.asc Description: PGP signature
[Qemu-devel] [PATCH] BCM2837 and machine raspi3
Dear All,
I've added support for "-M raspi3" to qemu. This is my first patch, I hope
it's okay. The github repo is here: https://github.com/bztsrc/qemu-raspi3
in case my patch does not work for some reason.
>From 1f10f957b57f336728097803bf8339a5577dd3c2 Mon Sep 17 00:00:00 2001
From: bzt
Date: Sun, 22 Oct 2017 14:59:20 +0200
Subject: [PATCH] BCM2837 and machine raspi3
Signed-off-by: bzt
---
hw/arm/Makefile.objs| 2 +-
hw/arm/bcm2835_peripherals.c| 10 ++-
hw/arm/bcm2836.c| 6 --
hw/arm/bcm2837.c| 179
hw/arm/raspi.c | 79 --
include/hw/arm/bcm2836.h| 6 ++
include/hw/arm/bcm2837.h| 19 +
include/hw/arm/raspi_platform.h | 2 +-
8 files changed, 287 insertions(+), 16 deletions(-)
create mode 100644 hw/arm/bcm2837.c
create mode 100644 include/hw/arm/bcm2837.h
diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
index 2794e08..72b60e1 100644
--- a/hw/arm/Makefile.objs
+++ b/hw/arm/Makefile.objs
@@ -11,7 +11,7 @@ obj-y += armv7m.o exynos4210.o pxa2xx.o pxa2xx_gpio.o
pxa2xx_pic.o
obj-$(CONFIG_DIGIC) += digic.o
obj-y += omap1.o omap2.o strongarm.o
obj-$(CONFIG_ALLWINNER_A10) += allwinner-a10.o cubieboard.o
-obj-$(CONFIG_RASPI) += bcm2835_peripherals.o bcm2836.o raspi.o
+obj-$(CONFIG_RASPI) += bcm2835_peripherals.o bcm2836.o bcm2837.o raspi.o
obj-$(CONFIG_STM32F205_SOC) += stm32f205_soc.o
obj-$(CONFIG_XLNX_ZYNQMP) += xlnx-zynqmp.o xlnx-zcu102.o
obj-$(CONFIG_FSL_IMX25) += fsl-imx25.o imx25_pdk.o
diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c
index 12e0dd1..f79ce36 100644
--- a/hw/arm/bcm2835_peripherals.c
+++ b/hw/arm/bcm2835_peripherals.c
@@ -212,7 +212,15 @@ static void bcm2835_peripherals_realize(DeviceState
*dev, Error **errp)
error_propagate(errp, err);
return;
}
-
+// check if parameters are valid
+if (ram_size < vcram_size + 64*1024*1024) {
+error_setg(errp, "%s: not enough ram for VideoCore",
+ __func__);
+return;
+}
+// if vcram_size is bigger than ram_size, this will silently overflow
+// and generate a not very informative "Parameter 'vcram-base' expects
+// uint32_t" message...
object_property_set_uint(OBJECT(&s->fb), ram_size - vcram_size,
"vcram-base", &err);
if (err) {
diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
index 8c43291..db40c8e 100644
--- a/hw/arm/bcm2836.c
+++ b/hw/arm/bcm2836.c
@@ -17,12 +17,6 @@
#include "hw/sysbus.h"
#include "exec/address-spaces.h"
-/* Peripheral base address seen by the CPU */
-#define BCM2836_PERI_BASE 0x3F00
-
-/* "QA7" (Pi2) interrupt controller and mailboxes etc. */
-#define BCM2836_CONTROL_BASE0x4000
-
static void bcm2836_init(Object *obj)
{
BCM2836State *s = BCM2836(obj);
diff --git a/hw/arm/bcm2837.c b/hw/arm/bcm2837.c
new file mode 100644
index 000..1bab93b
--- /dev/null
+++ b/hw/arm/bcm2837.c
@@ -0,0 +1,179 @@
+/*
+ * Raspberry Pi emulation (c) 2012 Gregory Estrade
+ * Upstreaming code cleanup [including bcm2835_*] (c) 2013 Jan Petrous
+ *
+ * Rasperry Pi 2 emulation and refactoring Copyright (c) 2015, Microsoft
+ * Written by Andrew Baumann
+ *
+ * Raspberry Pi 3 emulation 2017 by bzt
+ *
+ * This code is licensed under the GNU GPLv2 and later.
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qemu-common.h"
+#include "cpu.h"
+#include "hw/arm/bcm2836.h"
+#include "hw/arm/bcm2837.h"
+#include "hw/arm/raspi_platform.h"
+#include "hw/sysbus.h"
+#include "exec/address-spaces.h"
+
+/* According to
https://www.raspberrypi.org/documentation/hardware/raspberrypi/bcm2837/README.md
+ * The underlying architecture of the BCM2837 is identical to the BCM2836.
The only significant
+ * difference is the replacement of the ARMv7 quad core cluster with a
quad-core ARM Cortex A53
+ * (ARMv8) cluster. So we use cortex-a53- here. */
+
+static void bcm2837_init(Object *obj)
+{
+BCM2836State *s = BCM2837(obj);
+int n;
+
+for (n = 0; n < BCM2836_NCPUS; n++) {
+object_initialize(&s->cpus[n], sizeof(s->cpus[n]),
+ "cortex-a53-" TYPE_ARM_CPU);
+object_property_add_child(obj, "cpu[*]", OBJECT(&s->cpus[n]),
+ &error_abort);
+}
+
+object_initialize(&s->control, sizeof(s->control),
TYPE_BCM2836_CONTROL);
+object_property_add_child(obj, "control", OBJECT(&s->control), NULL);
+qdev_set_parent_bus(DEVICE(&s->control), sysbus_get_default());
+
+object_initialize(&s->peripherals, sizeof(s->peripherals),
+ TYPE_BCM2835_PERIPHERALS);
+object_property_add_child(obj, "peripherals", OBJECT(&s->peripherals),
+ &error_abort);
+object_property_add_alias(obj, "board-rev", OBJECT(&s->peripherals),
+ "board-rev", &error_abort);
+object_property_
Re: [Qemu-devel] [PATCH v2 1/1] virtio_balloon: include buffers and cached memory statistics
On Thu, 19 Oct 2017 16:12:20 +0300
"Michael S. Tsirkin" wrote:
> On Thu, Sep 21, 2017 at 02:55:41PM +0200, Tomáš Golembiovský wrote:
> > Add a new fields, VIRTIO_BALLOON_S_BUFFERS and VIRTIO_BALLOON_S_CACHED,
> > to virtio_balloon memory statistics protocol. The values correspond to
> > 'Buffers' and 'Cached' in /proc/meminfo.
> >
> > To be able to compute the value of 'Cached' memory it is necessary to
> > export total_swapcache_pages() to modules.
> >
> > Signed-off-by: Tomáš Golembiovský
>
> Does 'Buffers' actually make sense? It's a temporary storage -
> wouldn't it be significantly out of date by the time
> host receives it?
That would be best answered by somebody from kernel. But my personal
opinion is that it would not be out of date. The amount of memory
dedicated to Buffers does not seem to fluctuate too much.
Tomas
> > ---
> > drivers/virtio/virtio_balloon.c | 11 +++
> > include/uapi/linux/virtio_balloon.h | 4 +++-
> > mm/swap_state.c | 1 +
> > 3 files changed, 15 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/virtio/virtio_balloon.c
> > b/drivers/virtio/virtio_balloon.c
> > index f0b3a0b9d42f..c2558ec47a62 100644
> > --- a/drivers/virtio/virtio_balloon.c
> > +++ b/drivers/virtio/virtio_balloon.c
> > @@ -244,12 +244,19 @@ static unsigned int update_balloon_stats(struct
> > virtio_balloon *vb)
> > struct sysinfo i;
> > unsigned int idx = 0;
> > long available;
> > + long cached;
> >
> > all_vm_events(events);
> > si_meminfo(&i);
> >
> > available = si_mem_available();
> >
> > + cached = global_node_page_state(NR_FILE_PAGES) -
> > + total_swapcache_pages() - i.bufferram;
> > + if (cached < 0)
> > + cached = 0;
> > +
> > +
> > #ifdef CONFIG_VM_EVENT_COUNTERS
> > update_stat(vb, idx++, VIRTIO_BALLOON_S_SWAP_IN,
> > pages_to_bytes(events[PSWPIN]));
> > @@ -264,6 +271,10 @@ static unsigned int update_balloon_stats(struct
> > virtio_balloon *vb)
> > pages_to_bytes(i.totalram));
> > update_stat(vb, idx++, VIRTIO_BALLOON_S_AVAIL,
> > pages_to_bytes(available));
> > + update_stat(vb, idx++, VIRTIO_BALLOON_S_BUFFERS,
> > + pages_to_bytes(i.bufferram));
> > + update_stat(vb, idx++, VIRTIO_BALLOON_S_CACHED,
> > + pages_to_bytes(cached));
> >
> > return idx;
> > }
> > diff --git a/include/uapi/linux/virtio_balloon.h
> > b/include/uapi/linux/virtio_balloon.h
> > index 343d7ddefe04..d5dc8a56a497 100644
> > --- a/include/uapi/linux/virtio_balloon.h
> > +++ b/include/uapi/linux/virtio_balloon.h
> > @@ -52,7 +52,9 @@ struct virtio_balloon_config {
> > #define VIRTIO_BALLOON_S_MEMFREE 4 /* Total amount of free memory */
> > #define VIRTIO_BALLOON_S_MEMTOT 5 /* Total amount of memory */
> > #define VIRTIO_BALLOON_S_AVAIL6 /* Available memory as in /proc */
> > -#define VIRTIO_BALLOON_S_NR 7
> > +#define VIRTIO_BALLOON_S_BUFFERS 7 /* Buffers memory as in /proc */
> > +#define VIRTIO_BALLOON_S_CACHED 8 /* Cached memory as in /proc */
> > +#define VIRTIO_BALLOON_S_NR 9
> >
> > /*
> > * Memory statistics structure.
> > diff --git a/mm/swap_state.c b/mm/swap_state.c
> > index 71ce2d1ccbf7..f3a4ff7d6c52 100644
> > --- a/mm/swap_state.c
> > +++ b/mm/swap_state.c
> > @@ -95,6 +95,7 @@ unsigned long total_swapcache_pages(void)
> > rcu_read_unlock();
> > return ret;
> > }
> > +EXPORT_SYMBOL_GPL(total_swapcache_pages);
> >
> > static atomic_t swapin_readahead_hits = ATOMIC_INIT(4);
>
> Need an ack from MM crowd on that.
>
> > --
> > 2.14.1
--
Tomáš Golembiovský
Re: [Qemu-devel] [Qemu devel v3 PATCH] msf2: Remove dead code reported by Coverity
On Sun, Oct 22, 2017 at 06:58:02PM +0530, Subbaraya Sundeep wrote:
Fixed incorrect frame size mask, validated maximum frame
size in spi_write and removed dead code.
Signed-off-by: Subbaraya Sundeep
---
v3:
Added comment that [31:6] bits are reserved in R_SPI_DFSIZE
register and logged incorrect value too in guest error(suggested
by Darren).
v2:
else if -> else in set_fifodepth
log guest error when frame size is more than 32
hw/ssi/mss-spi.c | 18 ++
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/hw/ssi/mss-spi.c b/hw/ssi/mss-spi.c
index 5a8e308..e1b6227 100644
--- a/hw/ssi/mss-spi.c
+++ b/hw/ssi/mss-spi.c
@@ -76,9 +76,10 @@
#define C_BIGFIFO(1 << 29)
#define C_RESET (1 << 31)
-#define FRAMESZ_MASK 0x1F
+#define FRAMESZ_MASK 0x3F
#define FMCOUNT_MASK 0x0000
#define FMCOUNT_SHIFT8
+#define FRAMESZ_MAX 32
static void txfifo_reset(MSSSpiState *s)
{
@@ -104,10 +105,8 @@ static void set_fifodepth(MSSSpiState *s)
s->fifo_depth = 32;
} else if (size <= 16) {
s->fifo_depth = 16;
-} else if (size <= 32) {
-s->fifo_depth = 8;
} else {
-s->fifo_depth = 4;
+s->fifo_depth = 8;
}
}
@@ -301,6 +300,17 @@ static void spi_write(void *opaque, hwaddr addr,
if (s->enabled) {
break;
}
+/*
+ * [31:6] bits are reserved bits and for future use.
+ * [5:0] are for frame size. Only [5:0] bits are validated
+ * during write, [31:6] bits are untouched.
+ */
+if ((value & FRAMESZ_MASK) > FRAMESZ_MAX) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: Incorrect size %d provided."
+ "Maximum frame size is %d\n",
+ __func__, value & FRAMESZ_MASK, FRAMESZ_MAX);
Nitpicking here really, but shouldn't the %d be %u since value is a uint32_t?
Thanks,
Darren.
+break;
+}
s->regs[R_SPI_DFSIZE] = value;
break;
--
2.5.0
Re: [Qemu-devel] [libfdt][PATCH v2] implement strnlen for systems that need it
On Sun, 2017-10-22 at 10:41 -0400, Programmingkid wrote:
> >
> > On Oct 22, 2017, at 1:33 AM, David Gibson wrote:
> >
> > On Fri, Oct 20, 2017 at 04:44:58PM -0700, Richard Henderson wrote:
> > >
> > > On 10/20/2017 10:55 AM, John Arbuckle wrote:
> > > >
> > > > +static inline size_t strnlen(const char *string, size_t max_count)
> > > > +{
> > > > +size_t count;
> > > > +for (count = 0; count < max_count; count++) {
> > > > +if (string[count] == '\0') {
> > > > +break;
> > > > +}
> > > > +}
> > > > +return count;
> > > Not to nitpick, but
> > >
> > > const char *p = memchr(string, 0, max_count);
> > > return p ? max_count : p - string;
> > Richard's right, that's definitely a better implementation.
> His implementation is smaller, but this one is even smaller. Plus it uses the
> familiar strlen() function:
>
> size_t strnlen(const char *string, size_t max_count)
> {
> return strlen(string) < max_count ? strlen(string) : max_count;
> }
That is not a proper implementation of strnlen(), which is not supposed
to access any source-string bytes beyond max_count.
-- Ian
Re: [Qemu-devel] [libfdt][PATCH v2] implement strnlen for systems that need it
> On Oct 22, 2017, at 3:06 PM, Ian Lepore wrote:
>
> On Sun, 2017-10-22 at 10:41 -0400, Programmingkid wrote:
>>>
>>> On Oct 22, 2017, at 1:33 AM, David Gibson wrote:
>>>
>>> On Fri, Oct 20, 2017 at 04:44:58PM -0700, Richard Henderson wrote:
On 10/20/2017 10:55 AM, John Arbuckle wrote:
>
> +static inline size_t strnlen(const char *string, size_t max_count)
> +{
> +size_t count;
> +for (count = 0; count < max_count; count++) {
> +if (string[count] == '\0') {
> +break;
> +}
> +}
> +return count;
Not to nitpick, but
const char *p = memchr(string, 0, max_count);
return p ? max_count : p - string;
>>> Richard's right, that's definitely a better implementation.
>> His implementation is smaller, but this one is even smaller. Plus it uses
>> the familiar strlen() function:
>>
>> size_t strnlen(const char *string, size_t max_count)
>> {
>> return strlen(string) < max_count ? strlen(string) : max_count;
>> }
>
> That is not a proper implementation of strnlen(), which is not supposed
> to access any source-string bytes beyond max_count.
>
> -- Ian
http://pubs.opengroup.org/onlinepubs/9699919799/functions/strlen.html
This specification document should help anyone who wants more info.
The first implementation using the loop would never access anything beyond
max_count. My second implementation does go beyond max_count. The
implementation using memchr() will probably live up the requirement so I guess
it wins.
Thank you Ian for this information.
Re: [Qemu-devel] [PATCH] ACPI/unit-test: Add a testcase for RAM allocation in numa node
Hi Eduardo,
Thanks for your reply.
At 10/21/2017 03:15 AM, Eduardo Habkost wrote:
On Wed, Oct 18, 2017 at 11:50:32AM +0800, Dou Liyang wrote:
At 10/18/2017 11:48 AM, Dou Liyang wrote:
As QEMU supports the memory-less node, it is possible that there is
no RAM in the first numa node(also be called as node0). eg:
... \
-m 128,slots=3,maxmem=1G \
-numa node -numa node,mem=128M \
But, this makes it hard for QEMU to build a known-to-work ACPI SRAT
table. Only fixing it is not enough.
Add a testcase for this situation to make sure the ACPI table is
correct for guest.
Suggested-by: Eduardo Habkost
Signed-off-by: Dou Liyang
---
Changelog:
This patch can pass the 'make check' in Peter's machine,
Oops, s/can/can not :-).
Thanks,
dou.
But, in my own and Eduardo's machine, we can pass it.
So I rebase and spilt it independently.
Thanks!
Peter:
could you help me test it in your machine.
then, give me the output files, such as /tmp/asl-6QYK7Y.dsl and
/tmp/asl-1H1I7Y.dsl file.
As the failures reported by Peter were about the *.memhp test
case files, I think this patch is unlikely to be the cause of
those failures. I believe the problem is at:
From: Dou Liyang
Date: Fri, 1 Sep 2017 10:10:03 +0800
Message-ID: <1504231805-30957-3-git-send-email-douly.f...@cn.fujitsu.com>
Subject: [PATCH v6 2/4] hw/acpi-build: Make assignment statement
of next_base easy to read
This patch is just used for readability. so I discard it. and, we
need a test case for the commit
4926403c250f ("hw/acpi-build: Fix SRAT memory building in case of
node 0 without RAM")
So, I re-post this patch. ;-)
However, I would still like to get test results from Peter before
applying this patch again.
I see.
Thanks,
dou
Re: [Qemu-devel] Unable to clone git repository from git.qemu.org
On 22/10/2017 18:13, Eduardo A. Bustamante López wrote: > (please keep me CC'ed, I'm not subscribed) > > Hi, > > I apologize if this is not the right mailing list for this report. I'm unable > to clone the git repository from > git://git.qemu.org/qemu.git (which is listed on > https://www.qemu.org/contribute/). > > I'm getting TCP RST's when I try to clone it, as indicated by the packet > capture: > > dualbus@ubuntu:~/src/qemu$ git clone git://git.qemu.org/qemu.git > Cloning into 'qemu'... > fatal: read error: Connection reset by peer > > dualbus@ubuntu:~/src/qemu$ git clone git://git.qemu.org/qemu.git > Cloning into 'qemu'... > fatal: Could not read from remote repository. > > Please make sure you have the correct access rights > and the repository exists. Thanks for your report, we will look into it soon. In the meanwhile, you can use the github mirror: git clone git://github.com/qemu/qemu Paolo > > --- > > dualbus@ubuntu:~$ sudo tcpdump -n 'host git.qemu.org' > tcpdump: verbose output suppressed, use -v or -vv for full protocol decode > listening on eno1, link-type EN10MB (Ethernet), capture size 262144 bytes > 10:29:34.459497 IP 192.168.1.108.43168 > 172.99.69.163.9418: Flags [S], seq > 1870728944, win 29200, options [mss 1460,sackOK,TS val 500213983 ecr > 0,nop,wscale 7], length 0 > 10:29:35.487129 IP 192.168.1.108.43168 > 172.99.69.163.9418: Flags [S], seq > 1870728944, win 29200, options [mss 1460,sackOK,TS val 500214240 ecr > 0,nop,wscale 7], length 0 > 10:29:35.487854 IP 172.99.69.163.9418 > 192.168.1.108.43168: Flags [S.], > seq 1436706601, ack 1870728945, win 28960, options [mss 1380,sackOK,TS val > 3511630652 ecr 500213983,nop,wscale 10], length 0 > 10:29:35.487949 IP 192.168.1.108.43168 > 172.99.69.163.9418: Flags [.], ack > 1, win 229, options [nop,nop,TS val 500214240 ecr 3511630652], length 0 > 10:29:35.488077 IP 192.168.1.108.43168 > 172.99.69.163.9418: Flags [P.], > seq 1:49, ack 1, win 229, options [nop,nop,TS val 500214240 ecr 3511630652], > length 48 > 10:29:36.604320 IP 172.99.69.163.9418 > 192.168.1.108.43168: Flags [S.], > seq 1436706601, ack 1870728945, win 28960, options [mss 1380,sackOK,TS val > 3511631678 ecr 500213983,nop,wscale 10], length 0 > 10:29:36.604378 IP 192.168.1.108.43168 > 172.99.69.163.9418: Flags [.], ack > 1, win 229, options [nop,nop,TS val 500214519 ecr 3511630652], length 0 > 10:29:36.608418 IP 172.99.69.163.9418 > 192.168.1.108.43168: Flags [.], ack > 49, win 29, options [nop,nop,TS val 3511631682 ecr 500214240], length 0 > 10:29:36.608943 IP 172.99.69.163.9418 > 192.168.1.108.43168: Flags [R.], > seq 1, ack 49, win 29, options [nop,nop,TS val 0 ecr 500214240], length 0 > 10:29:37.553793 IP 172.99.69.163.9418 > 192.168.1.108.43168: Flags [R.], > seq 1, ack 49, win 229, length 0 > 10:29:40.037068 IP 192.168.1.108.43170 > 172.99.69.163.9418: Flags [S], seq > 4085609936, win 29200, options [mss 1460,sackOK,TS val 500215377 ecr > 0,nop,wscale 7], length 0 > 10:29:41.038109 IP 172.99.69.163.9418 > 192.168.1.108.43170: Flags [S.], > seq 1040747289, ack 4085609937, win 28960, options [mss 1380,sackOK,TS val > 3511636236 ecr 500215377,nop,wscale 10], length 0 > 10:29:41.038218 IP 192.168.1.108.43170 > 172.99.69.163.9418: Flags [.], ack > 1, win 229, options [nop,nop,TS val 500215627 ecr 3511636236], length 0 > 10:29:41.038398 IP 192.168.1.108.43170 > 172.99.69.163.9418: Flags [P.], > seq 1:49, ack 1, win 229, options [nop,nop,TS val 500215627 ecr 3511636236], > length 48 > 10:29:42.111622 IP 172.99.69.163.9418 > 192.168.1.108.43170: Flags [F.], > seq 1, ack 1, win 29, options [nop,nop,TS val 3511637236 ecr 500215627], > length 0 > 10:29:42.113384 IP 192.168.1.108.43170 > 172.99.69.163.9418: Flags [F.], > seq 49, ack 2, win 229, options [nop,nop,TS val 500215896 ecr 3511637236], > length 0 > 10:29:42.121555 IP 172.99.69.163.9418 > 192.168.1.108.43170: Flags [R], seq > 1040747290, win 0, length 0 > 10:29:43.121773 IP 172.99.69.163.9418 > 192.168.1.108.43170: Flags [R.], > seq 2, ack 50, win 229, length 0 > ^C > 18 packets captured > 27 packets received by filter > 0 packets dropped by kernel > > > I was able to clone it via HTTPS, but now I'm having issues updating the git > submodules > > dualbus@ubuntu:~/src/qemu/qemu$ git submodule update ui/keycodemapdb > Cloning into '/home/dualbus/src/qemu/qemu/ui/keycodemapdb'... > fatal: Could not read from remote repository. > > Please make sure you have the correct access rights > and the repository exists. > fatal: clone of 'git://git.qemu.org/keycodemapdb.git' into submodule path > '/home/dualbus/src/qemu/qemu/ui/keycodemapdb' > failed > Failed to clone 'ui/keycodemapdb'. Retry scheduled > Cloning into '/home/dualbus/src/qemu/qemu/ui/keycodemapdb'... > fatal: Could not read from remote repository. > > Please make sure you have the correct access rights > and the repository e
[Qemu-devel] [PATCH v4] NUMA: Enable adding NUMA node implicitly
Linux and Windows need ACPI SRAT table to make memory hotplug work properly,
however currently QEMU doesn't create SRAT table if numa options aren't present
on CLI.
Which breaks both linux and windows guests in certain conditions:
* Windows: won't enable memory hotplug without SRAT table at all
* Linux: if QEMU is started with initial memory all below 4Gb and no SRAT table
present, guest kernel will use nommu DMA ops, which breaks 32bit hw drivers
when memory is hotplugged and guest tries to use it with that drivers.
Fix above issues by automatically creating a numa node when QEMU is started with
memory hotplug enabled but without '-numa' options on CLI.
(PS: auto-create numa node only for new machine types so not to break
migration).
Which would provide SRAT table to guests without explicit -numa options on CLI
and would allow:
* Windows: to enable memory hotplug
* Linux: switch to SWIOTLB DMA ops, to bounce DMA transfers to 32bit allocated
buffers that legacy drivers/hw can handle.
[Rewritten by Igor]
Reported-by: Thadeu Lima de Souza Cascardo
Suggested-by: Igor Mammedov
Signed-off-by: Dou Liyang
Cc: Paolo Bonzini
Cc: Richard Henderson
Cc: Eduardo Habkost
Cc: "Michael S. Tsirkin"
Cc: Marcel Apfelbaum
Cc: Igor Mammedov
Cc: David Hildenbrand
Cc: Thomas Huth
Cc: Alistair Francis
Cc: f4...@amsat.org
Cc: Takao Indoh
Cc: Izumi Taku
---
changelog V3 --> V4:
-Move the parse_numa_opts behind the setup of current_machine
for safety suggested by Igor
hw/i386/pc.c | 1 +
hw/i386/pc_piix.c | 1 +
hw/i386/pc_q35.c | 1 +
include/hw/boards.h | 1 +
include/sysemu/numa.h | 2 +-
numa.c| 24 ++--
vl.c | 3 +--
7 files changed, 28 insertions(+), 5 deletions(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 8e307f7..ec4eb97 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -2325,6 +2325,7 @@ static void pc_machine_class_init(ObjectClass *oc, void
*data)
mc->cpu_index_to_instance_props = pc_cpu_index_to_props;
mc->get_default_cpu_node_id = pc_get_default_cpu_node_id;
mc->possible_cpu_arch_ids = pc_possible_cpu_arch_ids;
+mc->auto_enable_numa_with_memhp = true;
mc->has_hotpluggable_cpus = true;
mc->default_boot_order = "cad";
mc->hot_add_cpu = pc_hot_add_cpu;
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index f79d5cb..5e47528 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -446,6 +446,7 @@ static void pc_i440fx_2_10_machine_options(MachineClass *m)
m->is_default = 0;
m->alias = NULL;
SET_MACHINE_COMPAT(m, PC_COMPAT_2_10);
+m->auto_enable_numa_with_memhp = false;
}
DEFINE_I440FX_MACHINE(v2_10, "pc-i440fx-2.10", NULL,
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index da3ea60..d606004 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -318,6 +318,7 @@ static void pc_q35_2_10_machine_options(MachineClass *m)
m->alias = NULL;
SET_MACHINE_COMPAT(m, PC_COMPAT_2_10);
m->numa_auto_assign_ram = numa_legacy_auto_assign_ram;
+m->auto_enable_numa_with_memhp = false;
}
DEFINE_Q35_MACHINE(v2_10, "pc-q35-2.10", NULL,
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 191a5b3..f1077f1 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -192,6 +192,7 @@ struct MachineClass {
bool ignore_memory_transaction_failures;
int numa_mem_align_shift;
const char **valid_cpu_types;
+bool auto_enable_numa_with_memhp;
void (*numa_auto_assign_ram)(MachineClass *mc, NodeInfo *nodes,
int nb_nodes, ram_addr_t size);
diff --git a/include/sysemu/numa.h b/include/sysemu/numa.h
index 5c6df28..31d3ac0 100644
--- a/include/sysemu/numa.h
+++ b/include/sysemu/numa.h
@@ -30,7 +30,7 @@ struct NumaNodeMem {
};
extern NodeInfo numa_info[MAX_NODES];
-void parse_numa_opts(MachineState *ms);
+void parse_numa_opts(MachineState *ms, uint64_t ram_slots);
void query_numa_node_mem(NumaNodeMem node_mem[]);
extern QemuOptsList qemu_numa_opts;
void numa_set_mem_node_id(ram_addr_t addr, uint64_t size, uint32_t node);
diff --git a/numa.c b/numa.c
index 100a67f..ba8d813 100644
--- a/numa.c
+++ b/numa.c
@@ -423,12 +423,32 @@ void numa_default_auto_assign_ram(MachineClass *mc,
NodeInfo *nodes,
nodes[i].node_mem = size - usedmem;
}
-void parse_numa_opts(MachineState *ms)
+void parse_numa_opts(MachineState *ms, uint64_t ram_slots)
{
int i;
MachineClass *mc = MACHINE_GET_CLASS(ms);
+QemuOptsList *numa_opts = qemu_find_opts("numa");
-if (qemu_opts_foreach(qemu_find_opts("numa"), parse_numa, ms, NULL)) {
+/*
+ * If memory hotplug is enabled (slots > 0) but without '-numa'
+ * options explicitly on CLI, guestes will break.
+ *
+ * Windows: won't enable memory hotplug without SRAT table at all
+ *
+ * Linux: if QEMU is started with initial memory all below 4Gb
+ * and no SRAT table present, guest kernel will use nommu DMA
Re: [Qemu-devel] [PATCH v4] NUMA: Enable adding NUMA node implicitly
Hi, This series failed automatic build test. Please find the testing commands and their output below. If you have docker installed, you can probably reproduce it locally. Type: series Message-id: 1508722422-3861-1-git-send-email-douly.f...@cn.fujitsu.com Subject: [Qemu-devel] [PATCH v4] NUMA: Enable adding NUMA node implicitly === TEST SCRIPT BEGIN === #!/bin/bash set -e git submodule update --init dtc # Let docker tests dump environment info export SHOW_ENV=1 export J=8 time make docker-test-quick@centos6 time make docker-test-build@min-glib time make docker-test-mingw@fedora time make docker-test-block@fedora === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 From https://github.com/patchew-project/qemu * [new tag] patchew/1508722422-3861-1-git-send-email-douly.f...@cn.fujitsu.com -> patchew/1508722422-3861-1-git-send-email-douly.f...@cn.fujitsu.com t [tag update]patchew/cover.1508329282.git.riku.voi...@linaro.org -> patchew/cover.1508329282.git.riku.voi...@linaro.org Switched to a new branch 'test' b8c0d922bf NUMA: Enable adding NUMA node implicitly === OUTPUT BEGIN === Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc' Cloning into '/var/tmp/patchew-tester-tmp-z9ao6big/src/dtc'... fatal: Could not read from remote repository. Please make sure you have the correct access rights and the repository exists. fatal: clone of 'git://git.qemu-project.org/dtc.git' into submodule path '/var/tmp/patchew-tester-tmp-z9ao6big/src/dtc' failed Failed to clone 'dtc'. Retry scheduled Cloning into '/var/tmp/patchew-tester-tmp-z9ao6big/src/dtc'... fatal: Could not read from remote repository. Please make sure you have the correct access rights and the repository exists. fatal: clone of 'git://git.qemu-project.org/dtc.git' into submodule path '/var/tmp/patchew-tester-tmp-z9ao6big/src/dtc' failed Failed to clone 'dtc' a second time, aborting === OUTPUT END === Test command exited with code: 1 --- Email generated automatically by Patchew [http://patchew.org/]. Please send your feedback to patchew-de...@freelists.org
Re: [Qemu-devel] [PATCH v4] NUMA: Enable adding NUMA node implicitly
Hi Fam, At 10/23/2017 09:37 AM, no-re...@patchew.org wrote: Hi, This series failed automatic build test. Please find the testing commands and their output below. If you have docker installed, you can probably reproduce it locally. Type: series Message-id: 1508722422-3861-1-git-send-email-douly.f...@cn.fujitsu.com Subject: [Qemu-devel] [PATCH v4] NUMA: Enable adding NUMA node implicitly === TEST SCRIPT BEGIN === #!/bin/bash set -e git submodule update --init dtc # Let docker tests dump environment info export SHOW_ENV=1 export J=8 time make docker-test-quick@centos6 time make docker-test-build@min-glib time make docker-test-mingw@fedora time make docker-test-block@fedora === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 From https://github.com/patchew-project/qemu * [new tag] patchew/1508722422-3861-1-git-send-email-douly.f...@cn.fujitsu.com -> patchew/1508722422-3861-1-git-send-email-douly.f...@cn.fujitsu.com t [tag update]patchew/cover.1508329282.git.riku.voi...@linaro.org -> patchew/cover.1508329282.git.riku.voi...@linaro.org Switched to a new branch 'test' b8c0d922bf NUMA: Enable adding NUMA node implicitly === OUTPUT BEGIN === Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc' Cloning into '/var/tmp/patchew-tester-tmp-z9ao6big/src/dtc'... fatal: Could not read from remote repository. Wow, It seems there are something wrong with the test robot. Thanks, dou. Please make sure you have the correct access rights and the repository exists. fatal: clone of 'git://git.qemu-project.org/dtc.git' into submodule path '/var/tmp/patchew-tester-tmp-z9ao6big/src/dtc' failed Failed to clone 'dtc'. Retry scheduled Cloning into '/var/tmp/patchew-tester-tmp-z9ao6big/src/dtc'... fatal: Could not read from remote repository. Please make sure you have the correct access rights and the repository exists. fatal: clone of 'git://git.qemu-project.org/dtc.git' into submodule path '/var/tmp/patchew-tester-tmp-z9ao6big/src/dtc' failed Failed to clone 'dtc' a second time, aborting === OUTPUT END === Test command exited with code: 1 --- Email generated automatically by Patchew [http://patchew.org/]. Please send your feedback to patchew-de...@freelists.org
Re: [Qemu-devel] [PATCH v4] NUMA: Enable adding NUMA node implicitly
Hi, Fam At 10/23/2017 09:37 AM, no-re...@patchew.org wrote: Hi, This series failed build test on s390x host. Please find the details below. Type: series Message-id: 1508722422-3861-1-git-send-email-douly.f...@cn.fujitsu.com Subject: [Qemu-devel] [PATCH v4] NUMA: Enable adding NUMA node implicitly GEN hw/pci/trace.h GEN hw/s390x/trace.h GEN hw/vfio/trace.h GEN hw/acpi/trace.h GEN hw/arm/trace.h GEN hw/alpha/trace.h Makefile:30: recipe for target 'git-submodule-update' failed make: *** [git-submodule-update] Error 1 It also seems that there are something mistake in our test machines. Thanks, dou. make: *** Waiting for unfinished jobs === OUTPUT END === Test command exited with code: 2 --- Email generated automatically by Patchew [http://patchew.org/]. Please send your feedback to patchew-de...@freelists.org
[Qemu-devel] [libfdt][PATCH v3] implement strnlen for systems that need it
Prior the Mac OS 10.7, the function strnlen() was not available. This patch
implements strnlen() on Mac OS X versions that are below 10.7.
Signed-off-by: John Arbuckle
---
v3 changes:
- Replaced loop with memchr()
v2 changes:
- Simplified the code to make it static inline'ed
- Changed the type of count to size_t
libfdt/libfdt_env.h | 24
1 file changed, 24 insertions(+)
diff --git a/libfdt/libfdt_env.h b/libfdt/libfdt_env.h
index 952056c..d43b66b 100644
--- a/libfdt/libfdt_env.h
+++ b/libfdt/libfdt_env.h
@@ -109,4 +109,28 @@ static inline fdt64_t cpu_to_fdt64(uint64_t x)
#undef CPU_TO_FDT16
#undef EXTRACT_BYTE
+#ifdef __APPLE__
+#include
+
+#define MAC_OS_X_VERSION_10_7 1070
+
+/* strnlen() is not available on Mac OS < 10.7 */
+# if (MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_7)
+
+/*
+ * strnlen: returns the length of a string or max_count - which ever is
smallest
+ * Input 1 string: the string whose size is to be determined
+ * Input 2 max_count: the maximum value returned by this function
+ * Output: length of the string or max_count (the smallest of the two)
+ */
+static inline size_t strnlen(const char *string, size_t max_count)
+{
+const char *p = memchr(string, 0, max_count);
+return p ? p - string : max_count;
+}
+
+#endif /* (MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_7) */
+
+#endif /* __APPLE__ */
+
#endif /* _LIBFDT_ENV_H */
--
2.10.2
Re: [Qemu-devel] [Qemu devel v3 PATCH] msf2: Remove dead code reported by Coverity
Hi Darren,
On Mon, Oct 23, 2017 at 12:18 AM, Darren Kenny
wrote:
> On Sun, Oct 22, 2017 at 06:58:02PM +0530, Subbaraya Sundeep wrote:
>
>> Fixed incorrect frame size mask, validated maximum frame
>> size in spi_write and removed dead code.
>>
>> Signed-off-by: Subbaraya Sundeep
>> ---
>> v3:
>> Added comment that [31:6] bits are reserved in R_SPI_DFSIZE
>> register and logged incorrect value too in guest error(suggested
>>by Darren).
>> v2:
>>else if -> else in set_fifodepth
>>log guest error when frame size is more than 32
>>
>> hw/ssi/mss-spi.c | 18 ++
>> 1 file changed, 14 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/ssi/mss-spi.c b/hw/ssi/mss-spi.c
>> index 5a8e308..e1b6227 100644
>> --- a/hw/ssi/mss-spi.c
>> +++ b/hw/ssi/mss-spi.c
>> @@ -76,9 +76,10 @@
>> #define C_BIGFIFO(1 << 29)
>> #define C_RESET (1 << 31)
>>
>> -#define FRAMESZ_MASK 0x1F
>> +#define FRAMESZ_MASK 0x3F
>> #define FMCOUNT_MASK 0x0000
>> #define FMCOUNT_SHIFT8
>> +#define FRAMESZ_MAX 32
>>
>> static void txfifo_reset(MSSSpiState *s)
>> {
>> @@ -104,10 +105,8 @@ static void set_fifodepth(MSSSpiState *s)
>> s->fifo_depth = 32;
>> } else if (size <= 16) {
>> s->fifo_depth = 16;
>> -} else if (size <= 32) {
>> -s->fifo_depth = 8;
>> } else {
>> -s->fifo_depth = 4;
>> +s->fifo_depth = 8;
>> }
>> }
>>
>> @@ -301,6 +300,17 @@ static void spi_write(void *opaque, hwaddr addr,
>> if (s->enabled) {
>> break;
>> }
>> +/*
>> + * [31:6] bits are reserved bits and for future use.
>> + * [5:0] are for frame size. Only [5:0] bits are validated
>> + * during write, [31:6] bits are untouched.
>> + */
>> +if ((value & FRAMESZ_MASK) > FRAMESZ_MAX) {
>> +qemu_log_mask(LOG_GUEST_ERROR, "%s: Incorrect size %d
>> provided."
>> + "Maximum frame size is %d\n",
>> + __func__, value & FRAMESZ_MASK, FRAMESZ_MAX);
>>
>
> Nitpicking here really, but shouldn't the %d be %u since value is a
> uint32_t?
>
> Ok I will change to %u.
Thanks,
Sundeep
> Thanks,
>
> Darren.
>
>
> +break;
>> +}
>> s->regs[R_SPI_DFSIZE] = value;
>> break;
>>
>> --
>> 2.5.0
>>
>>
>>
Re: [Qemu-devel] [Qemu devel v3 PATCH] msf2: Remove dead code reported by Coverity
Hi Peter, On Sun, Oct 22, 2017 at 7:35 PM, wrote: > Hi, > > This series failed automatic build test. Please find the testing commands > and > their output below. If you have docker installed, you can probably > reproduce it > locally. > > Type: series > Message-id: 1508678882-4327-1-git-send-email-sundeep.l...@gmail.com > Subject: [Qemu-devel] [Qemu devel v3 PATCH] msf2: Remove dead code > reported by Coverity > > === TEST SCRIPT BEGIN === > #!/bin/bash > set -e > git submodule update --init dtc > # Let docker tests dump environment info > export SHOW_ENV=1 > export J=8 > time make docker-test-quick@centos6 > time make docker-test-build@min-glib > time make docker-test-mingw@fedora > time make docker-test-block@fedora > === TEST SCRIPT END === > > Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 > From https://github.com/patchew-project/qemu > * [new tag] patchew/1508678882-4327-1-git- > send-email-sundeep.l...@gmail.com -> patchew/1508678882-4327-1-git- > send-email-sundeep.l...@gmail.com > Switched to a new branch 'test' > a1c66b66e7 msf2: Remove dead code reported by Coverity > > === OUTPUT BEGIN === > Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path > 'dtc' > Cloning into '/var/tmp/patchew-tester-tmp-egubaaoc/src/dtc'... > fatal: Could not read from remote repository. > > Please make sure you have the correct access rights > and the repository exists. > fatal: clone of 'git://git.qemu-project.org/dtc.git' into submodule path > '/var/tmp/patchew-tester-tmp-egubaaoc/src/dtc' failed > Failed to clone 'dtc'. Retry scheduled > Cloning into '/var/tmp/patchew-tester-tmp-egubaaoc/src/dtc'... > fatal: Could not read from remote repository. > > Please make sure you have the correct access rights > and the repository exists. > fatal: clone of 'git://git.qemu-project.org/dtc.git' into submodule path > '/var/tmp/patchew-tester-tmp-egubaaoc/src/dtc' failed > Failed to clone 'dtc' a second time, aborting > === OUTPUT END === > > Test command exited with code: 1 > I did not understand. Is the error related to my patch? Thanks, Sundeep > > > --- > Email generated automatically by Patchew [http://patchew.org/]. > Please send your feedback to patchew-de...@freelists.org >
Re: [Qemu-devel] [PATCH v4 2/4] vhost-user-blk: introduce a new vhost-user-blk host device
> -Original Message-
> From: Stefan Hajnoczi [mailto:stefa...@gmail.com]
> Sent: Friday, October 20, 2017 5:55 PM
> To: Liu, Changpeng
> Cc: qemu-devel@nongnu.org; pbonz...@redhat.com; m...@redhat.com;
> marcandre.lur...@redhat.com; fel...@nutanix.com; Harris, James R
>
> Subject: Re: [PATCH v4 2/4] vhost-user-blk: introduce a new vhost-user-blk
> host
> device
>
> On Fri, Oct 20, 2017 at 01:47:58AM +, Liu, Changpeng wrote:
> > > > +static Property vhost_user_blk_properties[] = {
> > > > +DEFINE_PROP_CHR("chardev", VHostUserBlk, chardev),
> > > > +DEFINE_PROP_UINT16("num_queues", VHostUserBlk, num_queues, 1),
> > > > +DEFINE_PROP_UINT32("queue_size", VHostUserBlk, queue_size, 128),
> > > > +DEFINE_PROP_BIT64("f_size_max", VHostUserBlk, host_features,
> > > > + VIRTIO_BLK_F_SIZE_MAX, true),
> > > > +DEFINE_PROP_BIT64("f_sizemax", VHostUserBlk, host_features,
> > > > + VIRTIO_BLK_F_SIZE_MAX, true),
> > > > +DEFINE_PROP_BIT64("f_segmax", VHostUserBlk, host_features,
> > > > + VIRTIO_BLK_F_SEG_MAX, true),
> > > > +DEFINE_PROP_BIT64("f_geometry", VHostUserBlk, host_features,
> > > > + VIRTIO_BLK_F_GEOMETRY, true),
> > > > +DEFINE_PROP_BIT64("f_readonly", VHostUserBlk, host_features,
> > > > + VIRTIO_BLK_F_RO, false),
> > > > +DEFINE_PROP_BIT64("f_blocksize", VHostUserBlk, host_features,
> > > > + VIRTIO_BLK_F_BLK_SIZE, true),
> > > > +DEFINE_PROP_BIT64("f_topology", VHostUserBlk, host_features,
> > > > + VIRTIO_BLK_F_TOPOLOGY, true),
> > > > +DEFINE_PROP_BIT64("f_multiqueue", VHostUserBlk, host_features,
> > > > + VIRTIO_BLK_F_MQ, true),
> > > > +DEFINE_PROP_BIT64("f_flush", VHostUserBlk, host_features,
> > > > + VIRTIO_BLK_F_FLUSH, true),
> > > > +DEFINE_PROP_BIT64("f_barrier", VHostUserBlk, host_features,
> > > > + VIRTIO_BLK_F_BARRIER, false),
> > > > +DEFINE_PROP_BIT64("f_scsi", VHostUserBlk, host_features,
> > > > + VIRTIO_BLK_F_SCSI, false),
> > > > +DEFINE_PROP_BIT64("f_wce", VHostUserBlk, host_features,
> > > > + VIRTIO_BLK_F_WCE, false),
> > >
> > > Please explain how feature negotation works. The vhost-user slave
> > > advertises support features in the return value from
> > > VHOST_USER_GET_FEATURES. How does this additional feature mask work
> and
> > > why is it useful?
> > According to Paolo's previous comments, VIRTIO_BLK_F_WCE/
> VIRTIO_BLK_F_SCSI/ VIRTIO_BLK_F_BARRIER
> > should be removed. Here I added all the feature flags just want to avoid the
> case that vhost-user slave target
> > can support but Qemu vhost block driver cannot support it.
>
> Please explain a bit more how these options can be used.
>
> When I looked at the vhost code it seemed like the vhost slave can
> report any feature bits it wishes (even things QEMU doesn't know about).
> What is the purpose of override some of the feature bits on the QEMU
> command-line?
Hi Stefan,
Here I added a switch which can override vhost-slave's feature bits, for
example, vhost-slave reported `VIRTIO_BLK_F_RO`,
but Qemu vhost-master can disable it through command line when started the
Qemu. Users don't need to change any
vhost-slave's code to disable this feature, and this is also aligned with
vhost-scsi and vhost-net's implementation.
>
> Stefan
[Qemu-devel] [Bug 1711828] Re: lock mov non generated #UD
[Expired for QEMU because there has been no activity for 60 days.] ** Changed in: qemu Status: Incomplete => Expired -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1711828 Title: lock mov non generated #UD Status in QEMU: Expired Bug description: qemu 2.8.1 debian 9.1 To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1711828/+subscriptions
Re: [Qemu-devel] [PATCH v2 18/24] fixup! ppc: spapr: use generic cpu_model parsing
On 13/10/17 09:24, David Gibson wrote:
> On Thu, Oct 12, 2017 at 05:50:08PM +0200, Igor Mammedov wrote:
>> inot sure how it managed to compile locally and on travis
>
> Because target/ppc/kvm.c is only compiled on a ppc _host_.
that or in cross environment, to be precise.
>
>> but build fails with type mismatch on PPC host, fixup it
>> by casting to expected type
>
> I already hit this one and fixed it up in place.
>
>>
>> Signed-off-by: Igor Mammedov
>> ---
>> target/ppc/kvm.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
>> index 48dc3f7..9d57deb 100644
>> --- a/target/ppc/kvm.c
>> +++ b/target/ppc/kvm.c
>> @@ -2505,7 +2505,7 @@ static int kvm_ppc_register_host_cpu_type(MachineState
>> *ms)
>> }
>> type_info.parent = object_class_get_name(OBJECT_CLASS(pvr_pcc));
>> type_register(&type_info);
>> -if (object_dynamic_cast(ms, TYPE_SPAPR_MACHINE)) {
>> +if (object_dynamic_cast(OBJECT(ms), TYPE_SPAPR_MACHINE)) {
>> /* override TCG default cpu type with 'host' cpu model */
>> mc->default_cpu_type = TYPE_HOST_POWERPC_CPU;
>> }
>
--
Alexey
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v4 1/4] vhost-user: add new vhost user messages to support virtio config space
> -Original Message-
> From: Stefan Hajnoczi [mailto:stefa...@gmail.com]
> Sent: Friday, October 20, 2017 6:01 PM
> To: Michael S. Tsirkin
> Cc: Liu, Changpeng ; qemu-devel@nongnu.org;
> pbonz...@redhat.com; marcandre.lur...@redhat.com; fel...@nutanix.com;
> Harris, James R
> Subject: Re: [PATCH v4 1/4] vhost-user: add new vhost user messages to support
> virtio config space
>
> On Thu, Oct 19, 2017 at 06:36:00PM +0300, Michael S. Tsirkin wrote:
> > On Thu, Oct 19, 2017 at 04:09:35PM +0200, Stefan Hajnoczi wrote:
> > > On Thu, Oct 19, 2017 at 01:24:07PM +0800, Changpeng Liu wrote:
> > > > @@ -922,6 +931,91 @@ static void vhost_user_set_iotlb_callback(struct
> vhost_dev *dev, int enabled)
> > > > /* No-op as the receive channel is not dedicated to IOTLB
> > > > messages. */
> > > > }
> > > >
> > > > +static int vhost_user_get_config(struct vhost_dev *dev, uint8_t
> > > > *config,
> > > > + size_t config_len)
> > > > +{
> > > > +VhostUserMsg msg = {
> > > > +.request = VHOST_USER_GET_CONFIG,
> > > > +.flags = VHOST_USER_VERSION,
> > > > +.size = config_len,
> > > > +};
> > > > +
> > > > +if (config_len == 0 || config_len > VHOST_USER_PAYLOAD_SIZE) {
> > >
> > > config_len should be limited to 256 bytes:
> > >
> > > if (config_len == 0 || config_len > sizeof(msg.payload.config) {
> >
> > I would just limit it to a reasonable value, acceptable to
> > both master and slave, not fail if it's bigger.
> >
> >
> > > > +error_report("bad config length");
> > > > +return -1;
> > > > +}
> > > > +
> > > > +if (vhost_user_write(dev, &msg, NULL, 0) < 0) {
> > > > +return -1;
> > > > +}
> > > > +
> > > > +if (vhost_user_read(dev, &msg) < 0) {
> > > > +return -1;
> > > > +}
> > > > +
> > > > +if (msg.request != VHOST_USER_GET_CONFIG) {
> > > > +error_report("Received unexpected msg type. Expected %d
> received %d",
> > > > + VHOST_USER_GET_CONFIG, msg.request);
> > > > +return -1;
> > > > +}
> > > > +
> > > > +if (msg.size != config_len) {
> > > > +error_report("Received bad msg size.");
> > > > +return -1;
> > > > +}
> > > > +
> > > > +memcpy(config, &msg.payload.config, config_len);
> > >
> > > There is some complexity here: different virtio devices use different
> > > amounts of config space. Devices may append new fields to the config
> > > space to support new features.
> > >
> > > Therefore I think the simplest protocol is to always fetch the full
> > > 256-byte configuration space. This way the vhost-user slave process can
> > > implement feature bits that the master process does not know about.
> > >
> > > In other words, I don't think the master process knows how much of the
> > > config space is used so it should always request 256 bytes.
> >
> > Each device knows the max config space size.
> >
> > vdev->config_len = config_size;
>
> I see you're referring to the field that is set in:
>
> void virtio_init(VirtIODevice *vdev, const char *name,
>uint16_t device_id, size_t config_size)
>
> How does this work for vhost-user where different slave programs may
> offer different config sizes?
Each Qemu vhost controller e.g: vhost-user-scsi-pci and vhost-user-blk-pci
should has different char devices,
so vhost-slave knows those messages are from vhost-scsi or vhost-blk, of
course, each UNIX domain socket
should be assigned by users with types: vhsot-scsi or vhost-blk.
>
> The QEMU master process does not know the correct size ahead of time.
> The size depends on the vhost-user slave process. This is why I suggest
> using the full 256 bytes that the VIRTIO spec defines.
Re: [Qemu-devel] [Qemu devel v3 PATCH] msf2: Remove dead code reported by Coverity
On Mon, 10/23 08:59, sundeep subbaraya wrote: > > Cloning into '/var/tmp/patchew-tester-tmp-egubaaoc/src/dtc'... > > fatal: Could not read from remote repository. > > > > Please make sure you have the correct access rights > > and the repository exists. > > fatal: clone of 'git://git.qemu-project.org/dtc.git' into submodule path > > '/var/tmp/patchew-tester-tmp-egubaaoc/src/dtc' failed > > Failed to clone 'dtc' a second time, aborting > > === OUTPUT END === > > > > Test command exited with code: 1 > > > > I did not understand. Is the error related to my patch? Nope, there is something wrong with the testing bot. Fam
Re: [Qemu-devel] [PATCH] hw/pci-host: Fix x86 Host Bridges 64bit PCI hole
Hi, > The guest will still run with 40 bits physical! > (judging the code anyway, I hope I am wrong) > > Then, the stakes are not so big, the Guest kernel will disregard > the 64bit hole since is not CPU addressable and go on. But then there is no working 64bit hole for hotplug ... cheers, Gerd
Re: [Qemu-devel] [RFC v2 13/22] monitor: separate QMP parser and dispatcher
On Fri, Oct 20, 2017 at 11:19:44AM +0200, Paolo Bonzini wrote: > On 19/10/2017 15:13, Stefan Hajnoczi wrote: > > Up to you. I don't remember the details of out_lock usage well enough > > to know whether using the lock for the queues is good or bad. > > out_lock is called like that because it's only writes that can be done > from multiple threads (at both the chardev and monitor level). > IOThreads in particular want to generate QMP events. > > out_lock protects the monitor's own output buffer, while the character > device layer has its own locking (chr_write_lock). chr_write_lock > protects calls to the chr_write method of the Chardev object and writes > to the character device's logfd. > > Renaming out_lock and reusing it is just fine if the lock is only held > for short periods of time. But maybe it's simpler to just introduce > another lock, since the out_lock rules are very simple. OK, let me use a new lock. Thanks for explaining! -- Peter Xu
Re: [Qemu-devel] [PATCH] docker: Fix PATH for ccache
On Fri, 10/20 13:51, Daniel P. Berrange wrote: > On Wed, Oct 18, 2017 at 09:35:49AM +0100, Daniel P. Berrange wrote: > > On Wed, Oct 18, 2017 at 04:28:36PM +0800, Fam Zheng wrote: > > > On Wed, 10/18 01:08, no-re...@patchew.org wrote: > > > > Your branch is up-to-date with 'origin/test'. > > > > Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for > > > > path 'dtc' > > > > Cloning into > > > > '/var/tmp/patchew-tester-tmp-jhpnlaz0/src/docker-src.2017-10-18-04.03.32.32287/qemu.tar.vroot/dtc'... > > > > fatal: unable to connect to git.qemu-project.org: > > > > git.qemu-project.org[0: 172.99.69.163]: errno=Connection timed out > > > > > > > > fatal: clone of 'git://git.qemu-project.org/dtc.git' into submodule > > > > path > > > > '/var/tmp/patchew-tester-tmp-jhpnlaz0/src/docker-src.2017-10-18-04.03.32.32287/qemu.tar.vroot/dtc' > > > > failed > > > > Failed to clone 'dtc'. Retry scheduled > > > > Cloning into > > > > '/var/tmp/patchew-tester-tmp-jhpnlaz0/src/docker-src.2017-10-18-04.03.32.32287/qemu.tar.vroot/dtc'... > > > > fatal: unable to connect to git.qemu-project.org: > > > > git.qemu-project.org[0: 172.99.69.163]: errno=Connection timed out > > > > > > > > fatal: clone of 'git://git.qemu-project.org/dtc.git' into submodule > > > > path > > > > '/var/tmp/patchew-tester-tmp-jhpnlaz0/src/docker-src.2017-10-18-04.03.32.32287/qemu.tar.vroot/dtc' > > > > failed > > > > Failed to clone 'dtc' a second time, aborting > > > > failed to init submodule dtc > > > > > > So now patchew is less robust on network hiccups: a number of the same > > > "git > > > submodule update --init" calls have succeeded above, so this is > > > incidental. Any > > > idea how to improve that? Maybe if there is a checkout from $QEMU_SRC, > > > clone/cp > > > from there instead of fetch from submodule remote? > > > > If patchew has unreliable connectivity to git.qemu.org then the best thing > > is probably have a background job that mirrors the git.qemu.org repos on > > the patchew server. Then have the test job modify .gitmodules to point to > > your local mirror. That way, network access is used during the background > > job only, and actual builds just get a local clone from the cache. > > I thought of a simpler way to address this. > > Currently patchew runs 4 docker jobs, and each one triggers creation of > the $(DOCKER_SRC_COPY) rule. This runs archive-source.sh 4 times, because > $(DOCKER_SRC_COPY) contains the current timestamp and so is different > each time we run it. > > If you override DOCKER_SRC_COPY with the current git change hash, then > you would end up using the same archive for all 4 jobs, and so only do > the checkouts once > > make docker-foo DOCKER_SRC_COPY=docker-src.`git rev-parse HEAD` I'm fine with that, just not sure about if this handles uncommitted changes correctly? It certainly isn't a concern on patchew, but may be a problem for developers. Your previous idea about mirroring submodules to local makes much sense, I can try it though. Fam
Re: [Qemu-devel] [PATCH v4] NUMA: Enable adding NUMA node implicitly
On Mon, 10/23 09:54, Dou Liyang wrote: > Hi Fam, > > At 10/23/2017 09:37 AM, no-re...@patchew.org wrote: > > Hi, > > > > This series failed automatic build test. Please find the testing commands > > and > > their output below. If you have docker installed, you can probably > > reproduce it > > locally. > > > > Type: series > > Message-id: 1508722422-3861-1-git-send-email-douly.f...@cn.fujitsu.com > > Subject: [Qemu-devel] [PATCH v4] NUMA: Enable adding NUMA node implicitly > > > > === TEST SCRIPT BEGIN === > > #!/bin/bash > > set -e > > git submodule update --init dtc > > # Let docker tests dump environment info > > export SHOW_ENV=1 > > export J=8 > > time make docker-test-quick@centos6 > > time make docker-test-build@min-glib > > time make docker-test-mingw@fedora > > time make docker-test-block@fedora > > === TEST SCRIPT END === > > > > Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 > > From https://github.com/patchew-project/qemu > > * [new tag] > > patchew/1508722422-3861-1-git-send-email-douly.f...@cn.fujitsu.com -> > > patchew/1508722422-3861-1-git-send-email-douly.f...@cn.fujitsu.com > > t [tag update] > > patchew/cover.1508329282.git.riku.voi...@linaro.org -> > > patchew/cover.1508329282.git.riku.voi...@linaro.org > > Switched to a new branch 'test' > > b8c0d922bf NUMA: Enable adding NUMA node implicitly > > > > === OUTPUT BEGIN === > > Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path > > 'dtc' > > Cloning into '/var/tmp/patchew-tester-tmp-z9ao6big/src/dtc'... > > fatal: Could not read from remote repository. > > > > Wow, It seems there are something wrong with the test robot. Yes, I'm looking into that! Thanks. Fam
Re: [Qemu-devel] host physical address width issues/questions for x86_64
On Fri, Oct 20, 2017 at 03:54:21PM -0700, Prasad Singamsetty wrote: > > > On 10/18/2017 8:33 PM, Peter Xu wrote: > >On Wed, Oct 18, 2017 at 10:19:31AM -0700, Prasad Singamsetty wrote: > >> > >> > >>On 10/16/2017 8:56 PM, Peter Xu wrote: > >>>On Mon, Oct 16, 2017 at 10:02:25AM -0700, Prasad Singamsetty wrote: > > > On 10/14/2017 8:53 PM, Peter Xu wrote: > >On Fri, Oct 13, 2017 at 11:14:03AM -0600, Alex Williamson wrote: > >>On Fri, 13 Oct 2017 18:01:44 +0100 > >>"Dr. David Alan Gilbert" wrote: > >> > >>>* Prasad Singamsetty (prasad.singamse...@oracle.com) wrote: > Hi, > > I am new to the alias. I have some questions on this subject > and seek some clarifications from the experts in the team. > I ran into a couple of issues when I tried with large configuration > ( >= 1TB memory, > 255 CPUs) for x86_64 guest machine. > > 1. QEMU uses the default value of 40 (TCG_PHYS_ADDR_BITS) for address > width if user has not specified phys-bits or host-phys-bits=true > property. The default value is obviously not sufficient and > causing guest kernel to crash if configured with >= 1TB > memory. Depending on the linux kernel version in the guest the > panic was in different code paths. The workaround is for the > user to specify the phys-bits property or set the property > host-phys-bits=true. > > QUESTIONS: > >>... > 2. host_address_width in DMAR table structure > > In this case, the default value is set to 39 > (VTD_HOST_ADDRESS_WIDTH - 1). With interrupt remapping > enabled for the intel iommu and the guest is configured > with > 255 cpus and >= 1TB memory, the guest kernel hangs > during boot up. This need to be fixed. > > QUESTION: > The question here again is can we fix this to use the > real address width from the host as the default? > >>> > >>>I don't know DMAR stuff; chatting to Alex (cc'd) it does sound > >>>like that's an ommission that should be fixed. > >> > >>[CC +Peter] > >> > >>On physical hardware VT-d supports either 39 or 48 bit address widths > >>and generally you'd expect a sufficiently capable IOMMU to be matched > >>with the CPU. Seems QEMU has only implemented a lower bit width and > >>it should probably be forcing phys bits of the VM to 39 to match until > >>the extended width can be implemented. Thanks, > >> > >>Alex > > > >There were patches that tried to enable 48 bits GAW but it was > >not accepted somehow: > > > > https://lists.gnu.org/archive/html/qemu-devel/2016-12/msg01886.html > > > >Would this help in any way? > > > > Thanks Alex for the patch info. Just curious why the patch was not > accepted. Any way, I will try it. > >>> > >>>I don't sure I know the reason. Anyway, it originated from one of > >>>Fam's request for some NVMe tests. If it can really help for your use > >>>case as well, please feel free to revive those patches, or let me know > >>>so that I can respin. Thanks, > >>> > >> > >>Thanks Peter. I will start with your patch and see if I can get > >>it to work first. > >> > >>A quick question. Looking at the code, it doesn't look like there > >>is a way to disable dma remapping. User may have a case where he > >>is interested only in interrupt remapping (for > 255 cpus) and > >>not DMA remapping. Is that scenario considered before? > > > >It can be done in the guest if the guest doesn't want DMAR. > > > >Note that there are two isolated kernel tunables for the VT-d device: > > > >- intel_iommu: "on" to turn on DMAR, "off" to turn off DMAR > >- intremap:"on" to turn on IR, "off" to turn off IR > > > >So even if guest has "intel_iommu=off" in its boot parameter, IR will > >still be on by default (or specify it explicitly using "intremap=on"). > > Thanks Peter. I think I figured out the problem in my test case > due to VTD_HOST_ADDRESS_WIDTH. > > Problem scenario: > > Guest kernel (machine type q35) is configured with 1TB memory. > With interrupt remapping enabled, the interrupt remapping > table is allocated by the guest kernel which can be any > where in the available physical memory. In my test case, > the physical address of the table is 0xfc3ec0. And > this gets truncated by vtd_interrupt_remap_table_setup() > function to 0x7c3ec0. This causes guest kernel to > get invalid data later on and it loops forever in > qi_submit_sync() in the guest kernel trying check fault > status. > > This is after applying the patch from Peter Xu. The patch > is incomplete as the VTD_HAW_MASK is unchanged so it is > defined for 39 bits. There are several other masks defined > based on this in accessing iommu data structures. So, more > changes neede
