Re: [Qemu-devel] [PATCH 01/22] memory: add memory_region_ram_resize
On 28/03/2015 19:58, Michael S. Tsirkin wrote:
>> > +void memory_region_ram_resize(MemoryRegion *mr, ram_addr_t newsize, Error
>> > **errp)
>> > +{
>> > +assert(mr->terminates);
> Why? Does "terminates" guarantee that ram_addr
> is valid? In any case, I think this needs a comment.
More or less, but not really; I was just cutting/pasting it from other
functions that use ram_addr. I will clean it up.
Paolo
Re: [Qemu-devel] [PATCH 05/22] memory: return bitmap from memory_region_is_logging
On 28/03/2015 19:54, Michael S. Tsirkin wrote:
> This API is pretty easy to misuse, and return
> value is completely undocumented. In fact, if I see
> "is" I assume a boolean return type.
>
> As you are touching all users anyway, how about
> wrappers that answer specific questions:
> memory_region_is_logging_dirty_memory_vga
> memory_region_is_logging_dirty_memory_migration
> memory_region_is_logging_dirty_memory_except_migration
> ?
The full return value is used later in the series.
I'll add a "client" argument to memory_region_is_logging (so that the
compiler will catch anything I forger) and add
memory_region_get_dirty_log_mask for the other users.
Regarding memory_region_is_logging_dirty_memory_except_migration, it
will go away once migration stops being special cased via
log_global_start/log_global_stop.
Paolo
> As a bonus, compiler will catch you if you forget to
> change some caller.
>
>> diff --git a/xen-hvm.c b/xen-hvm.c
>> index 315864c..acd89c8 100644
>> --- a/xen-hvm.c
>> +++ b/xen-hvm.c
>> @@ -488,7 +488,8 @@ static void xen_set_memory(struct MemoryListener
>> *listener,
>> XenIOState *state = container_of(listener, XenIOState, memory_listener);
>> hwaddr start_addr = section->offset_within_address_space;
>> ram_addr_t size = int128_get64(section->size);
>> -bool log_dirty = memory_region_is_logging(section->mr);
>> +bool log_dirty =
>> +memory_region_is_logging(section->mr) & ~(1 <<
>> DIRTY_MEMORY_MIGRATION);
>> hvmmem_type_t mem_type;
>>
>> if (section->mr == &ram_memory) {
>> --
>> 2.3.3
>>
>
>
Re: [Qemu-devel] [PATCH v5 08/45] Return path: socket_writev_buffer: Block even on non-blocking fd's
On Sat, Mar 28, 2015 at 04:30:06PM +0100, Paolo Bonzini wrote:
>
>
> On 25/02/2015 17:51, Dr. David Alan Gilbert (git) wrote:
> > +if (err != EAGAIN) {
>
> if (err != EAGAIN && err != EWOULDBLOCK)
I assume that's for the benefit of non-Linux hosts? On Linux EAGAIN
== EWOULDBLOCK.
--
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
pgp_A4pBKkdbf.pgp
Description: PGP signature
[Qemu-devel] Simulation or just emulation?
Hello, I've testing some ARM machines and CPU's through qemu-arm, for sure had different results through different systems, but I'm still wondering, does qemu simulates the behavior of ARM machines, or just emulate the code? Thanks! مآبقى إلا أسمانا تنقش على سطح القمر، هيبة المطران من هيبة البحر الغزير.
Re: [Qemu-devel] [PATCH v5 13/28] qapi: Add some expr tests
Eric Blake writes: > On 03/27/2015 06:38 AM, Markus Armbruster wrote: >> Eric Blake writes: >> >>> On 03/26/2015 09:55 AM, Markus Armbruster wrote: Eric Blake writes: > Demonstrate that the qapi generator doesn't deal well with > expressions that aren't up to par. Later patches will improve > the expected results as the generator is made stricter. Only > one of the added tests actually behaves sanely at rejecting > obvious problems. > qapi-code-gen.txt documents the naming conventions: Types, commands, and events share a common namespace. Therefore, generally speaking, type definitions should always use CamelCase for user-defined type names, while built-in types are lowercase. Type definitions should not end in 'Kind', as this namespace is used for creating implicit C enums for visiting union types. Command names, and field names within a type, should be all lower case with words separated by a hyphen. However, some existing older commands and complex types use underscore; when extending such expressions, consistency is preferred over blindly avoiding underscore. Event names should be ALL_CAPS with words separated by underscore. The special string '**' appears for some commands that manually perform their own type checking rather than relying on the type-safe code produced by the qapi code generators. We should either enforce the conventions consistently, or not at all. Enforcing them makes certain kinds of name clashes in generated C impossible. If we don't enforce them, we should catch the clashes. Since I haven't read to the end of your series, I have to ask: do you intend to enforce them? >>> >>> I added tests to enforce it for event names, but did not enforce things >>> for command names or complex type members. I guess that can be added on >>> top, if desired. >>> >>> So, did this patch get R-b? >> >> I'd rather not enforce naming conventions just for events. >> >> If we want to enforce them, let's do it consistently, and in a separate >> series that includes this patch. Okay? > > Sounds like I need a v6 respin then, where I drop my patch that attempts > to enforce all-caps event naming but did not enforce type or command > naming; but I will keep everything else (enforcing that names are valid > C identifiers + '-' and '.' (which both get flattened to '_'). Sounds good!
Re: [Qemu-devel] [PATCH v5 16/28] qapi: Better error messages for duplicated expressions
Eric Blake writes:
> On 03/27/2015 01:52 AM, Markus Armbruster wrote:
>> One more...
>>
>> Eric Blake writes:
>>
>> [...]
>>> diff --git a/scripts/qapi.py b/scripts/qapi.py
>>> index 90eb3bc..5d0dc91 100644
>>> --- a/scripts/qapi.py
>>> +++ b/scripts/qapi.py
>> [...]
>>> @@ -560,12 +585,22 @@ def type_name(name):
>>> return c_list_type(name[0])
>>> return name
>>>
>>> -enum_types = []
>>> -struct_types = []
>>> -union_types = []
>>> +def add_name(name, info, meta, implicit = False):
>>> +global all_names
>>> +if name in all_names:
>>> +raise QAPIExprError(info,
>>> +"%s '%s' is already defined"
>>> +%(all_names[name], name))
>>
>> We say "struct 'Foo'", and expect the user to know that 'struct' means
>> 'complex type'. It'll do, it's just a development tool.
>
> In fact, I considered making it "type 'Foo'", to match that the item is
> declared with { 'type':'Foo' ...} and not { 'struct':'Foo' ...}. But
> type is an ambiguous word. I'm half tempted to do a global
> search-and-replace of s/'type'/'struct'/ in the json files, since
> 'union' is also a type. Obviously as its own patch.
No objections. The churn will be a bit annoying with git-blame, but I'd
prefer that to continuing with confusing terminology.
>>
>> I'm not really happy with 'complex type', though. Isn't a union type
>> complex, too? Anyway, we can clean up our confused terminology later;
>> this series is long enough.
>
> Hmm. If I _do_ the global rename, then we have a nice hierarchy:
>
> type - simple type: built-in, enum
> - alternate
> - complex type: struct, union
Indeed.
The odd in-between-ness of alternate here is additional justification
for you separating it from union.
Re: [Qemu-devel] [PATCH v5 08/45] Return path: socket_writev_buffer: Block even on non-blocking fd's
On 29/03/2015 06:07, David Gibson wrote:
> On Sat, Mar 28, 2015 at 04:30:06PM +0100, Paolo Bonzini wrote:
>>
>>
>> On 25/02/2015 17:51, Dr. David Alan Gilbert (git) wrote:
>>> +if (err != EAGAIN) {
>>
>> if (err != EAGAIN && err != EWOULDBLOCK)
>
> I assume that's for the benefit of non-Linux hosts? On Linux
> EAGAIN == EWOULDBLOCK.
Yes, that's just the standard idiom in QEMU. This is generic code, so
assumption based on the host platform are not wise. :)
Paolo
Re: [Qemu-devel] [PATCH v5 21/28] qapi: Require valid names
Eric Blake writes:
[...]
> +valid_characters = set(string.ascii_letters + string.digits + '.' + '-' +
> '_')
> +def check_name(expr_info, source, name, allow_optional = False):
> +membername = name
> +
> +if not isinstance(name, str):
> +raise QAPIExprError(expr_info,
> +"%s requires a string name" % source)
> +if name == '**':
> +return
> +if name.startswith('*'):
> +membername = name[1:]
> +if not allow_optional:
> +raise QAPIExprError(expr_info,
> +"%s does not allow optional name '%s'"
> +% (source, name))
> +if not set(membername) <= valid_characters:
> +raise QAPIExprError(expr_info,
> +"%s uses invalid name '%s'" % (source, name))
> +
This accepts names starting with a digit. Sure we generate valid C
identifiers for such beauties?
qapi-code-gen.txt:
Downstream vendors may add extensions; such extensions should begin
with a prefix matching "__RFQDN_" (for the reverse-fully-qualified-
domain-name of the vendor), even if the rest of the command or field
name uses dash (example: __com.redhat_drive-mirror). Other than the
dots used in RFQDN of a downstream extension, all command, type, and
field names should begin with a letter, and contain only ASCII
letters, numbers, dash, and underscore.
One, I think "all command, type, and field names" is too narrow, what
about event names, or enumeration value names? Suggest say just "all
names".
Two, "letters, digits, dash, and underscore", please.
Three, I think check_name() should enforce "starts with letter or '_'".
[...]
Re: [Qemu-devel] [PATCH v5 21/28] qapi: Require valid names
Eric Blake writes:
> On 03/27/2015 02:48 AM, Markus Armbruster wrote:
>> Eric Blake writes:
>>
>>> Previous commits demonstrated that the generator overlooked various
>>> bad naming situations:
>>> - types, commands, and events need a valid name
>>> - union and alternate branches cannot be marked optional
>>>
>>> The set of valid names includes [a-zA-Z0-9._-] (where '.' is
>>> useful only in downstream extensions).
>>>
>
>>> +valid_characters = set(string.ascii_letters + string.digits + '.'
>>> + '-' + '_')
>>
>> strings.ascii_letters depends on the locale...
>
> https://docs.python.org/2/library/string.html
>
> string.ascii_letters
>
> The concatenation of the ascii_lowercase and ascii_uppercase
> constants described below. This value is not locale-dependent.
>
> You are thinking of string.letters, which IS locale-dependent. I
> intentionally used ascii_letters.
You're right, I misread the docs.
>>> +def check_name(expr_info, source, name, allow_optional = False):
>>> +membername = name
>>> +
>>> +if not isinstance(name, str):
>>> +raise QAPIExprError(expr_info,
>>> +"%s requires a string name" % source)
>>> +if name == '**':
>>> +return
>>
>> Doesn't this permit '**' anywhere, not just as pseudo-type in command
>> arguments and results?
>
> Yes, on the grounds that check_type then filters it appropriately. But
> worthy of a comment (probably both in the commit message AND in the code
> base). Grounds for a v6 respin.
>
>>
>>> +if name.startswith('*'):
>>> +membername = name[1:]
>>> +if not allow_optional:
>>> +raise QAPIExprError(expr_info,
>>> +"%s does not allow optional name '%s'"
>>> +% (source, name))
>>> +if not set(membername) <= valid_characters:
>>
>> ... so this check would break if we called locale.setlocale() in this
>> program. While I don't think we need to worry about it, I think you
>> could just as well use something like
>>
>> valid_name = re.compile(r"^[A-Za-z0-9-._]+$")
>>
>> if not valid_name.match(membername):
>
> regex is slightly slower than string matching _if the regex is
> precompiled_, and MUCH slower than string matching if the regex is
> compiled every time. In turn, string matching is slower than
> open-coding things, but has the benefit of being more compact and
> maintainable (open-coded loops are the worst on that front). Here's
> where I got my inspiration:
>
> https://stackoverflow.com/questions/1323364/in-python-how-to-check-if-a-string-only-contains-certain-characters
>
> But I may just go with the regex after all (I don't know how efficient
> python is about reusing a regex when a function is called multiple
> times, rather than recompiling the regex every time. Personal side
> note: back in 2009 or so, I was able to make 'm4' significantly faster
> in the context of 'autoconf' when I taught it to cache the compilation
> of the 8 most-recently-encountered regex, rather than recompiling every
> time; and then made 'autoconf' even faster when I taught it to do
> actions that didn't require regex use from 'm4' in the first place.)
Neat.
> The nice thing, though, is that I factored things so that the
> implementation of this one function can change without having to hunt
> down all call-sites, if I keep the contract the same.
I don't care for matching performance here, I do care for readability.
Please pick the solution you find easiest to understand.
>>> discriminator_type = base_fields.get(discriminator)
>>> if not discriminator_type:
>>> raise QAPIExprError(expr_info,
>>
>> What happens when I try 'discriminator': '**'?
>
> No clue. Good thing for me to add somewhere in my series. However, I
I had a second look. I think the generator accepting '**' in exactly
the right places relies on:
(1) check_name() accepts only proper names, not '**'.
(2) All names get checked with check_name().
(3) Except check_type() accepts special type name '**' when allow_star.
(4) allow_star is set for exactly the right places.
With check_name()'s superfluous / incorrect '**' check gone, (1)
obviously holds. (2) isn't obvious, thus food for code review. (3) is
trivial. (4) is trivial except for "exactly the right places".
qapi-code-gen.txt:
In rare cases, QAPI cannot express a type-safe representation of a
corresponding Client JSON Protocol command. In these cases, if the
command expression includes the key 'gen' with boolean value false,
then the 'data' or 'returns' member that intends to bypass generated
type-safety and do its own manual validation should use '**' rather
than a valid type name.
Unfortunately, "the 'data' or 'returns' member that intends to bypass
[...] should use '**' rather than a valid type name" can be read in (at
least) two ways:
1. It applies to the 'data', 'returns' members of the command object.
2. It appl
Re: [Qemu-devel] [PATCH v5 22/28] qapi: Whitelist commands that don't return dictionary
Eric Blake writes: > On 03/27/2015 10:19 AM, Markus Armbruster wrote: >> Eric Blake writes: >> >>> ...or an array of dictionaries. Although we have to cater to >>> existing commands, returning a non-dictionary means the command >>> is not extensible (no new name/value pairs can be added if more >>> information must be returned in parallel). By making the >>> whitelist explicit, any new command that falls foul of this >>> practice will have to be self-documenting, which will encourage >>> developers to either justify the action or rework the design to >>> use a dictionary after all. >>> >>> Signed-off-by: Eric Blake >>> --- > > >> >> Thinking on introspection, I started to wonder whether there's actually >> a command returning a union, yet. So I applied the appended stupid >> patch on top, and found the following commands returning (list of) >> non-struct type: >> >> * qapi-schema.json: >> >> 'ringbuf-read'built-in type 'str' >> 'human-monitor-command' built-in type 'str' >> 'query-migrate-cache-size'built-in type 'int' >> 'query-tpm-models'enum type 'TpmModel' > > More precisely, "array of enum type 'TpmModel'" (or "list", depending on > whether we go with JSON array/object terminology, or QObject dict/list). > I wonder if it is worth trying to tweak the error message to more > precisely track when I strip away the [] earlier in check_type to still > report sane messages about 'array of ...' if a later check fails. I figure the error message is understandable enough as is. But if it improving it would be easy, then why not. Could be done on top. >> 'query-tpm-types' enum type 'TpmType' >> 'query-memory-devices'union type 'MemoryDeviceInfo' >> >> * qga/qapi-schema.json: >> >> 'guest-sync-delimited'built-in type 'int' >> 'guest-sync' built-in type 'int' >> 'guest-get-time' built-in type 'int' >> 'guest-file-open' built-in type 'int' >> 'guest-fsfreeze-status' enum type 'GuestFsfreezeStatus' >> 'guest-fsfreeze-freeze' built-in type 'int' >> 'guest-fsfreeze-freeze-list' built-in type 'int' >> 'guest-fsfreeze-thaw' built-in type 'int' >> 'guest-set-vcpus' built-in type 'int' > > Good - your patch found all of my whitelists, plus... > >> >> The sole example for union is 'MemoryDeviceInfo'. It has one case %-} > > Yeah, MemoryDeviceInfo as a union currently has only one type, but it > was done that way in case we add other memory devices. So it was > actually quite forward-thinking. > > ...one additional thing. But returning (an array of) a union should be > okay (it is a dictionary, and therefore extensible); this patch was only > about flagging non-dictionaries. > > [side note: again, my idea of renaming 'type' into 'struct' in the .json > files would make it easier to talk about "complex types" as the set of > "struct" and "union" types, rather than the current confusion of > deciding if "type" means all meta-types or just struct meta-types.] Yes.
[Qemu-devel] [Bug 1437811] [NEW] target-tricore/op_helper.c:2576: bad if statement
Public bug reported:
[qemu/target-tricore/op_helper.c:2576]: (style) Expression '(X &
0x40) == 0x1' is always false.
if ((env->PCXI & MASK_PCXI_UL) == 1) {
/* CTYP trap */
}
** 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/1437811
Title:
target-tricore/op_helper.c:2576: bad if statement
Status in QEMU:
New
Bug description:
[qemu/target-tricore/op_helper.c:2576]: (style) Expression '(X &
0x40) == 0x1' is always false.
if ((env->PCXI & MASK_PCXI_UL) == 1) {
/* CTYP trap */
}
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1437811/+subscriptions
Re: [Qemu-devel] [PULL for-2.3 0/2] Ide patches
On Friday, 27 March 2015, John Snow wrote: > The following changes since commit > b27e767e8c8d56cb7c9d0b78eadd89521bdf836c: > > Merge remote-tracking branch 'remotes/stefanha/tags/net-pull-request' > into staging (2015-03-27 12:12:27 +) > > are available in the git repository at: > > https://github.com/jnsnow/qemu.git tags/ide-pull-request > > for you to fetch changes up to fc3d8e1138cd0c843d6fd75272633a31be6554ef: > > AHCI: Protect cmd register (2015-03-27 15:48:11 -0400) > > Applied, thanks. --PMM -- 12345678901234567890123456789012345678901234567890123456789012345678901234567890 1 2 3 4 5 6 7 8
Re: [Qemu-devel] [PATCH v2] qemu-m68k: add support for interrupt masking/unmasking
Hi Stefan,
Stefan Weil wrote,
> Am 28.03.2015 um 17:07 schrieb Waldemar Brodkorb:
> >Fixes following problem, when trying to boot linux:
> >qemu: hardware error: mcf_intc_write: Bad write offset 28
> >
> >CPU #0:
> >D0 = 00ff A0 = 402ea5dc F0 = ( 0)
> >D1 = 0004 A1 = 402ea5e0 F1 = ( 0)
> >D2 = 0040 A2 = 40040752 F2 = ( 0)
> >D3 = A3 = 40040a98 F3 = ( 0)
> >D4 = A4 = 400407b4 F4 = ( 0)
> >D5 = A5 = F5 = ( 0)
> >D6 = A6 = 40195ff8 F6 = ( 0)
> >D7 = A7 = 40195fd0 F7 = ( 0)
> >PC = 401b2058 SR = 2704 --Z-- FPRESULT =0
> >Aborted
> >
> >System started via:
> >qemu-system-m68k -nographic -nographic -M mcf5208evb -cpu m5208 -kernel
> >kernel
> >
> >Patch originally posted here:
> >http://lists.busybox.net/pipermail/buildroot/2012-April/052585.html
> >
> >Signed-off-by: Thomas Petazzoni
> >Tested-by: Waldemar Brodkorb
> >Signed-off-by: Waldemar Brodkorb
> >---
> >v1 -> v2:
> > - add {} to conform to Qemu Coding Style suggested by Stefan Weil
> > - add short comments to case statements with return 0 suggested by
> > Peter Maydell
> > - ull as suffix to integer 1 suggested by Peter Maydell does not
> > work for me
> > as I get a kernel panic shortly after boot
>
> Maybe that's an indicator that it only works with 1ULL. :-)
>
> Did you add it at both locations (for set and clear of interrupt mask)?
Yes.
> If not: does it work if you fix this?
> If yes: does it work if you only use 1ULL for SIMR?
No.
> You can debug the kernel panic by attaching a cross debugger to the
> running kernel.
> If you have a kernel image with debug symbols, this is very comfortable.
How would I do this?
Tried to start qemu with -s -S and then attach with my cross-gdb
using the kernel with debug symbols. But gdb does not recognize the
panic:
Command: mdev -s
Command: ifconfig lo 127.0.0.1 up
Execution Finished, Exiting
Sash command shell (version 1.1.1)
/> Kernel panic - not syncing: Attempted to kill init!
exitcode=0x000b
---[ end Kernel panic - not syncing: Attempted to kill init!
exitcode=0x000b
best regards
Waldemar
Using this:
diff --git a/hw/m68k/mcf_intc.c b/hw/m68k/mcf_intc.c
index 621423c..bcdd7c4 100644
--- a/hw/m68k/mcf_intc.c
+++ b/hw/m68k/mcf_intc.c
@@ -65,6 +65,9 @@ static uint64_t mcf_intc_read(void *opaque, hwaddr
addr,
return (uint32_t)(s->ifr >> 32);
case 0x14:
return (uint32_t)s->ifr;
+case 0x1c: /* SIMR */
+case 0x1d: /* CIMR */
+ return 0;
case 0xe0: /* SWIACK. */
return s->active_vector;
case 0xe1: case 0xe2: case 0xe3: case 0xe4:
@@ -102,6 +105,22 @@ static void mcf_intc_write(void *opaque, hwaddr
addr,
case 0x0c:
s->imr = (s->imr & 0xull) | (uint32_t)val;
break;
+/* SIMR allows to easily mask interrupts */
+case 0x1c:
+ if (val & 0x40) {
+ s->imr = UINT64_MAX;
+ } else {
+ s->imr |= ((uint64_t)1 << (val & 0x3f));
+ }
+ break;
+/* CIMR allows to easily unmask interrupts */
+case 0x1d:
+ if (val & 0x40) {
+ s->imr = 0;
+ } else {
+ s->imr &= ~((uint64_t)1 << (val & 0x3f));
+ }
+ break;
default:
hw_error("mcf_intc_write: Bad write offset %d\n", offset);
break;
--
Re: [Qemu-devel] [PATCH v5 21/28] qapi: Require valid names
Markus Armbruster writes: [...] > I had a second look. I think the generator accepting '**' in exactly > the right places relies on: > > (1) check_name() accepts only proper names, not '**'. > > (2) All names get checked with check_name(). > > (3) Except check_type() accepts special type name '**' when allow_star. > > (4) allow_star is set for exactly the right places. > > With check_name()'s superfluous / incorrect '**' check gone, (1) > obviously holds. (2) isn't obvious, thus food for code review. (3) is > trivial. (4) is trivial except for "exactly the right places". > qapi-code-gen.txt: > > In rare cases, QAPI cannot express a type-safe representation of a > corresponding Client JSON Protocol command. In these cases, if the > command expression includes the key 'gen' with boolean value false, > then the 'data' or 'returns' member that intends to bypass generated > type-safety and do its own manual validation should use '**' rather > than a valid type name. > > Unfortunately, "the 'data' or 'returns' member that intends to bypass > [...] should use '**' rather than a valid type name" can be read in (at > least) two ways: > > 1. It applies to the 'data', 'returns' members of the command object. > > 2. It applies to members of 'data', 'returns' members of the command > object. > > The schema uses both readings: qom-get has 'returns': '**', and qom-set, > netdev_add, object_add have 'data' members of the form KEY: '**'. > > Note that neither code nor tests have 'data': '**' or KEY: '**' in > 'returns'. Type '**' generally means "any JSON value". However, a command's 'data' must be a JSON object (see docs/qmp/qmp-spec.txt). Ways to deal with this: A. Ignore. Conforming to the schema is necessary but not sufficient for a command being accepted anyway. B. The meaning of type '**' depends on context: it's "any JSON object" for a command's 'data', else "any JSON value". C. Outlaw 'data': '**' for now. I prefer C, I can accept A, I dislike B. > qapi.py appears to implement a third way: '**' may appear as type name > anywhere within 'data' or 'returns'. May well make sense, and may well > work, but we have no test coverage for it. > > We can extend tests to cover what the generator accepts (separate series > recommended), or restrict the generator to what we actually use and > test. I'm leaning towards the latter. > > Further note that '**' can only be used right in a command declaration. > Factoring out the right hand side of 'data' or 'returns' into a separate > struct type declaration isn't possible when it contains '**'. We could treat '**' as top type rather than as hack for a few commands. Conceptually clean, but hard to justify without users. [...]
Re: [Qemu-devel] [PATCH v2] qemu-m68k: add support for interrupt masking/unmasking
Am 29.03.2015 um 15:47 schrieb Waldemar Brodkorb: Hi Stefan, Stefan Weil wrote, You can debug the kernel panic by attaching a cross debugger to the running kernel. If you have a kernel image with debug symbols, this is very comfortable. How would I do this? Tried to start qemu with -s -S and then attach with my cross-gdb using the kernel with debug symbols. But gdb does not recognize the panic: Command: mdev -s Command: ifconfig lo 127.0.0.1 up Execution Finished, Exiting Sash command shell (version 1.1.1) /> Kernel panic - not syncing: Attempted to kill init! exitcode=0x000b ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x000b Yes, the cross gdb won't recognize a kernel panic. But you can set a breakpoint on the kernel code which handles such panics, on one of the functions shown in the kernel panic backtrace, or on printk. Regards Stefan
Re: [Qemu-devel] [PATCH v5 00/28] drop qapi nested structs
What happens when I define a member key multiple times in a struct or
union type?
If I do it directly, the parser rejects the duplicate key in
get_members(). Covered by tests/qapi-schema/duplicate-key.json.
What if I hide the duplicate in a base type?
If I stick this into qapi-schema-test.json:
{ 'type': 'Base', 'data': { 'foo': 'str', 'bar': 'str' } }
{ 'type': 'Clash', 'base': 'Base', 'data': { 'foo': 'int' } }
{ 'command': 'clash', 'data': 'Clash' }
the resulting test-qmp-commands.h declares qmp_clash(), but
test-qmp-marshal.c doesn't define it. WTF?!?
Re: [Qemu-devel] [PATCH v2] qemu-m68k: add support for interrupt masking/unmasking
Am 29.03.2015 um 17:53 schrieb Stefan Weil: Am 29.03.2015 um 15:47 schrieb Waldemar Brodkorb: Hi Stefan, Stefan Weil wrote, You can debug the kernel panic by attaching a cross debugger to the running kernel. If you have a kernel image with debug symbols, this is very comfortable. How would I do this? Tried to start qemu with -s -S and then attach with my cross-gdb using the kernel with debug symbols. But gdb does not recognize the panic: Command: mdev -s Command: ifconfig lo 127.0.0.1 up Execution Finished, Exiting Sash command shell (version 1.1.1) /> Kernel panic - not syncing: Attempted to kill init! exitcode=0x000b ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x000b Is this the kernel panic which you get? I did not have a closer look on it before, but now I see that it is something quite common: Your kernel runs an init script (or binary) which terminates (obviously normally). Then the kernel does not know what to do, so it throws a kernel panic "Attempted to kill init". Usually the init process should only terminate at a system shutdown. Stefan
Re: [Qemu-devel] [PATCH v2] microblaze: fix memory leak
28.03.2015 10:46, Gonglei wrote: [] >> Can this go through -trivial? >> > It's ok, but I don't know if -trivial branch maintainer has a plan to send a > pull request > for rc2. I do have plan to send a pull request, but this thing is ugly. The original code is ugly, and your patch does not make it better. I wanted to make some changes myself but didn't find fime for that. Ofcourse it is okay to apply your change to plug the leak, but.. oh well... :( And if we do, we'll sure forget about that completely. Now at least you're pinging me from time to time... ;)) Thanks, /mjt
[Qemu-devel] [PATCH for-2.3?] qom: Fix object_property_add_alias() with [*]
Commit 8074264 (qom: Add description field in ObjectProperty struct) introduced property descriptions and copied them for alias properties. Instead of using the caller-supplied property name, use the returned property name for setting the description. This avoids an Error when setting a property description for a property with literal "[*]" that doesn't exist due to automatic property naming in object_property_add(). Cc: Gonglei Cc: Paolo Bonzini Cc: Michael S. Tsirkin Signed-off-by: Andreas Färber --- qom/object.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qom/object.c b/qom/object.c index d167038..b8dff43 100644 --- a/qom/object.c +++ b/qom/object.c @@ -1761,7 +1761,7 @@ void object_property_add_alias(Object *obj, const char *name, } op->resolve = property_resolve_alias; -object_property_set_description(obj, name, +object_property_set_description(obj, op->name, target_prop->description, &error_abort); -- 2.1.4
Re: [Qemu-devel] [RFC] acpi: add reset register to fadt
On 03/28/2015 05:46 PM, Reza Jelveh wrote:
Some operating systems such as FreeBSD and Mac OSX need the reset_register
section of the FADT filled to know which port to write to for a system reset.
What is the right way to set the reset_val and the reset addr in this case?
---
hw/i386/acpi-build.c | 5 +
hw/i386/acpi-defs.h | 2 ++
2 files changed, 7 insertions(+)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index d0a5c85..21c1453 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -361,6 +361,11 @@ static void fadt_setup(AcpiFadtDescriptorRev1 *fadt,
AcpiPmInfo *pm)
(1 << ACPI_FADT_F_SLP_BUTTON) |
(1 << ACPI_FADT_F_RTC_S4));
fadt->flags |= cpu_to_le32(1 << ACPI_FADT_F_USE_PLATFORM_CLOCK);
+fadt->flags |= cpu_to_le32(1 << ACPI_FADT_F_RESET_REG_SUP);
+fadt->reset_val = 0xf;
+fadt->reset_reg.address_space_id = aml_system_io;
+fadt->reset_reg.register_bit_width = 8;
+fadt->reset_reg.address= ICH9_RST_CNT_IOPORT;
Hi,
Is this for both PC and Q35? Because I don't think PC has ICH9_RST_CNT_IOPORT.
Thanks,
Marcel
/* APIC destination mode ("Flat Logical") has an upper limit of 8 CPUs
* For more than 8 CPUs, "Clustered Logical" mode has to be used
*/
diff --git a/hw/i386/acpi-defs.h b/hw/i386/acpi-defs.h
index c4468f8..960c833 100644
--- a/hw/i386/acpi-defs.h
+++ b/hw/i386/acpi-defs.h
@@ -132,6 +132,8 @@ struct AcpiFadtDescriptorRev1
uint8_t reserved4a; /* Reserved */
uint8_t reserved4b; /* Reserved */
uint32_t flags;
+Acpi20GenericAddress reset_reg;
+uint8_t reset_val;
} QEMU_PACKED;
typedef struct AcpiFadtDescriptorRev1 AcpiFadtDescriptorRev1;
[Qemu-devel] [PATCH RFC for-2.3? 3/8] serial: Move ISASerialState to header
To be used for embedding the device.
Add gtk-doc private/public markers for parent field.
Signed-off-by: Andreas Färber
---
hw/char/serial-isa.c | 12
include/hw/char/serial.h | 14 ++
2 files changed, 14 insertions(+), 12 deletions(-)
diff --git a/hw/char/serial-isa.c b/hw/char/serial-isa.c
index f3db024..ae2235b 100644
--- a/hw/char/serial-isa.c
+++ b/hw/char/serial-isa.c
@@ -24,18 +24,6 @@
*/
#include "hw/char/serial.h"
-#include "hw/isa/isa.h"
-
-#define ISA_SERIAL(obj) OBJECT_CHECK(ISASerialState, (obj), TYPE_ISA_SERIAL)
-
-typedef struct ISASerialState {
-ISADevice parent_obj;
-
-uint32_t index;
-uint32_t iobase;
-uint32_t isairq;
-SerialState state;
-} ISASerialState;
static const int isa_serial_io[MAX_SERIAL_PORTS] = {
0x3f8, 0x2f8, 0x3e8, 0x2e8
diff --git a/include/hw/char/serial.h b/include/hw/char/serial.h
index 15beb6b..77cb95f 100644
--- a/include/hw/char/serial.h
+++ b/include/hw/char/serial.h
@@ -26,6 +26,7 @@
#define HW_SERIAL_H 1
#include "hw/hw.h"
+#include "hw/isa/isa.h"
#include "sysemu/sysemu.h"
#include "exec/memory.h"
#include "qemu/fifo8.h"
@@ -92,6 +93,19 @@ SerialState *serial_mm_init(MemoryRegion *address_space,
/* serial-isa.c */
#define TYPE_ISA_SERIAL "isa-serial"
+#define ISA_SERIAL(obj) OBJECT_CHECK(ISASerialState, (obj), TYPE_ISA_SERIAL)
+
+typedef struct ISASerialState {
+/*< private >*/
+ISADevice parent_obj;
+/*< public >*/
+
+uint32_t index;
+uint32_t iobase;
+uint32_t isairq;
+SerialState state;
+} ISASerialState;
+
void serial_hds_isa_init(ISABus *bus, int n);
#endif
--
2.1.4
[Qemu-devel] [PATCH RFC for-2.3? 0/8] prep: Fix pc87312 for -device usage
Hello Markus et al., This series attempts to fix the -device pc87312 issues you reported. I can't add alias properties for devices that don't get created before realize. Therefore this involves moving code for various ISA devices, to enable us to initialize the objects early for alias properties and realizing them as part of the composite device once the configuration is known, also fixing error propagation while at it. Probably needs a further iteration. Yes, it's terribly invasive, that's why the code was as it is. But the code movements are quite trivial, as long as no in-air conflicts occur. A follow-up would be to respin my old ISA enabled/disabled series to allow inactive ISADevices sitting on an ISABus. Regards, Andreas Cc: Markus Armbruster Cc: Hervé Poussineau Cc: qemu-...@nongnu.org Cc: qemu-bl...@nongnu.org Cc: John Snow Andreas Färber (8): parallel: Factor out header for ISAParallelState struct pc87312: Create isa-parallel in-place and add alias par0-chardev property serial: Move ISASerialState to header pc87312: Create UARTs in-place and add alias properties fdb: Move FDCtrlISABus to header pc87312: Create FDC in-place ide: Move ISAIDEState to header pc87312: Create IDE in-place hw/block/fdc.c | 87 --- hw/char/parallel.c | 30 +--- hw/char/serial-isa.c | 12 hw/ide/internal.h | 155 hw/ide/isa.c | 13 hw/isa/pc87312.c | 107 hw/ppc/prep.c | 33 + include/hw/block/fdc.h | 88 +++ include/hw/char/parallel.h | 62 include/hw/char/serial.h | 14 include/hw/ide.h | 173 + include/hw/isa/pc87312.h | 23 +++--- 12 files changed, 442 insertions(+), 355 deletions(-) create mode 100644 include/hw/char/parallel.h -- 2.1.4
[Qemu-devel] [PATCH RFC for-2.3? 2/8] pc87312: Create isa-parallel in-place and add alias par0-chardev property
Move the parallel_hds[] code to the PReP machine.
Signed-off-by: Andreas Färber
---
hw/isa/pc87312.c | 25 +++--
hw/ppc/prep.c| 9 +
include/hw/isa/pc87312.h | 5 ++---
3 files changed, 26 insertions(+), 13 deletions(-)
diff --git a/hw/isa/pc87312.c b/hw/isa/pc87312.c
index 40a1106..ded6c01 100644
--- a/hw/isa/pc87312.c
+++ b/hw/isa/pc87312.c
@@ -268,6 +268,7 @@ static void pc87312_realize(DeviceState *dev, Error **errp)
ISABus *bus;
CharDriverState *chr;
DriveInfo *drive;
+Error *local_err = NULL;
char name[5];
int i;
@@ -278,20 +279,19 @@ static void pc87312_realize(DeviceState *dev, Error
**errp)
pc87312_hard_reset(s);
if (is_parallel_enabled(s)) {
-chr = parallel_hds[0];
-if (chr == NULL) {
-chr = qemu_chr_new("par0", "null", NULL);
-}
-isa = isa_create(bus, "isa-parallel");
-d = DEVICE(isa);
-qdev_prop_set_uint32(d, "index", 0);
+d = DEVICE(&s->parallel);
+qdev_set_parent_bus(d, BUS(bus));
qdev_prop_set_uint32(d, "iobase", get_parallel_iobase(s));
qdev_prop_set_uint32(d, "irq", get_parallel_irq(s));
-qdev_prop_set_chr(d, "chardev", chr);
-qdev_init_nofail(d);
-s->parallel.dev = isa;
trace_pc87312_info_parallel(get_parallel_iobase(s),
get_parallel_irq(s));
+object_property_set_bool(OBJECT(&s->parallel), true, "realized",
+ &local_err);
+object_unref(OBJECT(&s->parallel));
+if (local_err) {
+error_propagate(errp, local_err);
+return;
+}
}
for (i = 0; i < 2; i++) {
@@ -351,6 +351,11 @@ static void pc87312_initfn(Object *obj)
PC87312State *s = PC87312(obj);
memory_region_init_io(&s->io, obj, &pc87312_io_ops, s, "pc87312", 2);
+
+object_initialize(&s->parallel, sizeof(s->parallel), TYPE_ISA_PARALLEL);
+object_property_add_alias(obj, "par0-chardev",
+ OBJECT(&s->parallel), "chardev", &error_abort);
+qdev_prop_set_uint32(DEVICE(&s->parallel), "index", 0);
}
static const VMStateDescription vmstate_pc87312 = {
diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index 7f52662..1dfa051 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -40,6 +40,7 @@
#include "hw/isa/pc87312.h"
#include "sysemu/block-backend.h"
#include "sysemu/arch_init.h"
+#include "sysemu/char.h"
#include "sysemu/qtest.h"
#include "exec/address-spaces.h"
#include "elf.h"
@@ -528,6 +529,7 @@ static void ppc_prep_init(MachineState *machine)
PCIDevice *pci;
ISABus *isa_bus;
ISADevice *isa;
+CharDriverState *chr;
qemu_irq *cpu_exit_irq;
int ppc_boot_device;
DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
@@ -639,6 +641,13 @@ static void ppc_prep_init(MachineState *machine)
/* Super I/O (parallel + serial ports) */
isa = isa_create(isa_bus, TYPE_PC87312);
dev = DEVICE(isa);
+chr = parallel_hds[0];
+if (chr == NULL) {
+chr = qemu_chr_new("par0", "null", NULL);
+}
+if (chr != NULL) {
+qdev_prop_set_chr(dev, "par0-chardev", chr);
+}
qdev_prop_set_uint8(dev, "config", 13); /* fdc, ser0, ser1, par0 */
qdev_init_nofail(dev);
diff --git a/include/hw/isa/pc87312.h b/include/hw/isa/pc87312.h
index bf74470..b473b3b 100644
--- a/include/hw/isa/pc87312.h
+++ b/include/hw/isa/pc87312.h
@@ -26,6 +26,7 @@
#define QEMU_PC87312_H
#include "hw/isa/isa.h"
+#include "hw/char/parallel.h"
#define TYPE_PC87312 "pc87312"
@@ -37,9 +38,7 @@ typedef struct PC87312State {
uint32_t iobase;
uint8_t config; /* initial configuration */
-struct {
-ISADevice *dev;
-} parallel;
+ISAParallelState parallel;
struct {
ISADevice *dev;
--
2.1.4
[Qemu-devel] [PATCH RFC for-2.3? 4/8] pc87312: Create UARTs in-place and add alias properties
Move serial_hds[] code into PReP machine.
Signed-off-by: Andreas Färber
---
hw/isa/pc87312.c | 33 -
hw/ppc/prep.c| 13 +
include/hw/isa/pc87312.h | 6 ++
3 files changed, 35 insertions(+), 17 deletions(-)
diff --git a/hw/isa/pc87312.c b/hw/isa/pc87312.c
index ded6c01..207eaa8 100644
--- a/hw/isa/pc87312.c
+++ b/hw/isa/pc87312.c
@@ -266,10 +266,8 @@ static void pc87312_realize(DeviceState *dev, Error **errp)
DeviceState *d;
ISADevice *isa;
ISABus *bus;
-CharDriverState *chr;
DriveInfo *drive;
Error *local_err = NULL;
-char name[5];
int i;
s = PC87312(dev);
@@ -296,21 +294,19 @@ static void pc87312_realize(DeviceState *dev, Error
**errp)
for (i = 0; i < 2; i++) {
if (is_uart_enabled(s, i)) {
-chr = serial_hds[i];
-if (chr == NULL) {
-snprintf(name, sizeof(name), "ser%d", i);
-chr = qemu_chr_new(name, "null", NULL);
-}
-isa = isa_create(bus, "isa-serial");
-d = DEVICE(isa);
-qdev_prop_set_uint32(d, "index", i);
+d = DEVICE(&s->uart[i]);
+qdev_set_parent_bus(d, BUS(bus));
qdev_prop_set_uint32(d, "iobase", get_uart_iobase(s, i));
qdev_prop_set_uint32(d, "irq", get_uart_irq(s, i));
-qdev_prop_set_chr(d, "chardev", chr);
-qdev_init_nofail(d);
-s->uart[i].dev = isa;
trace_pc87312_info_serial(i, get_uart_iobase(s, i),
get_uart_irq(s, i));
+object_property_set_bool(OBJECT(&s->uart[i]), true, "realized",
+ &local_err);
+object_unref(OBJECT(&s->uart[i]));
+if (local_err) {
+error_propagate(errp, local_err);
+return;
+}
}
}
@@ -349,6 +345,7 @@ static void pc87312_realize(DeviceState *dev, Error **errp)
static void pc87312_initfn(Object *obj)
{
PC87312State *s = PC87312(obj);
+int i;
memory_region_init_io(&s->io, obj, &pc87312_io_ops, s, "pc87312", 2);
@@ -356,6 +353,16 @@ static void pc87312_initfn(Object *obj)
object_property_add_alias(obj, "par0-chardev",
OBJECT(&s->parallel), "chardev", &error_abort);
qdev_prop_set_uint32(DEVICE(&s->parallel), "index", 0);
+
+for (i = 0; i < 2; i++) {
+char *propname = g_strdup_printf("ser%d-chardev", i);
+object_initialize(&s->uart[i], sizeof(s->uart[i]), TYPE_ISA_SERIAL);
+qdev_prop_set_uint32(DEVICE(&s->uart[i]), "index", i);
+object_property_add_alias(obj, propname,
+ OBJECT(&s->uart[i]), "chardev",
+ &error_abort);
+g_free(propname);
+}
}
static const VMStateDescription vmstate_pc87312 = {
diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index 1dfa051..eb29d3c 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -648,6 +648,19 @@ static void ppc_prep_init(MachineState *machine)
if (chr != NULL) {
qdev_prop_set_chr(dev, "par0-chardev", chr);
}
+for (i = 0; i < 2; i++) {
+chr = serial_hds[i];
+if (chr == NULL) {
+char *name = g_strdup_printf("ser%d", i);
+chr = qemu_chr_new(name, "null", NULL);
+g_free(name);
+}
+if (chr != NULL) {
+char *name = g_strdup_printf("ser%d-chardev", i);
+qdev_prop_set_chr(dev, name, chr);
+g_free(name);
+}
+}
qdev_prop_set_uint8(dev, "config", 13); /* fdc, ser0, ser1, par0 */
qdev_init_nofail(dev);
diff --git a/include/hw/isa/pc87312.h b/include/hw/isa/pc87312.h
index b473b3b..3256e0f 100644
--- a/include/hw/isa/pc87312.h
+++ b/include/hw/isa/pc87312.h
@@ -27,6 +27,7 @@
#include "hw/isa/isa.h"
#include "hw/char/parallel.h"
+#include "hw/char/serial.h"
#define TYPE_PC87312 "pc87312"
@@ -39,10 +40,7 @@ typedef struct PC87312State {
uint8_t config; /* initial configuration */
ISAParallelState parallel;
-
-struct {
-ISADevice *dev;
-} uart[2];
+ISASerialState uart[2];
struct {
ISADevice *dev;
--
2.1.4
[Qemu-devel] [PATCH RFC for-2.3? 1/8] parallel: Factor out header for ISAParallelState struct
To be used for embedding the device.
Add gtk-doc private/public markers for parent field.
Signed-off-by: Andreas Färber
---
hw/char/parallel.c | 30 +-
include/hw/char/parallel.h | 62 ++
2 files changed, 63 insertions(+), 29 deletions(-)
create mode 100644 include/hw/char/parallel.h
diff --git a/hw/char/parallel.c b/hw/char/parallel.c
index 4079554..fcb3764 100644
--- a/hw/char/parallel.c
+++ b/hw/char/parallel.c
@@ -24,7 +24,7 @@
*/
#include "hw/hw.h"
#include "sysemu/char.h"
-#include "hw/isa/isa.h"
+#include "hw/char/parallel.h"
#include "hw/i386/pc.h"
#include "sysemu/sysemu.h"
@@ -64,34 +64,6 @@
#define PARA_CTR_SIGNAL
(PARA_CTR_SELECT|PARA_CTR_INIT|PARA_CTR_AUTOLF|PARA_CTR_STROBE)
-typedef struct ParallelState {
-MemoryRegion iomem;
-uint8_t dataw;
-uint8_t datar;
-uint8_t status;
-uint8_t control;
-qemu_irq irq;
-int irq_pending;
-CharDriverState *chr;
-int hw_driver;
-int epp_timeout;
-uint32_t last_read_offset; /* For debugging */
-/* Memory-mapped interface */
-int it_shift;
-} ParallelState;
-
-#define TYPE_ISA_PARALLEL "isa-parallel"
-#define ISA_PARALLEL(obj) \
-OBJECT_CHECK(ISAParallelState, (obj), TYPE_ISA_PARALLEL)
-
-typedef struct ISAParallelState {
-ISADevice parent_obj;
-
-uint32_t index;
-uint32_t iobase;
-uint32_t isairq;
-ParallelState state;
-} ISAParallelState;
static void parallel_update_irq(ParallelState *s)
{
diff --git a/include/hw/char/parallel.h b/include/hw/char/parallel.h
new file mode 100644
index 000..b20e403
--- /dev/null
+++ b/include/hw/char/parallel.h
@@ -0,0 +1,62 @@
+/*
+ * QEMU Parallel PORT emulation
+ *
+ * Copyright (c) 2003-2005 Fabrice Bellard
+ * Copyright (c) 2007 Marko Kohtala
+ * Copyright (c) 2015 SUSE Linux GmbH
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+#ifndef HW_CHAR_PARALLEL_H
+#define HW_CHAR_PARALLEL_H
+
+#include "hw/isa/isa.h"
+
+typedef struct ParallelState {
+MemoryRegion iomem;
+uint8_t dataw;
+uint8_t datar;
+uint8_t status;
+uint8_t control;
+qemu_irq irq;
+int irq_pending;
+CharDriverState *chr;
+int hw_driver;
+int epp_timeout;
+uint32_t last_read_offset; /* For debugging */
+/* Memory-mapped interface */
+int it_shift;
+} ParallelState;
+
+#define TYPE_ISA_PARALLEL "isa-parallel"
+#define ISA_PARALLEL(obj) \
+OBJECT_CHECK(ISAParallelState, (obj), TYPE_ISA_PARALLEL)
+
+typedef struct ISAParallelState {
+/*< private >*/
+ISADevice parent_obj;
+/*< public >*/
+
+uint32_t index;
+uint32_t iobase;
+uint32_t isairq;
+ParallelState state;
+} ISAParallelState;
+
+#endif
--
2.1.4
[Qemu-devel] [PATCH RFC for-2.3? 5/8] fdb: Move FDCtrlISABus to header
To be used for embedding the device.
Add gtk-doc private/public markers for parent field.
Signed-off-by: Andreas Färber
---
hw/block/fdc.c | 87 -
include/hw/block/fdc.h | 88 ++
2 files changed, 88 insertions(+), 87 deletions(-)
diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 2bf87c9..da521f1 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -31,7 +31,6 @@
#include "hw/block/fdc.h"
#include "qemu/error-report.h"
#include "qemu/timer.h"
-#include "hw/isa/isa.h"
#include "hw/sysbus.h"
#include "sysemu/block-backend.h"
#include "sysemu/blockdev.h"
@@ -167,33 +166,7 @@ static void pick_geometry(BlockBackend *blk, int *nb_heads,
#define FD_SECTOR_SC 2 /* Sector size code */
#define FD_RESET_SENSEI_COUNT 4 /* Number of sense interrupts on RESET */
-typedef struct FDCtrl FDCtrl;
-
/* Floppy disk drive emulation */
-typedef enum FDiskFlags {
-FDISK_DBL_SIDES = 0x01,
-} FDiskFlags;
-
-typedef struct FDrive {
-FDCtrl *fdctrl;
-BlockBackend *blk;
-/* Drive status */
-FDriveType drive;
-uint8_t perpendicular;/* 2.88 MB access mode*/
-/* Position */
-uint8_t head;
-uint8_t track;
-uint8_t sect;
-/* Media */
-FDiskFlags flags;
-uint8_t last_sect;/* Nb sector per track*/
-uint8_t max_track;/* Nb of tracks */
-uint16_t bps; /* Bytes per sector */
-uint8_t ro; /* Is read-only */
-uint8_t media_changed;/* Is media changed */
-uint8_t media_rate; /* Data rate of medium*/
-} FDrive;
-
static void fd_init(FDrive *drv)
{
/* Drive */
@@ -498,53 +471,6 @@ enum {
#define FD_MULTI_TRACK(state) ((state) & FD_STATE_MULTI)
#define FD_FORMAT_CMD(state) ((state) & FD_STATE_FORMAT)
-struct FDCtrl {
-MemoryRegion iomem;
-qemu_irq irq;
-/* Controller state */
-QEMUTimer *result_timer;
-int dma_chann;
-/* Controller's identification */
-uint8_t version;
-/* HW */
-uint8_t sra;
-uint8_t srb;
-uint8_t dor;
-uint8_t dor_vmstate; /* only used as temp during vmstate */
-uint8_t tdr;
-uint8_t dsr;
-uint8_t msr;
-uint8_t cur_drv;
-uint8_t status0;
-uint8_t status1;
-uint8_t status2;
-/* Command FIFO */
-uint8_t *fifo;
-int32_t fifo_size;
-uint32_t data_pos;
-uint32_t data_len;
-uint8_t data_state;
-uint8_t data_dir;
-uint8_t eot; /* last wanted sector */
-/* States kept only to be returned back */
-/* precompensation */
-uint8_t precomp_trk;
-uint8_t config;
-uint8_t lock;
-/* Power down config (also with status regB access mode */
-uint8_t pwrd;
-/* Floppy drives */
-uint8_t num_floppies;
-/* Sun4m quirks? */
-int sun4m;
-FDrive drives[MAX_FD];
-int reset_sensei;
-uint32_t check_media_rate;
-/* Timers state */
-uint8_t timer0;
-uint8_t timer1;
-};
-
#define TYPE_SYSBUS_FDC "base-sysbus-fdc"
#define SYSBUS_FDC(obj) OBJECT_CHECK(FDCtrlSysBus, (obj), TYPE_SYSBUS_FDC)
@@ -556,19 +482,6 @@ typedef struct FDCtrlSysBus {
struct FDCtrl state;
} FDCtrlSysBus;
-#define ISA_FDC(obj) OBJECT_CHECK(FDCtrlISABus, (obj), TYPE_ISA_FDC)
-
-typedef struct FDCtrlISABus {
-ISADevice parent_obj;
-
-uint32_t iobase;
-uint32_t irq;
-uint32_t dma;
-struct FDCtrl state;
-int32_t bootindexA;
-int32_t bootindexB;
-} FDCtrlISABus;
-
static uint32_t fdctrl_read (void *opaque, uint32_t reg)
{
FDCtrl *fdctrl = opaque;
diff --git a/include/hw/block/fdc.h b/include/hw/block/fdc.h
index d48b2f8..86d852d 100644
--- a/include/hw/block/fdc.h
+++ b/include/hw/block/fdc.h
@@ -2,6 +2,7 @@
#define HW_FDC_H
#include "qemu-common.h"
+#include "hw/isa/isa.h"
/* fdc.c */
#define MAX_FD 2
@@ -13,7 +14,94 @@ typedef enum FDriveType {
FDRIVE_DRV_NONE = 0x03, /* No drive connected */
} FDriveType;
+typedef enum FDiskFlags {
+FDISK_DBL_SIDES = 0x01,
+} FDiskFlags;
+
+typedef struct FDCtrl FDCtrl;
+
+typedef struct FDrive {
+FDCtrl *fdctrl;
+BlockBackend *blk;
+/* Drive status */
+FDriveType drive;
+uint8_t perpendicular;/* 2.88 MB access mode*/
+/* Position */
+uint8_t head;
+uint8_t track;
+uint8_t sect;
+/* Media */
+FDiskFlags flags;
+uint8_t last_sect;/* Nb sector per track*/
+uint8_t max_track;/* Nb of tracks */
+uint16_t bps; /* Bytes per sector */
+uint8_t ro; /* Is read-only */
+uint8_t media_changed;/* Is media changed */
+uint8_t media_rate; /* Data rate of medium*/
+} FDrive;
+
+struct FDCtrl {
+MemoryRegion iomem;
+qemu_irq irq;
+/* Controller state */
+QEMUTimer *result_timer;
+int dma_chann;
+/* Controller's identification */
+
[Qemu-devel] [PATCH RFC for-2.3? 8/8] pc87312: Create IDE in-place
Signed-off-by: Andreas Färber
---
hw/isa/pc87312.c | 17 -
include/hw/isa/pc87312.h | 6 ++
2 files changed, 14 insertions(+), 9 deletions(-)
diff --git a/hw/isa/pc87312.c b/hw/isa/pc87312.c
index d35eb0e..37e2df4 100644
--- a/hw/isa/pc87312.c
+++ b/hw/isa/pc87312.c
@@ -324,14 +324,18 @@ static void pc87312_realize(DeviceState *dev, Error
**errp)
}
if (is_ide_enabled(s)) {
-isa = isa_create(bus, "isa-ide");
-d = DEVICE(isa);
+d = DEVICE(&s->ide);
+qdev_set_parent_bus(d, BUS(bus));
qdev_prop_set_uint32(d, "iobase", get_ide_iobase(s));
qdev_prop_set_uint32(d, "iobase2", get_ide_iobase(s) + 0x206);
-qdev_prop_set_uint32(d, "irq", 14);
-qdev_init_nofail(d);
-s->ide.dev = isa;
trace_pc87312_info_ide(get_ide_iobase(s));
+object_property_set_bool(OBJECT(&s->ide), true, "realized",
+ &local_err);
+object_unref(OBJECT(&s->fdc));
+if (local_err) {
+error_propagate(errp, local_err);
+return;
+}
}
}
@@ -363,6 +367,9 @@ static void pc87312_initfn(Object *obj)
object_property_add_alias(obj, "fdc-driveB",
OBJECT(&s->fdc), "driveB", &error_abort);
qdev_prop_set_uint32(DEVICE(&s->fdc), "irq", 6);
+
+object_initialize(&s->ide, sizeof(s->ide), TYPE_ISA_IDE);
+qdev_prop_set_uint32(DEVICE(&s->ide), "irq", 14);
}
static const VMStateDescription vmstate_pc87312 = {
diff --git a/include/hw/isa/pc87312.h b/include/hw/isa/pc87312.h
index c49a06d..ffc2b96 100644
--- a/include/hw/isa/pc87312.h
+++ b/include/hw/isa/pc87312.h
@@ -29,6 +29,7 @@
#include "hw/char/parallel.h"
#include "hw/char/serial.h"
#include "hw/block/fdc.h"
+#include "hw/ide.h"
#define TYPE_PC87312 "pc87312"
@@ -43,10 +44,7 @@ typedef struct PC87312State {
ISAParallelState parallel;
ISASerialState uart[2];
FDCtrlISABus fdc;
-
-struct {
-ISADevice *dev;
-} ide;
+ISAIDEState ide;
MemoryRegion io;
--
2.1.4
[Qemu-devel] [PATCH RFC for-2.3? 6/8] pc87312: Create FDC in-place
Move drive_get() code to PReP machine.
Signed-off-by: Andreas Färber
---
hw/isa/pc87312.c | 32
hw/ppc/prep.c| 11 +++
include/hw/isa/pc87312.h | 6 ++
3 files changed, 29 insertions(+), 20 deletions(-)
diff --git a/hw/isa/pc87312.c b/hw/isa/pc87312.c
index 207eaa8..d35eb0e 100644
--- a/hw/isa/pc87312.c
+++ b/hw/isa/pc87312.c
@@ -266,7 +266,6 @@ static void pc87312_realize(DeviceState *dev, Error **errp)
DeviceState *d;
ISADevice *isa;
ISABus *bus;
-DriveInfo *drive;
Error *local_err = NULL;
int i;
@@ -311,23 +310,17 @@ static void pc87312_realize(DeviceState *dev, Error
**errp)
}
if (is_fdc_enabled(s)) {
-isa = isa_create(bus, "isa-fdc");
-d = DEVICE(isa);
+d = DEVICE(&s->fdc);
+qdev_set_parent_bus(d, BUS(bus));
qdev_prop_set_uint32(d, "iobase", get_fdc_iobase(s));
-qdev_prop_set_uint32(d, "irq", 6);
-drive = drive_get(IF_FLOPPY, 0, 0);
-if (drive != NULL) {
-qdev_prop_set_drive_nofail(d, "driveA",
- blk_by_legacy_dinfo(drive));
-}
-drive = drive_get(IF_FLOPPY, 0, 1);
-if (drive != NULL) {
-qdev_prop_set_drive_nofail(d, "driveB",
- blk_by_legacy_dinfo(drive));
-}
-qdev_init_nofail(d);
-s->fdc.dev = isa;
trace_pc87312_info_floppy(get_fdc_iobase(s));
+object_property_set_bool(OBJECT(&s->fdc), true, "realized",
+ &local_err);
+object_unref(OBJECT(&s->fdc));
+if (local_err) {
+error_propagate(errp, local_err);
+return;
+}
}
if (is_ide_enabled(s)) {
@@ -363,6 +356,13 @@ static void pc87312_initfn(Object *obj)
&error_abort);
g_free(propname);
}
+
+object_initialize(&s->fdc, sizeof(s->fdc), TYPE_ISA_FDC);
+object_property_add_alias(obj, "fdc-driveA",
+ OBJECT(&s->fdc), "driveA", &error_abort);
+object_property_add_alias(obj, "fdc-driveB",
+ OBJECT(&s->fdc), "driveB", &error_abort);
+qdev_prop_set_uint32(DEVICE(&s->fdc), "irq", 6);
}
static const VMStateDescription vmstate_pc87312 = {
diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index eb29d3c..272a284 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -530,6 +530,7 @@ static void ppc_prep_init(MachineState *machine)
ISABus *isa_bus;
ISADevice *isa;
CharDriverState *chr;
+DriveInfo *drive;
qemu_irq *cpu_exit_irq;
int ppc_boot_device;
DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
@@ -661,6 +662,16 @@ static void ppc_prep_init(MachineState *machine)
g_free(name);
}
}
+drive = drive_get(IF_FLOPPY, 0, 0);
+if (drive != NULL) {
+qdev_prop_set_drive_nofail(dev, "fdc-driveA",
+ blk_by_legacy_dinfo(drive));
+}
+drive = drive_get(IF_FLOPPY, 0, 1);
+if (drive != NULL) {
+qdev_prop_set_drive_nofail(dev, "fdc-driveB",
+ blk_by_legacy_dinfo(drive));
+}
qdev_prop_set_uint8(dev, "config", 13); /* fdc, ser0, ser1, par0 */
qdev_init_nofail(dev);
diff --git a/include/hw/isa/pc87312.h b/include/hw/isa/pc87312.h
index 3256e0f..c49a06d 100644
--- a/include/hw/isa/pc87312.h
+++ b/include/hw/isa/pc87312.h
@@ -28,6 +28,7 @@
#include "hw/isa/isa.h"
#include "hw/char/parallel.h"
#include "hw/char/serial.h"
+#include "hw/block/fdc.h"
#define TYPE_PC87312 "pc87312"
@@ -41,10 +42,7 @@ typedef struct PC87312State {
ISAParallelState parallel;
ISASerialState uart[2];
-
-struct {
-ISADevice *dev;
-} fdc;
+FDCtrlISABus fdc;
struct {
ISADevice *dev;
--
2.1.4
[Qemu-devel] [PATCH RFC for-2.3? 7/8] ide: Move ISAIDEState to header
To be used for embedding the device.
Add gtk-doc private/public markers for parent fields.
Signed-off-by: Andreas Färber
---
hw/ide/internal.h | 155
hw/ide/isa.c | 13
include/hw/ide.h | 173 ++
3 files changed, 173 insertions(+), 168 deletions(-)
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index 965cc55..d22f66d 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -19,12 +19,6 @@
//#define DEBUG_AIO
#define USE_DMA_CDROM
-typedef struct IDEBus IDEBus;
-typedef struct IDEDevice IDEDevice;
-typedef struct IDEState IDEState;
-typedef struct IDEDMA IDEDMA;
-typedef struct IDEDMAOps IDEDMAOps;
-
#define TYPE_IDE_BUS "IDE"
#define IDE_BUS(obj) OBJECT_CHECK(IDEBus, (obj), TYPE_IDE_BUS)
@@ -317,158 +311,9 @@ typedef struct IDEDMAOps IDEDMAOps;
#define SMART_DISABLE 0xd9
#define SMART_STATUS 0xda
-typedef enum { IDE_HD, IDE_CD, IDE_CFATA } IDEDriveKind;
-
-typedef void EndTransferFunc(IDEState *);
-
-typedef void DMAStartFunc(IDEDMA *, IDEState *, BlockCompletionFunc *);
-typedef void DMAVoidFunc(IDEDMA *);
-typedef int DMAIntFunc(IDEDMA *, int);
-typedef int32_t DMAInt32Func(IDEDMA *, int);
-typedef void DMAu32Func(IDEDMA *, uint32_t);
-typedef void DMAStopFunc(IDEDMA *, bool);
-typedef void DMARestartFunc(void *, int, RunState);
-
-struct unreported_events {
-bool eject_request;
-bool new_media;
-};
-
-enum ide_dma_cmd {
-IDE_DMA_READ,
-IDE_DMA_WRITE,
-IDE_DMA_TRIM,
-};
-
#define ide_cmd_is_read(s) \
((s)->dma_cmd == IDE_DMA_READ)
-/* NOTE: IDEState represents in fact one drive */
-struct IDEState {
-IDEBus *bus;
-uint8_t unit;
-/* ide config */
-IDEDriveKind drive_kind;
-int cylinders, heads, sectors, chs_trans;
-int64_t nb_sectors;
-int mult_sectors;
-int identify_set;
-uint8_t identify_data[512];
-int drive_serial;
-char drive_serial_str[21];
-char drive_model_str[41];
-uint64_t wwn;
-/* ide regs */
-uint8_t feature;
-uint8_t error;
-uint32_t nsector;
-uint8_t sector;
-uint8_t lcyl;
-uint8_t hcyl;
-/* other part of tf for lba48 support */
-uint8_t hob_feature;
-uint8_t hob_nsector;
-uint8_t hob_sector;
-uint8_t hob_lcyl;
-uint8_t hob_hcyl;
-
-uint8_t select;
-uint8_t status;
-
-/* set for lba48 access */
-uint8_t lba48;
-BlockBackend *blk;
-char version[9];
-/* ATAPI specific */
-struct unreported_events events;
-uint8_t sense_key;
-uint8_t asc;
-bool tray_open;
-bool tray_locked;
-uint8_t cdrom_changed;
-int packet_transfer_size;
-int elementary_transfer_size;
-int32_t io_buffer_index;
-int lba;
-int cd_sector_size;
-int atapi_dma; /* true if dma is requested for the packet cmd */
-BlockAcctCookie acct;
-BlockAIOCB *pio_aiocb;
-struct iovec iov;
-QEMUIOVector qiov;
-/* ATA DMA state */
-int32_t io_buffer_offset;
-int32_t io_buffer_size;
-QEMUSGList sg;
-/* PIO transfer handling */
-int req_nb_sectors; /* number of sectors per interrupt */
-EndTransferFunc *end_transfer_func;
-uint8_t *data_ptr;
-uint8_t *data_end;
-uint8_t *io_buffer;
-/* PIO save/restore */
-int32_t io_buffer_total_len;
-int32_t cur_io_buffer_offset;
-int32_t cur_io_buffer_len;
-uint8_t end_transfer_fn_idx;
-QEMUTimer *sector_write_timer; /* only used for win2k install hack */
-uint32_t irq_count; /* counts IRQs when using win2k install hack */
-/* CF-ATA extended error */
-uint8_t ext_error;
-/* CF-ATA metadata storage */
-uint32_t mdata_size;
-uint8_t *mdata_storage;
-int media_changed;
-enum ide_dma_cmd dma_cmd;
-/* SMART */
-uint8_t smart_enabled;
-uint8_t smart_autosave;
-int smart_errors;
-uint8_t smart_selftest_count;
-uint8_t *smart_selftest_data;
-/* AHCI */
-int ncq_queues;
-};
-
-struct IDEDMAOps {
-DMAStartFunc *start_dma;
-DMAVoidFunc *start_transfer;
-DMAInt32Func *prepare_buf;
-DMAu32Func *commit_buf;
-DMAIntFunc *rw_buf;
-DMAVoidFunc *restart_dma;
-DMAStopFunc *set_inactive;
-DMAVoidFunc *cmd_done;
-DMAVoidFunc *reset;
-};
-
-struct IDEDMA {
-const struct IDEDMAOps *ops;
-struct iovec iov;
-QEMUIOVector qiov;
-BlockAIOCB *aiocb;
-};
-
-struct IDEBus {
-BusState qbus;
-IDEDevice *master;
-IDEDevice *slave;
-IDEState ifs[2];
-QEMUBH *bh;
-
-int bus_id;
-int max_units;
-IDEDMA *dma;
-uint8_t unit;
-uint8_t cmd;
-qemu_irq irq;
-
-int error_status;
-uint8_t retry_unit;
-int64_t retry_sector_num;
-uint32_t retry_nsector;
-};
-
#define TYPE_IDE_DEVICE "ide-device"
#define IDE_DEVICE(obj) \
OBJECT_CHECK(IDEDevice, (obj), TYPE_IDE_DEVICE)
diff --git a/hw/ide/isa.c b/hw/ide/isa.c
index 9f80503..05d1fd5 100644
--
[Qemu-devel] [Bug 1392504] Re: libvirt not relabeling devices on USB Passthrough
Hey nickmaelao, your outputs show you've only changed the ownership of the USB bus and not the USB device itself...I'd suspect if you looked at 'ls -la /dev/bus/usb/001/' then the actual USB device will still have root ownership. Ergo if libvirtd is still creating the vm's with qemu and a non-root user you will still have the problem. chmod libvirt-qemu:kvm /dev/bus/usb/001/xxx (xxx being the USB device numeber) should resolve. Alternatively you could change /etc/libvirtd/qemu.conf and make libvirtd create guests with qemu as the root user, look for the 'user = ' and 'group = ' lines. I can't comment on the risk associated to this so you'll need to look into that yourself but I've taken this approach and have no problems with USB attachment and I don't need to manually change device ownerships. -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1392504 Title: libvirt not relabeling devices on USB Passthrough Status in QEMU: New Status in libvirt package in Ubuntu: Triaged Bug description: After upgrading from Ubuntu 14.04 to Ubuntu 14.10 USB passthrough in QEMU (version is now 2.1.0 - Debian2.1+dfsg-4ubuntu6.1) is not working any more. Already tried to remove and attach the USB devices. I use 1 USB2 HDD + 1 USB3 HDD to a virtual linux machine; 1 USB2 HDD to a virual FNAS machine and a USB 2camera to virtual Win7 machine. All these devices are not working any more. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1392504/+subscriptions
Re: [Qemu-devel] [PATCH] tcg: optimise memory layout of TCGTemp
On Mar 27, 2015 14:09, "Emilio G. Cota" wrote: > > On Fri, Mar 27, 2015 at 09:55:03 +, Alex Bennée wrote: > > Have you been able to measure any performance improvement with these new > > structures? In theory, if aligned with cache lines, performance should > > improve but real numbers would be nice. > > I haven't benchmarked anything, which makes me very uneasy. All > I've checked is that the system boots, and FWIW I appreciate no > difference in boot time. No decrease in boot time is good. We /know/ we're saving memory, after all. > > Is there a benchmark suite to test TCG changes? No, sorry. r~
[Qemu-devel] [Bug 1392504] Re: libvirt not relabeling devices on USB Passthrough
The syslog output looks like its from the Host not the guest? What does the libvirtd log say for the guest...typically at /var/log/libvirt/qemu/XXX (XXX being the name of the guest). There are 2 separate issues/related in this thread, first being USB attachment to guests not working which is believed ownership related hence my *workaround* in post #23. The second is about how the USB device is re-atrached to the Host once the guest has been destroyed...this is what the syslog/dmesg outputs are about and why Serge has renamed the bug report. Your posts say you are having problem with both which seems odd and not possible? -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1392504 Title: libvirt not relabeling devices on USB Passthrough Status in QEMU: New Status in libvirt package in Ubuntu: Triaged Bug description: After upgrading from Ubuntu 14.04 to Ubuntu 14.10 USB passthrough in QEMU (version is now 2.1.0 - Debian2.1+dfsg-4ubuntu6.1) is not working any more. Already tried to remove and attach the USB devices. I use 1 USB2 HDD + 1 USB3 HDD to a virtual linux machine; 1 USB2 HDD to a virual FNAS machine and a USB 2camera to virtual Win7 machine. All these devices are not working any more. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1392504/+subscriptions
Re: [Qemu-devel] [PATCH target-arm v4 13/16] arm: Add xlnx-ep108 machine
On Mon, Mar 23, 2015 at 9:05 PM, Peter Crosthwaite
wrote:
> Add a machine model for the Xilinx ZynqMP SoC EP108 board.
>
> Signed-off-by: Peter Crosthwaite
Looks good
Reviewed-by: Alistair Francis
Thanks,
Alistair
> ---
> Chaned since v1:
> Change board name to ep108
>
> hw/arm/Makefile.objs | 2 +-
> hw/arm/xlnx-ep108.c | 53
>
> 2 files changed, 54 insertions(+), 1 deletion(-)
> create mode 100644 hw/arm/xlnx-ep108.c
>
> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
> index d7cd5f4..a75a182 100644
> --- a/hw/arm/Makefile.objs
> +++ b/hw/arm/Makefile.objs
> @@ -10,4 +10,4 @@ obj-$(CONFIG_DIGIC) += digic.o
> obj-y += omap1.o omap2.o strongarm.o
> obj-$(CONFIG_ALLWINNER_A10) += allwinner-a10.o cubieboard.o
> obj-$(CONFIG_STM32F205_SOC) += stm32f205_soc.o
> -obj-$(CONFIG_XLNX_ZYNQMP) += xlnx-zynqmp.o
> +obj-$(CONFIG_XLNX_ZYNQMP) += xlnx-zynqmp.o xlnx-ep108.o
> diff --git a/hw/arm/xlnx-ep108.c b/hw/arm/xlnx-ep108.c
> new file mode 100644
> index 000..81704bb
> --- /dev/null
> +++ b/hw/arm/xlnx-ep108.c
> @@ -0,0 +1,53 @@
> +/*
> + * Xilinx ZynqMP EP108 board
> + *
> + * Copyright (C) 2015 Xilinx Inc
> + * Written by Peter Crosthwaite
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful, but
> WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
> + * for more details.
> + */
> +
> +#include "hw/arm/xlnx-zynqmp.h"
> +#include "hw/boards.h"
> +#include "qemu/error-report.h"
> +
> +typedef struct XlnxEP108 {
> +XlnxZynqMPState soc;
> +} XlnxEP108;
> +
> +static void xlnx_ep108_init(MachineState *machine)
> +{
> +XlnxEP108 *s = g_new0(XlnxEP108, 1);
> +Error *err = NULL;
> +
> +object_initialize(&s->soc, sizeof(s->soc), TYPE_XLNX_ZYNQMP);
> +object_property_add_child(OBJECT(machine), "soc", OBJECT(&s->soc),
> + &error_abort);
> +
> +object_property_set_bool(OBJECT(&s->soc), true, "realized", &err);
> +if (err) {
> +error_report("%s", error_get_pretty(err));
> +exit(1);
> +}
> +}
> +
> +static QEMUMachine xlnx_ep108_machine = {
> +.name = "xlnx-ep108",
> +.desc = "Xilinx ZynqMP EP108 board",
> +.init = xlnx_ep108_init,
> +};
> +
> +static void xlnx_ep108_machine_init(void)
> +{
> +qemu_register_machine(&xlnx_ep108_machine);
> +}
> +
> +machine_init(xlnx_ep108_machine_init);
> --
> 2.3.1.2.g90df61e.dirty
>
>
Re: [Qemu-devel] [PATCH target-arm v4 04/16] arm: Introduce Xilinx ZynqMP SoC
On Mon, Mar 23, 2015 at 9:05 PM, Peter Crosthwaite
wrote:
> With quad Cortex-A53 CPUs.
>
> Signed-off-by: Peter Crosthwaite
> ---
> changed since v2:
> Added [*] to cpu child property name.
> changed since v1:
> Add &error_abort to CPU child adder call.
>
> default-configs/aarch64-softmmu.mak | 2 +-
> hw/arm/Makefile.objs| 1 +
> hw/arm/xlnx-zynqmp.c| 72
> +
> include/hw/arm/xlnx-zynqmp.h| 21 +++
> 4 files changed, 95 insertions(+), 1 deletion(-)
> create mode 100644 hw/arm/xlnx-zynqmp.c
> create mode 100644 include/hw/arm/xlnx-zynqmp.h
>
> diff --git a/default-configs/aarch64-softmmu.mak
> b/default-configs/aarch64-softmmu.mak
> index 6d3b5c7..96dd994 100644
> --- a/default-configs/aarch64-softmmu.mak
> +++ b/default-configs/aarch64-softmmu.mak
> @@ -3,4 +3,4 @@
> # We support all the 32 bit boards so need all their config
> include arm-softmmu.mak
>
> -# Currently no 64-bit specific config requirements
> +CONFIG_XLNX_ZYNQMP=y
> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
> index 2577f68..d7cd5f4 100644
> --- a/hw/arm/Makefile.objs
> +++ b/hw/arm/Makefile.objs
> @@ -10,3 +10,4 @@ obj-$(CONFIG_DIGIC) += digic.o
> obj-y += omap1.o omap2.o strongarm.o
> obj-$(CONFIG_ALLWINNER_A10) += allwinner-a10.o cubieboard.o
> obj-$(CONFIG_STM32F205_SOC) += stm32f205_soc.o
> +obj-$(CONFIG_XLNX_ZYNQMP) += xlnx-zynqmp.o
> diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
> new file mode 100644
> index 000..41c207a
> --- /dev/null
> +++ b/hw/arm/xlnx-zynqmp.c
> @@ -0,0 +1,72 @@
> +/*
> + * Xilinx Zynq MPSoC emulation
> + *
> + * Copyright (C) 2015 Xilinx Inc
> + * Written by Peter Crosthwaite
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful, but
> WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
> + * for more details.
> + */
> +
> +#include "hw/arm/xlnx-zynqmp.h"
> +
> +static void xlnx_zynqmp_init(Object *obj)
> +{
> +XlnxZynqMPState *s = XLNX_ZYNQMP(obj);
> +int i;
> +
> +for (i = 0; i < XLNX_ZYNQMP_NUM_CPUS; i++) {
> +object_initialize(&s->cpu[i], sizeof(s->cpu[i]),
> + "cortex-a53-" TYPE_ARM_CPU);
> +object_property_add_child(obj, "cpu[*]", OBJECT(&s->cpu[i]),
> + &error_abort);
> +}
> +}
> +
> +#define ERR_PROP_CHECK_RETURN(err, errp) do { \
> +if (err) { \
> +error_propagate((errp), (err)); \
> +return; \
> +} \
> +} while (0)
> +
> +static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
> +{
> +XlnxZynqMPState *s = XLNX_ZYNQMP(dev);
> +uint8_t i;
> +Error *err = NULL;
> +
> +for (i = 0; i < XLNX_ZYNQMP_NUM_CPUS; i++) {
> +object_property_set_bool(OBJECT(&s->cpu[i]), true, "realized", &err);
> +ERR_PROP_CHECK_RETURN(err, errp);
> +}
> +}
> +
> +static void xlnx_zynqmp_class_init(ObjectClass *oc, void *data)
> +{
> +DeviceClass *dc = DEVICE_CLASS(oc);
> +
> +dc->realize = xlnx_zynqmp_realize;
> +}
> +
> +static const TypeInfo xlnx_zynqmp_type_info = {
> +.name = TYPE_XLNX_ZYNQMP,
> +.parent = TYPE_DEVICE,
> +.instance_size = sizeof(XlnxZynqMPState),
> +.instance_init = xlnx_zynqmp_init,
> +.class_init = xlnx_zynqmp_class_init,
> +};
> +
> +static void xlnx_zynqmp_register_types(void)
> +{
> +type_register_static(&xlnx_zynqmp_type_info);
> +}
> +
> +type_init(xlnx_zynqmp_register_types)
> diff --git a/include/hw/arm/xlnx-zynqmp.h b/include/hw/arm/xlnx-zynqmp.h
> new file mode 100644
> index 000..d6b3b92
> --- /dev/null
> +++ b/include/hw/arm/xlnx-zynqmp.h
> @@ -0,0 +1,21 @@
> +#ifndef XLNX_ZYNQMP_H_
> +
> +#include "qemu-common.h"
> +#include "hw/arm/arm.h"
> +
> +#define TYPE_XLNX_ZYNQMP "xlnx,zynqmp"
> +#define XLNX_ZYNQMP(obj) OBJECT_CHECK(XlnxZynqMPState, (obj), \
> + TYPE_XLNX_ZYNQMP)
> +
> +#define XLNX_ZYNQMP_NUM_CPUS 4
> +
> +typedef struct XlnxZynqMPState {
> +/*< private >*/
> +DeviceState parent_obj;
> +/*< public >*/
I have the same nit pick here as the others, I would just
space it out differently. That's not important though.
Reviewed-by: Alistair Francis
Thanks,
Alistair
> +
> +ARMCPU cpu[XLNX_ZYNQMP_NUM_CPUS];
> +} XlnxZynqMPState;
> +
> +#define XLNX_ZYNQMP_H_
> +#endif
> --
> 2.3.1.2.g90df61e.dirty
>
>
Re: [Qemu-devel] [PATCH target-arm v4 00/16] Next Generation Xilinx Zynq SoC
On Mon, Mar 23, 2015 at 9:05 PM, Peter Crosthwaite wrote: > Hi Peter and all, > > Xilinx's next gen SoC has been announced. This series adds a SoC and > board. > > Series start with addition of ARM cortex A53 support (P1 and P2). The > Soc skeleton is then added with GIC, EMACs and UARTs are added. The > pre-existing models for GEM and UART are not SoC friendly (no visible > state struct), so those are refactored for SoC. > > Create a model of the EP108 board. Currently this doesn't have any > EP108 specific features but is a usable board exposing the user visible > features of the raw SoC. I'm able to boot u-boot using this command: ./aarch64-softmmu/qemu-system-aarch64 -M xlnx-ep108 -nographic -serial mon:stdio -kernel ./u-boot.elf -m 0x800 Tested-by: Alistair Francis Thanks, Alistair > > changed since v3: > Included CPU thread kick fix > Addressed Alistair review > > changed since v2: > Fix CPU child prop adder > Add DTS compat string > > changed since v1: > Addressed Alistair review (individual changes on resp. patches) > Changed board name to EP108 > Changed naming scheme to "zynqmp" / "ZYNQMP" (Michal review) > > Regards, > Peter > > > Peter Crosthwaite (16): > cpus: Don't kick un-realized cpus. > target-arm: cpu64: Factor out ARM cortex init > target-arm: cpu64: Add support for cortex-a53 > arm: Introduce Xilinx ZynqMP SoC > arm: xlnx-zynqmp: Add GIC > arm: xlnx-zynqmp: Connect CPU Timers to GIC > net: cadence_gem: Clean up variable names > net: cadence_gem: Split state struct and type into header > arm: xilinx-zynqmp: Add GEM support > char: cadence_uart: Clean up variable names > char: cadence_uart: Split state struct and type into header > arm: xilinx-zynqmp: Add UART support > arm: Add xlnx-ep108 machine > arm: xilinx-ep108: Add external RAM > arm: xilinx-ep108: Add bootloading > arm: xlnx-zynqmp: Add PSCI setup > > cpus.c | 2 +- > default-configs/aarch64-softmmu.mak | 2 +- > hw/arm/Makefile.objs| 1 + > hw/arm/xlnx-ep108.c | 82 ++ > hw/arm/xlnx-zynqmp.c| 168 > > hw/char/cadence_uart.c | 115 ++-- > hw/net/cadence_gem.c| 95 ++-- > include/hw/arm/xlnx-zynqmp.h| 29 +++ > include/hw/char/cadence_uart.h | 35 > include/hw/net/cadence_gem.h| 49 +++ > target-arm/cpu64.c | 50 --- > 11 files changed, 476 insertions(+), 152 deletions(-) > create mode 100644 hw/arm/xlnx-ep108.c > create mode 100644 hw/arm/xlnx-zynqmp.c > create mode 100644 include/hw/arm/xlnx-zynqmp.h > create mode 100644 include/hw/char/cadence_uart.h > create mode 100644 include/hw/net/cadence_gem.h > > -- > 2.3.1.2.g90df61e.dirty > >
Re: [Qemu-devel] [v3][PATCH 2/2] libxl: introduce gfx_passthru_kind
On 2015/3/27 17:54, Ian Campbell wrote:
On Fri, 2015-03-27 at 09:29 +0800, Chen, Tiejun wrote:
On 2015/3/26 18:06, Ian Campbell wrote:
On Thu, 2015-03-26 at 08:53 +0800, Chen, Tiejun wrote:
Hrm, OK. I suppose we can live with autodetect and igd both meaning igd
and whoever adds a new type will have to remember to add a check for
qemu-trad then.
When we really have to introduce a new type, this means we probably need
to change something inside qemu codes. So a new type should just go into
that table to support qemu upstream since now we shouldn't refactor
anything in qemu-xen-traditional, right?
We'd want to error out on attempts to use qemu-xen-trad with non-IGD
passthru.
On qemu-xen-traditional side, we always recognize this as BOOLEAN,
if (libxl_defbool_val(b_info->u.hvm.gfx_passthru)) {
flexarray_append(dm_args, "-gfx_passthru");
}
Additionally, this is also clarified explicitly in manpage, and
especially we don't change this behavior now, so I'm just wondering why
we should do this :)
If someone does gfx_passthru = "foobar" and device_model_version =
"qemu-xen-traditional" in their xl config then it would be rather mean
of us to silently enable IGD passthru for them instead. When this occurs
libxl should notice and fail.
IGD is currently the only option, so this code would be needed when
someone adds "foobar" support.
Sounds it should be a legacy fix to qemu-xen-tranditional :) So lets do
it now,
@@ -326,6 +326,10 @@ static char **
libxl__build_device_model_args_old(libxl__gc *gc,
}
if (libxl_defbool_val(b_info->u.hvm.gfx_passthru)) {
flexarray_append(dm_args, "-gfx_passthru");
+if (b_info->u.hvm.gfx_passthru_kind >
+LIBXL_GFX_PASSTHRU_KIND_IGD)
+LOG(ERROR, "unsupported device type for
\"gfx_passthru\".\n");
+return NULL;
}
} else {
if (!sdl && !vnc)
Thanks
Tiejun
Re: [Qemu-devel] [PATCH target-arm v4 06/16] arm: xlnx-zynqmp: Connect CPU Timers to GIC
On Mon, Mar 23, 2015 at 9:05 PM, Peter Crosthwaite
wrote:
> Connect the GPIO outputs from the individual CPUs for the timers to the
> GIC.
>
> Signed-off-by: Peter Crosthwaite
> ---
> hw/arm/xlnx-zynqmp.c | 16
> 1 file changed, 16 insertions(+)
>
> diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
> index 9465185..29954f5 100644
> --- a/hw/arm/xlnx-zynqmp.c
> +++ b/hw/arm/xlnx-zynqmp.c
> @@ -19,9 +19,17 @@
>
> #define GIC_NUM_SPI_INTR 128
>
> +#define ARM_PHYS_TIMER_PPI 30
> +#define ARM_VIRT_TIMER_PPI 27
> +
> #define GIC_DIST_ADDR 0xf901
> #define GIC_CPU_ADDR0xf902
Hey Peter,
I'm wondering if the #define's should be in the header file?
>
> +static inline int arm_gic_ppi_index(int cpu_nr, int ppi_index)
> +{
> +return GIC_NUM_SPI_INTR + cpu_nr * 32 + ppi_index;
Should the 32 also be a #define? Everything else is.
Thanks,
Alistair
> +}
> +
> static void xlnx_zynqmp_init(Object *obj)
> {
> XlnxZynqMPState *s = XLNX_ZYNQMP(obj);
> @@ -60,11 +68,19 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error
> **errp)
> sysbus_mmio_map(SYS_BUS_DEVICE(&s->gic), 1, GIC_CPU_ADDR);
>
> for (i = 0; i < XLNX_ZYNQMP_NUM_CPUS; i++) {
> +qemu_irq irq;
> +
> object_property_set_bool(OBJECT(&s->cpu[i]), true, "realized", &err);
> ERR_PROP_CHECK_RETURN(err, errp);
>
> sysbus_connect_irq(SYS_BUS_DEVICE(&s->gic), i,
> qdev_get_gpio_in(DEVICE(&s->cpu[i]),
> ARM_CPU_IRQ));
> +irq = qdev_get_gpio_in(DEVICE(&s->gic),
> + arm_gic_ppi_index(i, ARM_PHYS_TIMER_PPI));
> +qdev_connect_gpio_out(DEVICE(&s->cpu[i]), 0, irq);
> +irq = qdev_get_gpio_in(DEVICE(&s->gic),
> + arm_gic_ppi_index(i, ARM_VIRT_TIMER_PPI));
> +qdev_connect_gpio_out(DEVICE(&s->cpu[i]), 1, irq);
> }
> }
>
> --
> 2.3.1.2.g90df61e.dirty
>
>
Re: [Qemu-devel] [PATCH target-arm v4 14/16] arm: xilinx-ep108: Add external RAM
On Mon, Mar 23, 2015 at 9:05 PM, Peter Crosthwaite
wrote:
> Zynq MPSoC supports external DDR RAM. Add a RAM at 0 to the model.
>
> Signed-off-by: Peter Crosthwaite
Reviewed-by: Alistair Francis
Thanks,
Alistair
> ---
> changed since v1:
> Add ram size clamps and warnings
>
> hw/arm/xlnx-ep108.c | 21 +
> 1 file changed, 21 insertions(+)
>
> diff --git a/hw/arm/xlnx-ep108.c b/hw/arm/xlnx-ep108.c
> index 81704bb..6e89456 100644
> --- a/hw/arm/xlnx-ep108.c
> +++ b/hw/arm/xlnx-ep108.c
> @@ -18,11 +18,16 @@
> #include "hw/arm/xlnx-zynqmp.h"
> #include "hw/boards.h"
> #include "qemu/error-report.h"
> +#include "exec/address-spaces.h"
>
> typedef struct XlnxEP108 {
> XlnxZynqMPState soc;
> +MemoryRegion ddr_ram;
> } XlnxEP108;
>
> +/* Max 2GB RAM */
> +#define EP108_MAX_RAM_SIZE 0x8000ull
> +
> static void xlnx_ep108_init(MachineState *machine)
> {
> XlnxEP108 *s = g_new0(XlnxEP108, 1);
> @@ -37,6 +42,22 @@ static void xlnx_ep108_init(MachineState *machine)
> error_report("%s", error_get_pretty(err));
> exit(1);
> }
> +
> +if (machine->ram_size > EP108_MAX_RAM_SIZE) {
> +error_report("WARNING: RAM size " RAM_ADDR_FMT " above max
> supported, "
> + "reduced to %llx", machine->ram_size,
> EP108_MAX_RAM_SIZE);
> +machine->ram_size = EP108_MAX_RAM_SIZE;
> +}
> +
> +if (machine->ram_size <= 0x0800) {
> +error_report("WARNING: RAM size " RAM_ADDR_FMT " is small for
> EP108\n",
> + machine->ram_size);
> +}
> +
> +memory_region_init_ram(&s->ddr_ram, NULL, "ddr-ram", machine->ram_size,
> + &error_abort);
> +vmstate_register_ram_global(&s->ddr_ram);
> +memory_region_add_subregion(get_system_memory(), 0, &s->ddr_ram);
> }
>
> static QEMUMachine xlnx_ep108_machine = {
> --
> 2.3.1.2.g90df61e.dirty
>
>
Re: [Qemu-devel] [PATCH target-arm v4 15/16] arm: xilinx-ep108: Add bootloading
On Mon, Mar 23, 2015 at 9:05 PM, Peter Crosthwaite
wrote:
> Using standard ARM bootloader.
>
> Signed-off-by: Peter Crosthwaite
Reviewed-by: Alistair Francis
Thanks,
Alistair
> ---
> hw/arm/xlnx-ep108.c | 8
> 1 file changed, 8 insertions(+)
>
> diff --git a/hw/arm/xlnx-ep108.c b/hw/arm/xlnx-ep108.c
> index 6e89456..a86f595 100644
> --- a/hw/arm/xlnx-ep108.c
> +++ b/hw/arm/xlnx-ep108.c
> @@ -28,6 +28,8 @@ typedef struct XlnxEP108 {
> /* Max 2GB RAM */
> #define EP108_MAX_RAM_SIZE 0x8000ull
>
> +static struct arm_boot_info xlnx_ep108_binfo;
> +
> static void xlnx_ep108_init(MachineState *machine)
> {
> XlnxEP108 *s = g_new0(XlnxEP108, 1);
> @@ -58,6 +60,12 @@ static void xlnx_ep108_init(MachineState *machine)
> &error_abort);
> vmstate_register_ram_global(&s->ddr_ram);
> memory_region_add_subregion(get_system_memory(), 0, &s->ddr_ram);
> +
> +xlnx_ep108_binfo.ram_size = machine->ram_size;
> +xlnx_ep108_binfo.kernel_filename = machine->kernel_filename;
> +xlnx_ep108_binfo.kernel_cmdline = machine->kernel_cmdline;
> +xlnx_ep108_binfo.initrd_filename = machine->initrd_filename;
> +arm_load_kernel(&s->soc.cpu[0], &xlnx_ep108_binfo);
> }
>
> static QEMUMachine xlnx_ep108_machine = {
> --
> 2.3.1.2.g90df61e.dirty
>
>
Re: [Qemu-devel] [PATCH for-2.3?] qom: Fix object_property_add_alias() with [*]
On 2015/3/30 1:19, Andreas Färber wrote: > Commit 8074264 (qom: Add description field in ObjectProperty struct) > introduced property descriptions and copied them for alias properties. > > Instead of using the caller-supplied property name, use the returned > property name for setting the description. This avoids an Error when > setting a property description for a property with literal "[*]" that > doesn't exist due to automatic property naming in object_property_add(). > > Cc: Gonglei > Cc: Paolo Bonzini > Cc: Michael S. Tsirkin > Signed-off-by: Andreas Färber > --- > qom/object.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/qom/object.c b/qom/object.c > index d167038..b8dff43 100644 > --- a/qom/object.c > +++ b/qom/object.c > @@ -1761,7 +1761,7 @@ void object_property_add_alias(Object *obj, const char > *name, > } > op->resolve = property_resolve_alias; > > -object_property_set_description(obj, name, > +object_property_set_description(obj, op->name, > target_prop->description, > &error_abort); > Looks good to me. I think this is a candidate for 2.3 rc2. Cc: qemu-stable Reviewed-by: Gonglei Regards, -Gonglei
Re: [Qemu-devel] [PATCH v2] spapr: populate ibm,loc-code
On 03/27/2015 08:49 PM, Nikunj A Dadhania wrote:
Each hardware instance has a platform unique location code. The OF
device tree that describes a part of a hardware entity must include
the “ibm,loc-code” property with a value that represents the location
code for that hardware entity.
Introduce an hcall to populate ibm,loc-code.
1) PCI passthru devices need to identify with its own ibm,loc-code
available on the host.
2) Emulated devices encode as following: qemu_:.
Signed-off-by: Nikunj A Dadhania
---
Changelog
v1:
* Dropped is_vfio patch and using TYPE_SPAPR_PCI_VFIO_HOST_BRIDGE
to recognise vfio devices
* Removed wrapper for hcall
* Added sPAPRPHBClass::get_loc_code
hw/ppc/spapr_hcall.c| 1 +
hw/ppc/spapr_pci.c | 38 +
hw/ppc/spapr_pci_vfio.c | 51 +
include/hw/pci-host/spapr.h | 1 +
include/hw/ppc/spapr.h | 8 ++-
5 files changed, 98 insertions(+), 1 deletion(-)
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 4f76f1c..b394681 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -1010,6 +1010,7 @@ static void hypercall_register_types(void)
/* ibm,client-architecture-support support */
spapr_register_hypercall(KVMPPC_H_CAS, h_client_architecture_support);
+spapr_register_hypercall(KVMPPC_H_GET_LOC_CODE, phb_get_loc_code);
}
type_init(hypercall_register_types)
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 05f4fac..c2ee476 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -248,6 +248,44 @@ static void spapr_msi_setmsg(PCIDevice *pdev, hwaddr addr,
bool msix,
}
}
+target_ulong phb_get_loc_code(PowerPCCPU *cpu, sPAPREnvironment *spapr,
+ target_ulong opcode, target_ulong *args)
+{
+sPAPRPHBState *sphb = NULL;
+sPAPRPHBClass *spc = NULL;
+char *buf = NULL, path[PATH_MAX];
+PCIDevice *pdev;
+target_ulong buid = args[0];
+target_ulong config_addr = args[1];
+target_ulong loc_code = args[2];
+target_ulong size = args[3];
+
+sphb = find_phb(spapr, buid);
+pdev = find_dev(spapr, buid, config_addr);
+
+if (!sphb || !pdev) {
+return H_PARAMETER;
+}
+
+spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb);
+if (spc->get_loc_code && spc->get_loc_code(sphb, pdev, &buf)) {
+cpu_physical_memory_write(loc_code, buf, strlen(buf));
+g_free(buf);
+return H_SUCCESS;
+}
+
+/*
+ * For non-vfio devices and failures make up the location code out
+ * of the name, slot and function.
+ *
+ * qemu_:.
+ */
+snprintf(path, sizeof(path), "qemu_%s:%02d.%1d", pdev->name,
+ PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn));
+cpu_physical_memory_write(loc_code, path, size);
Move the chunk above to a sPAPRPHBState::get_loc_code. And I'd add PHB's
@index in the device name to make it unique across the guest.
+return H_SUCCESS;
+}
+
static void rtas_ibm_change_msi(PowerPCCPU *cpu, sPAPREnvironment *spapr,
uint32_t token, uint32_t nargs,
target_ulong args, uint32_t nret,
diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c
index 99a1be5..bfdfa67 100644
--- a/hw/ppc/spapr_pci_vfio.c
+++ b/hw/ppc/spapr_pci_vfio.c
@@ -171,6 +171,56 @@ static int spapr_phb_vfio_eeh_reset(sPAPRPHBState *sphb,
int option)
return RTAS_OUT_SUCCESS;
}
+static bool spapr_phb_vfio_get_devspec(PCIDevice *pdev, char **value)
s/value/devspec/ ?
+{
+char *host;
+char path[PATH_MAX];
+struct stat st;
+
+host = object_property_get_str(OBJECT(pdev), "host", NULL);
+if (!host) {
+return false;
+}
+
+snprintf(path, sizeof(path), "/sys/bus/pci/devices/%s/devspec", host);
+g_free(host);
+if (stat(path, &st) < 0) {
+return false;
+}
+
+return g_file_get_contents(path, value, NULL, NULL);
g_file_get_contents() is expected to return FALSE if the file is missing so
stat() seems redundant here.
+}
+
+static int spapr_phb_vfio_get_loc_code(sPAPRPHBState *sphb, PCIDevice *pdev,
+char **loc_code)
+{
+sPAPRPHBVFIOState *svphb = SPAPR_PCI_VFIO_HOST_BRIDGE(sphb);
+char path[PATH_MAX], *buf = NULL;
+struct stat st;
+
+/* Non VFIO devices */
+if (!svphb) {
+return false;
The function returns int, not bool. s/false/-1/ and s/true/0/ please.
+}
+
+/* We have a vfio host bridge lets get the path. */
+if (!spapr_phb_vfio_get_devspec(pdev, &buf)) {
s/buf/devspec/
+return false;
+}
+
+snprintf(path, sizeof(path), "/proc/device-tree%s/ibm,loc-code", buf);
+g_free(buf);
+if (stat(path, &st) < 0) {
+return false;
Just return what stat() returned.
+}
+
+/* A valid file, now read the loc-code */
+if (g_file_get_contents(path, loc_cod
Re: [Qemu-devel] [PATCH v2] spapr: populate ibm,loc-code
On Mon, Mar 30, 2015 at 01:18:01PM +1100, Alexey Kardashevskiy wrote:
> On 03/27/2015 08:49 PM, Nikunj A Dadhania wrote:
> >Each hardware instance has a platform unique location code. The OF
> >device tree that describes a part of a hardware entity must include
> >the “ibm,loc-code” property with a value that represents the location
> >code for that hardware entity.
> >
> >Introduce an hcall to populate ibm,loc-code.
> >1) PCI passthru devices need to identify with its own ibm,loc-code
> >available on the host.
> >2) Emulated devices encode as following: qemu_:.
> >
> >Signed-off-by: Nikunj A Dadhania
[snip]
> >diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> >index af71e8b..95157ac 100644
> >--- a/include/hw/ppc/spapr.h
> >+++ b/include/hw/ppc/spapr.h
> >@@ -310,7 +310,10 @@ typedef struct sPAPREnvironment {
> > #define KVMPPC_H_LOGICAL_MEMOP (KVMPPC_HCALL_BASE + 0x1)
> > /* Client Architecture support */
> > #define KVMPPC_H_CAS(KVMPPC_HCALL_BASE + 0x2)
> >-#define KVMPPC_HCALL_MAXKVMPPC_H_CAS
> >+#define KVMPPC_H_RTAS_UPDATE(KVMPPC_HCALL_BASE + 0x3)
> >+#define KVMPPC_H_REPORT_MC_ERR (KVMPPC_HCALL_BASE + 0x4)
> >+#define KVMPPC_H_GET_LOC_CODE (KVMPPC_HCALL_BASE + 0x5)
> >+#define KVMPPC_HCALL_MAXKVMPPC_H_GET_LOC_CODE
>
>
> Please add only relevant codes. And what happened to patches adding
> H_RTAS_UPDATE and H_REPORT_MC_ERR?
>
> Also (it is probably a very stupid question but still :) ), why are all
> these callbacks - hypercalls, not RTAS calls? The hypercalls are numbered in
> sPAPR and we kind of stealing numbers from that space while we are
> allocating RTAS tokens ourselves and have more freedom.
Also, I thought the plan was to remove PCI device enumeration from
SLOF and move it to qemu (since we need to partially do that for
hotplug). That removes the need for the hcall entirely.
--
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
pgp7ECpz2pAAE.pgp
Description: PGP signature
Re: [Qemu-devel] [PATCH 2/2] sPAPR: Reenable EEH functionality on reboot
On Thu, Mar 26, 2015 at 04:35:02PM +1100, Gavin Shan wrote: > When rebooting the guest, some PEs might be in frozen state. The > contained PCI devices won't work properly if their frozen states > aren't cleared in time. One case running into this situation would > be maximal EEH error times encountered in the guest. > > The patch reenables the EEH functinality on PEs on PHB's reset > callback, which will clear their frozen states if needed. > > Signed-off-by: Gavin Shan Reviewed-by: David Gibson -- 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 pgpbd6sYqnh8U.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH v3 1/2] VFIO: Clear stale MSIx table during EEH reset
On Thu, Mar 26, 2015 at 04:35:01PM +1100, Gavin Shan wrote:
> The PCI device MSIx table is cleaned out in hardware after EEH PE
> reset. However, we still hold the stale MSIx entries in QEMU, which
> should be cleared accordingly. Otherwise, we will run into another
> (recursive) EEH error and the PCI devices contained in the PE have
> to be offlined exceptionally.
>
> The patch introduces function vfio_eeh_pe_reset(), which is called
> by sPAPR when asserting hot or fundamental reset, to clear stale MSIx
> table before EEH PE reset so that MSIx table could be restored properly
> after EEH PE reset.
>
> Signed-off-by: Gavin Shan
> ---
> hw/ppc/spapr_pci_vfio.c | 13 +
> hw/vfio/Makefile.objs | 6 +-
> hw/vfio/pci-stub.c | 16
> hw/vfio/pci.c | 36
> include/hw/vfio/vfio.h | 2 ++
> 5 files changed, 68 insertions(+), 5 deletions(-)
> create mode 100644 hw/vfio/pci-stub.c
>
> diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c
> index 99a1be5..6fa3afe 100644
> --- a/hw/ppc/spapr_pci_vfio.c
> +++ b/hw/ppc/spapr_pci_vfio.c
> @@ -151,19 +151,24 @@ static int spapr_phb_vfio_eeh_reset(sPAPRPHBState
> *sphb, int option)
> switch (option) {
> case RTAS_SLOT_RESET_DEACTIVATE:
> op.op = VFIO_EEH_PE_RESET_DEACTIVATE;
> +ret = vfio_container_ioctl(&svphb->phb.iommu_as,
> + svphb->iommugroupid,
> + VFIO_EEH_PE_OP, &op);
For consistency, I think all the reset operations should go through
vfio_eeh_pe_reset(), even though in this case it won't do more than
call vfio_container_ioctl().
> break;
> case RTAS_SLOT_RESET_HOT:
> -op.op = VFIO_EEH_PE_RESET_HOT;
> +ret = vfio_eeh_pe_reset(&svphb->phb.iommu_as,
> +svphb->iommugroupid,
> +VFIO_EEH_PE_RESET_HOT);
> break;
> case RTAS_SLOT_RESET_FUNDAMENTAL:
> -op.op = VFIO_EEH_PE_RESET_FUNDAMENTAL;
> +ret = vfio_eeh_pe_reset(&svphb->phb.iommu_as,
> +svphb->iommugroupid,
> +VFIO_EEH_PE_RESET_FUNDAMENTAL);
> break;
> default:
> return RTAS_OUT_PARAM_ERROR;
> }
>
> -ret = vfio_container_ioctl(&svphb->phb.iommu_as, svphb->iommugroupid,
> - VFIO_EEH_PE_OP, &op);
> if (ret < 0) {
> return RTAS_OUT_HW_ERROR;
> }
> diff --git a/hw/vfio/Makefile.objs b/hw/vfio/Makefile.objs
> index e31f30e..1b8a065 100644
> --- a/hw/vfio/Makefile.objs
> +++ b/hw/vfio/Makefile.objs
> @@ -1,4 +1,8 @@
> ifeq ($(CONFIG_LINUX), y)
> obj-$(CONFIG_SOFTMMU) += common.o
> -obj-$(CONFIG_PCI) += pci.o
> +ifeq ($(CONFIG_PCI), y)
> +obj-y += pci.o
> +else
> +obj-y += pci-stub.o
> +endif
> endif
> diff --git a/hw/vfio/pci-stub.c b/hw/vfio/pci-stub.c
> new file mode 100644
> index 000..f317c1e
> --- /dev/null
> +++ b/hw/vfio/pci-stub.c
> @@ -0,0 +1,16 @@
> +/*
> + * To include the file on !CONFIG_PCI
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2. See
> + * the COPYING file in the top-level directory.
> + */
> +
> +#include
> +
> +#include "exec/memory.h"
> +#include "hw/vfio/vfio.h"
> +
> +int vfio_eeh_pe_reset(AddressSpace *as, int32_t groupid, uint32_t option)
> +{
> +return -1;
Probably should have assert(0) here - this should never be called if
!CONFIG_PCI.
> +}
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 6b80539..d0fd4b4 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -3319,6 +3319,42 @@ static void vfio_unregister_req_notifier(VFIOPCIDevice
> *vdev)
> vdev->req_enabled = false;
> }
>
> +int vfio_eeh_pe_reset(AddressSpace *as, int32_t groupid, uint32_t option)
> +{
> +VFIOGroup *group;
> +VFIODevice *vbasedev;
> +VFIOPCIDevice *vdev;
> +struct vfio_eeh_pe_op op = {
> +.argsz = sizeof(op),
> +.op = option
> +};
> +
> +group = vfio_get_group(groupid, as);
> +if (!group) {
> +error_report("vfio: group %d not found\n", groupid);
> +return -1;
> +}
> +
> +/*
> + * The MSIx table will be cleaned out by reset. We need
> + * disable it so that it can be reenabled properly. Also,
> + * the cached MSIx table should be cleared as it's not
> + * reflecting the contents in hardware.
> + */
> +QLIST_FOREACH(vbasedev, &group->device_list, next) {
> +vdev = container_of(vbasedev, VFIOPCIDevice, vbasedev);
> +if (msix_enabled(&vdev->pdev)) {
> +vfio_disable_msix(vdev);
> +}
> +
> +msix_reset(&vdev->pdev);
> +}
> +
> +vfio_put_group(group);
> +
> +return vfio_container_ioctl(as, groupid, VFIO_EEH_PE_OP, &op);
> +}
> +
> static int vfio_initfn(PCIDevice *pdev)
> {
> VFIOPCIDevice *vdev = DO_UPCAST(VFIOPCIDevice, pdev, pdev);
> diff --git a/include/
Re: [Qemu-devel] [PATCH 0/6] Clean up ISA dependencies so we make ISA optional to build
On Tue, Mar 10, 2015 at 10:56:22AM -0400, Luiz Capitulino wrote: > On Tue, 10 Mar 2015 15:20:29 +0100 > "Michael S. Tsirkin" wrote: > > > On Fri, Mar 06, 2015 at 03:18:20PM +1100, David Gibson wrote: > > > At present, ISA bus support is always included in the build for all > > > targets. However these days there are a number of targets that have > > > never had ISA, and even more where many of the individual machines > > > don't have ISA. > > > > > > Unfortunately there are some awkward dependencies in the core code on > > > ISA, although b19c1c0 "isa: remove isa_mem_base variable" did already > > > remove one. > > > > > > This series engages in some yak shaving to make the necessary > > > dependency cleanups, then make inclusion of ISA support optional. > > > > For PC/PCI changes > > Acked-by: Michael S. Tsirkin > > > > > > > > > > Given the date, this is obviously aimed at qemu 2.4, not 2.3. > > > > Looks like the date for 2.3 is unclear, so it might be > > ok to merge. > > > > Who's taking this? Luiz? most changes are monitor-related. > > I can take them, but the most important question for me is > who's reviewing them? I could do it, but not right now. Any further thoughts on this? I don't know who would be suitable for review, beyond those I've already CCed. -- 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 pgpbA02Bx0dwL.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH v5 0/7] QEMU memory hot unplug support
Question: When migrating from old qemu to one with this hot remove feature, I found this feature would not work any more in new qemu. The reason is that DSDT table will not re-generate in new qemu when migration. So the hot remove will not work in the new qemu even though qemu has this feature. Is it possible to re-generate dsdt table when migration? Thanks, Zhu On 03/27/2015 05:20 PM, Zhu Guihua wrote: Memory hot unplug are both asynchronous procedures. When the unplug operation happens, unplug request cb is called first. And when guest OS finished handling unplug, unplug cb will be called to do the real removal of device. v5: -reorganize the patchset -add documentation to understand patch easily -add MEMORY_SLOT_EJECT for initiating device eject -add support to send qmp event to notify mgmt about memory unplug error v4: -reorganize the patchset -drop the new API acpi_send_gpe_event() -update ssdt-mem v3: -commit message changes -reorganize the patchset, squash and separate some patches -update specs about acpi_mem_hotplug -first cleanup external state, then un-map and un-register memory device v2: -do a generic for acpi to send gpe event -unparent object by PC_MACHINE -update description in acpi_mem_hotplug.txt -combine the last two patches in the last version -cleanup external state in acpi_memory_unplug_cb Tang Chen (3): acpi, mem-hotplug: add acpi_memory_slot_status() to get MemStatus acpi, mem-hotplug: add unplug request cb for memory device acpi, mem-hotplug: add unplug cb for memory device Zhu Guihua (4): docs: update documentation for memory hot unplug acpi: extend aml_field() to support UpdateRule acpi: add hardware implementation for memory hot unplug qmp-event: add event notification for memory hot unplug error docs/memory-hotplug.txt | 24 -- docs/qmp/qmp-events.txt | 17 +++ docs/specs/acpi_mem_hotplug.txt | 32 -- hw/acpi/aml-build.c | 4 +- hw/acpi/ich9.c| 19 ++-- hw/acpi/memory_hotplug.c | 93 --- hw/acpi/piix4.c | 17 +-- hw/core/qdev.c| 2 +- hw/i386/acpi-build.c | 25 --- hw/i386/acpi-dsdt-mem-hotplug.dsl | 13 +- hw/i386/pc.c | 62 -- include/hw/acpi/aml-build.h | 10 - include/hw/acpi/memory_hotplug.h | 12 + include/hw/acpi/pc-hotplug.h | 3 ++ include/hw/qdev-core.h| 1 + monitor.c | 1 + qapi/event.json | 14 ++ trace-events | 2 + 18 files changed, 315 insertions(+), 36 deletions(-)
Re: [Qemu-devel] [PATCH v2] spapr: populate ibm,loc-code
On 03/30/2015 01:25 PM, David Gibson wrote:
On Mon, Mar 30, 2015 at 01:18:01PM +1100, Alexey Kardashevskiy wrote:
On 03/27/2015 08:49 PM, Nikunj A Dadhania wrote:
Each hardware instance has a platform unique location code. The OF
device tree that describes a part of a hardware entity must include
the “ibm,loc-code” property with a value that represents the location
code for that hardware entity.
Introduce an hcall to populate ibm,loc-code.
1) PCI passthru devices need to identify with its own ibm,loc-code
available on the host.
2) Emulated devices encode as following: qemu_:.
Signed-off-by: Nikunj A Dadhania
[snip]
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index af71e8b..95157ac 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -310,7 +310,10 @@ typedef struct sPAPREnvironment {
#define KVMPPC_H_LOGICAL_MEMOP (KVMPPC_HCALL_BASE + 0x1)
/* Client Architecture support */
#define KVMPPC_H_CAS(KVMPPC_HCALL_BASE + 0x2)
-#define KVMPPC_HCALL_MAXKVMPPC_H_CAS
+#define KVMPPC_H_RTAS_UPDATE(KVMPPC_HCALL_BASE + 0x3)
+#define KVMPPC_H_REPORT_MC_ERR (KVMPPC_HCALL_BASE + 0x4)
+#define KVMPPC_H_GET_LOC_CODE (KVMPPC_HCALL_BASE + 0x5)
+#define KVMPPC_HCALL_MAXKVMPPC_H_GET_LOC_CODE
Please add only relevant codes. And what happened to patches adding
H_RTAS_UPDATE and H_REPORT_MC_ERR?
Also (it is probably a very stupid question but still :) ), why are all
these callbacks - hypercalls, not RTAS calls? The hypercalls are numbered in
sPAPR and we kind of stealing numbers from that space while we are
allocating RTAS tokens ourselves and have more freedom.
Also, I thought the plan was to remove PCI device enumeration from
SLOF and move it to qemu (since we need to partially do that for
hotplug). That removes the need for the hcall entirely.
There was a strong opposition to PCI scan done by QEMU (although it was ok
if PCI hotplug does some resource assignment in QEMU). Has this changed?
I added Michael in cc: and hope Alexander may enlighten us on this topic...
--
Alexey
Re: [Qemu-devel] [PATCH v5 7/7] qmp-event: add event notification for memory hot unplug error
On 03/27/2015 10:53 PM, Eric Blake wrote:
On 03/27/2015 03:20 AM, Zhu Guihua wrote:
When memory hot unplug fails, this patch adds support to send
QMP event to notify mgmt about this failure.
Signed-off-by: Zhu Guihua
---
docs/qmp/qmp-events.txt | 17 +
hw/acpi/memory_hotplug.c | 8 +++-
monitor.c| 1 +
qapi/event.json | 14 ++
4 files changed, 39 insertions(+), 1 deletion(-)
diff --git a/docs/qmp/qmp-events.txt b/docs/qmp/qmp-events.txt
index d759d19..7a05705 100644
--- a/docs/qmp/qmp-events.txt
+++ b/docs/qmp/qmp-events.txt
@@ -226,6 +226,23 @@ Example:
{ "event": "GUEST_PANICKED",
"data": { "action": "pause" } }
+MEM_HOT_UNPLUG_ERROR
+
+Emitted when memory hot unplug occurs error.
s/occurs error/error occurs/
+
+Data:
+
+- "device": device name (json-string)
+- "msg": Informative message (e.g., reason for the error) (json-string)
+
+Example:
+
+{ "event": "MEM_HOT_UNPLUG_ERROR"
+ "data": { "device": "dimm1",
+"msg": "acpi: device unplug for not supported device"
s/not supported/unsupported/
+ },
+ "timestamp": { "seconds": 1265044230, "microseconds": 450486 } }
+
NIC_RX_FILTER_CHANGED
-
diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
index 2a1b866..f1cef71 100644
--- a/hw/acpi/memory_hotplug.c
+++ b/hw/acpi/memory_hotplug.c
@@ -94,6 +94,7 @@ static void acpi_memory_hotplug_write(void *opaque, hwaddr
addr, uint64_t data,
ACPIOSTInfo *info;
DeviceState *dev = NULL;
HotplugHandler *hotplug_ctrl = NULL;
+Error *local_err = NULL;
if (!mem_st->dev_count) {
return;
@@ -148,7 +149,12 @@ static void acpi_memory_hotplug_write(void *opaque, hwaddr
addr, uint64_t data,
dev = DEVICE(mdev->dimm);
hotplug_ctrl = qdev_get_hotplug_handler(dev);
/* call pc-dimm unplug cb */
-hotplug_handler_unplug(hotplug_ctrl, dev, NULL);
+hotplug_handler_unplug(hotplug_ctrl, dev, &local_err);
+if (local_err) {
+qapi_event_send_mem_unplug_error(dev->id,
+ error_get_pretty(local_err),
+ &error_abort);
+}
}
Does this leak local_err?
Here should handle this error not to forget it.
It is a good way to send qmp event to notify mgmt.
+++ b/qapi/event.json
@@ -330,3 +330,17 @@
##
{ 'event': 'VSERPORT_CHANGE',
'data': { 'id': 'str', 'open': 'bool' } }
+
+##
+# @MEM_UNPLUG_ERROR
+#
+# Emitted when memory hot unplug occurs error.
s/occurs error/error occurs/
+#
+# @device: device name
+#
+# @msg: Informative message
+#
+# Since: 2.3
You're awfully late for 2.3; this may need to be 2.4.
Got it, thanks.
Regards,
Zhu
+##
+{ 'event': 'MEM_UNPLUG_ERROR',
+ 'data': { 'device': 'str', 'msg': 'str' } }
Re: [Qemu-devel] [PATCH v5 17/45] Add wrappers and handlers for sending/receiving the postcopy-ram migration messages.
On Fri, Mar 27, 2015 at 10:48:52AM +, Dr. David Alan Gilbert wrote:
> * David Gibson (da...@gibson.dropbear.id.au) wrote:
> > On Thu, Mar 26, 2015 at 04:33:28PM +, Dr. David Alan Gilbert wrote:
> > > (Only replying to some of the items in this mail - the others I'll get
> > > to another time).
[snip]
> > > > DISCARD will typically immediately precede LISTEN, won't it? Is there
> > > > a reason not to put the discard data into the LISTEN command?
> > >
> > > Discard data can be quite large, so I potentially send multiple discard
> > > commands.
> > > (Also as you can tell generally I've got a preference for one message
> > > does one
> > > thing, and thus I have tried to keep them separate).
> >
> > So, I like the idea of one message per action in principle - but only
> > if that action really is well-defined without reference to what
> > operations come before and after it. If there are hidden dependencies
> > about what actions have to come in what order, I'd rather bake that
> > into the command structure.
>
> In no way is it hidden; the commands match the state transitions.
Not all of them. In particular DISCARD's relation to state
transitions is not at all obvious. Likewise I don't think the
connection of LISTEN to the internal state machines at each end is
terribly clear.
> > > > > +static int loadvm_postcopy_handle_advise(MigrationIncomingState *mis,
> > > > > + uint64_t remote_hps,
> > > > > + uint64_t remote_tps)
> > > > > +{
> > > > > +PostcopyState ps = postcopy_state_get(mis);
> > > > > +trace_loadvm_postcopy_handle_advise();
> > > > > +if (ps != POSTCOPY_INCOMING_NONE) {
> > > > > +error_report("CMD_POSTCOPY_ADVISE in wrong postcopy state
> > > > > (%d)", ps);
> > > > > +return -1;
> > > > > +}
> > > > > +
> > > > > +if (remote_hps != getpagesize()) {
> > > > > +/*
> > > > > + * Some combinations of mismatch are probably possible but
> > > > > it gets
> > > > > + * a bit more complicated. In particular we need to place
> > > > > whole
> > > > > + * host pages on the dest at once, and we need to ensure
> > > > > that we
> > > > > + * handle dirtying to make sure we never end up sending part
> > > > > of
> > > > > + * a hostpage on it's own.
> > > > > + */
> > > > > +error_report("Postcopy needs matching host page sizes (s=%d
> > > > > d=%d)",
> > > > > + (int)remote_hps, getpagesize());
> > > > > +return -1;
> > > > > +}
> > > > > +
> > > > > +if (remote_tps != (1ul << qemu_target_page_bits())) {
> > > > > +/*
> > > > > + * Again, some differences could be dealt with, but for now
> > > > > keep it
> > > > > + * simple.
> > > > > + */
> > > > > +error_report("Postcopy needs matching target page sizes
> > > > > (s=%d d=%d)",
> > > > > + (int)remote_tps, 1 << qemu_target_page_bits());
> > > > > +return -1;
> > > > > +}
> > > > > +
> > > > > +postcopy_state_set(mis, POSTCOPY_INCOMING_ADVISE);
> > > >
> > > > Should you be checking the return value here to make sure it's still
> > > > POSTCOPY_INCOMING_NONE? Atomic xchgs seem overkill if you still have
> > > > a race between the fetch at the top and the set here.
> > > >
> > > > Or, in fact, should you be just doing an atomic exchange-then-check at
> > > > the top, rather than checking at the top, then changing at the bottom.
> > >
> > > There's no race at this point yet; going from None->advise we still only
> > > have one thread. The check at the top is a check against protocol
> > > violations (e.g. getting two advise or something like that).
> >
> > Yeah.. and having to have intimate knowledge of the thread structure
> > at each point in order to analyze the atomic operation correctness is
> > exactly what bothers me about it.
>
> No, you don't; By always using the postcopy_state_set you don't need
> to worry about the thread structure or protocol to understand the atomic
> operation
> correctness. The only reason you got into that comparison is because
> you worried that it might be overkill in this case; by keeping it consistent
> like it already is then you don't need to understand the thread structure.
You really do, though. You may be using the same function in the
original, but not the same idiom: in this case you don't check the
return value and deal with it accordingly.
Because of that, this looks pretty much exactly like the classic
"didn't understand what was atomic in the atomic" race condition, and
you need to know that there's only one thread at this point to realize
it's correct after all.
--
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.ozlab
Re: [Qemu-devel] [PATCH v2] spapr: populate ibm,loc-code
Alexey Kardashevskiy writes:
> On 03/27/2015 08:49 PM, Nikunj A Dadhania wrote:
>> Each hardware instance has a platform unique location code. The OF
>> device tree that describes a part of a hardware entity must include
>> the “ibm,loc-code” property with a value that represents the location
>> code for that hardware entity.
>>
>> Introduce an hcall to populate ibm,loc-code.
>> 1) PCI passthru devices need to identify with its own ibm,loc-code
>> available on the host.
>> 2) Emulated devices encode as following: qemu_:.
>>
>> Signed-off-by: Nikunj A Dadhania
>> ---
>>
>> Changelog
>> v1:
>> * Dropped is_vfio patch and using TYPE_SPAPR_PCI_VFIO_HOST_BRIDGE
>>to recognise vfio devices
>> * Removed wrapper for hcall
>> * Added sPAPRPHBClass::get_loc_code
>>
>> hw/ppc/spapr_hcall.c| 1 +
>> hw/ppc/spapr_pci.c | 38 +
>> hw/ppc/spapr_pci_vfio.c | 51
>> +
>> include/hw/pci-host/spapr.h | 1 +
>> include/hw/ppc/spapr.h | 8 ++-
>> 5 files changed, 98 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
>> index 4f76f1c..b394681 100644
>> --- a/hw/ppc/spapr_hcall.c
>> +++ b/hw/ppc/spapr_hcall.c
>> @@ -1010,6 +1010,7 @@ static void hypercall_register_types(void)
>>
>> /* ibm,client-architecture-support support */
>> spapr_register_hypercall(KVMPPC_H_CAS, h_client_architecture_support);
>> +spapr_register_hypercall(KVMPPC_H_GET_LOC_CODE, phb_get_loc_code);
>> }
>>
>> type_init(hypercall_register_types)
>> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
>> index 05f4fac..c2ee476 100644
>> --- a/hw/ppc/spapr_pci.c
>> +++ b/hw/ppc/spapr_pci.c
>> @@ -248,6 +248,44 @@ static void spapr_msi_setmsg(PCIDevice *pdev, hwaddr
>> addr, bool msix,
>> }
>> }
>>
>> +target_ulong phb_get_loc_code(PowerPCCPU *cpu, sPAPREnvironment *spapr,
>> + target_ulong opcode, target_ulong *args)
>> +{
>> +sPAPRPHBState *sphb = NULL;
>> +sPAPRPHBClass *spc = NULL;
>> +char *buf = NULL, path[PATH_MAX];
>> +PCIDevice *pdev;
>> +target_ulong buid = args[0];
>> +target_ulong config_addr = args[1];
>> +target_ulong loc_code = args[2];
>> +target_ulong size = args[3];
>> +
>> +sphb = find_phb(spapr, buid);
>> +pdev = find_dev(spapr, buid, config_addr);
>> +
>> +if (!sphb || !pdev) {
>> +return H_PARAMETER;
>> +}
>> +
>> +spc = SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb);
>> +if (spc->get_loc_code && spc->get_loc_code(sphb, pdev, &buf)) {
>> +cpu_physical_memory_write(loc_code, buf, strlen(buf));
>> +g_free(buf);
>> +return H_SUCCESS;
>> +}
>> +
>> +/*
>> + * For non-vfio devices and failures make up the location code out
>> + * of the name, slot and function.
>> + *
>> + * qemu_:.
>> + */
>> +snprintf(path, sizeof(path), "qemu_%s:%02d.%1d", pdev->name,
>> + PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn));
>> +cpu_physical_memory_write(loc_code, path, size);
>
>
> Move the chunk above to a sPAPRPHBState::get_loc_code. And I'd add PHB's
> @index in the device name to make it unique across the guest.
Sure.
>
>
>> +return H_SUCCESS;
>> +}
>> +
>> static void rtas_ibm_change_msi(PowerPCCPU *cpu, sPAPREnvironment *spapr,
>> uint32_t token, uint32_t nargs,
>> target_ulong args, uint32_t nret,
>> diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c
>> index 99a1be5..bfdfa67 100644
>> --- a/hw/ppc/spapr_pci_vfio.c
>> +++ b/hw/ppc/spapr_pci_vfio.c
>> @@ -171,6 +171,56 @@ static int spapr_phb_vfio_eeh_reset(sPAPRPHBState
>> *sphb, int option)
>> return RTAS_OUT_SUCCESS;
>> }
>>
>> +static bool spapr_phb_vfio_get_devspec(PCIDevice *pdev, char **value)
>
> s/value/devspec/ ?
Yes, should be value now.
>
>> +{
>> +char *host;
>> +char path[PATH_MAX];
>> +struct stat st;
>> +
>> +host = object_property_get_str(OBJECT(pdev), "host", NULL);
>> +if (!host) {
>> +return false;
>> +}
>> +
>> +snprintf(path, sizeof(path), "/sys/bus/pci/devices/%s/devspec", host);
>> +g_free(host);
>> +if (stat(path, &st) < 0) {
>> +return false;
>> +}
>> +
>> +return g_file_get_contents(path, value, NULL, NULL);
>
>
> g_file_get_contents() is expected to return FALSE if the file is missing so
> stat() seems redundant here.
Did not notive, will change it.
>
>
>> +}
>> +
>> +static int spapr_phb_vfio_get_loc_code(sPAPRPHBState *sphb, PCIDevice
>> *pdev,
>> +char **loc_code)
>> +{
>> +sPAPRPHBVFIOState *svphb = SPAPR_PCI_VFIO_HOST_BRIDGE(sphb);
>> +char path[PATH_MAX], *buf = NULL;
>> +struct stat st;
>> +
>> +/* Non VFIO devices */
>> +if (!svphb) {
>> +return false;
>
>
> The function returns int, not bo
Re: [Qemu-devel] [PATCH v2] spapr: populate ibm,loc-code
David Gibson writes:
> On Mon, Mar 30, 2015 at 01:18:01PM +1100, Alexey Kardashevskiy wrote:
>> On 03/27/2015 08:49 PM, Nikunj A Dadhania wrote:
>> >Each hardware instance has a platform unique location code. The OF
>> >device tree that describes a part of a hardware entity must include
>> >the “ibm,loc-code” property with a value that represents the location
>> >code for that hardware entity.
>> >
>> >Introduce an hcall to populate ibm,loc-code.
>> >1) PCI passthru devices need to identify with its own ibm,loc-code
>> >available on the host.
>> >2) Emulated devices encode as following: qemu_:.
>> >
>> >Signed-off-by: Nikunj A Dadhania
> [snip]
>> >diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
>> >index af71e8b..95157ac 100644
>> >--- a/include/hw/ppc/spapr.h
>> >+++ b/include/hw/ppc/spapr.h
>> >@@ -310,7 +310,10 @@ typedef struct sPAPREnvironment {
>> > #define KVMPPC_H_LOGICAL_MEMOP (KVMPPC_HCALL_BASE + 0x1)
>> > /* Client Architecture support */
>> > #define KVMPPC_H_CAS(KVMPPC_HCALL_BASE + 0x2)
>> >-#define KVMPPC_HCALL_MAXKVMPPC_H_CAS
>> >+#define KVMPPC_H_RTAS_UPDATE(KVMPPC_HCALL_BASE + 0x3)
>> >+#define KVMPPC_H_REPORT_MC_ERR (KVMPPC_HCALL_BASE + 0x4)
>> >+#define KVMPPC_H_GET_LOC_CODE (KVMPPC_HCALL_BASE + 0x5)
>> >+#define KVMPPC_HCALL_MAXKVMPPC_H_GET_LOC_CODE
>>
>>
>> Please add only relevant codes. And what happened to patches adding
>> H_RTAS_UPDATE and H_REPORT_MC_ERR?
>>
>> Also (it is probably a very stupid question but still :) ), why are all
>> these callbacks - hypercalls, not RTAS calls? The hypercalls are numbered in
>> sPAPR and we kind of stealing numbers from that space while we are
>> allocating RTAS tokens ourselves and have more freedom.
>
> Also, I thought the plan was to remove PCI device enumeration from
> SLOF and move it to qemu (since we need to partially do that for
> hotplug).
For me it was a short term plan. IMHO, this cant be done in short time.
> That removes the need for the hcall entirely.
>
> --
> 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
Re: [Qemu-devel] [PATCH] tcg: optimise memory layout of TCGTemp
Am 29.03.2015 um 23:52 schrieb Richard Henderson: On Mar 27, 2015 14:09, "Emilio G. Cota" wrote: On Fri, Mar 27, 2015 at 09:55:03 +, Alex Bennée wrote: Have you been able to measure any performance improvement with these new structures? In theory, if aligned with cache lines, performance should improve but real numbers would be nice. I haven't benchmarked anything, which makes me very uneasy. All I've checked is that the system boots, and FWIW I appreciate no difference in boot time. No decrease in boot time is good. We /know/ we're saving memory, after all. Is there a benchmark suite to test TCG changes? No, sorry. r~ Benchmarking TCG with QEMU's system emulation is nearly impossible because operating systems usually contain lots of timer based operations. The TCG interpreter for example is really slow, but a BIOS will boot faster than expected with it. The user mode emulation is much better for benchmarks. Run some command line Linux application which mainly does computations (not file i/o) using user mode emulation on Linux. The OpenSSL package contains bntest which can be used as a benchmark for TCG. Redirect all output to /dev/null when you run it. Binaries for i386 and x86_64 are available from http://qemu.weilnetz.de/test/user/. Stefan
Re: [Qemu-devel] [Qemu-ppc] [PATCH v2] spapr: populate ibm, loc-code
Nikunj A Dadhania writes:
> David Gibson writes:
>
>> On Mon, Mar 30, 2015 at 01:18:01PM +1100, Alexey Kardashevskiy wrote:
>>> On 03/27/2015 08:49 PM, Nikunj A Dadhania wrote:
>>> >Each hardware instance has a platform unique location code. The OF
>>> >device tree that describes a part of a hardware entity must include
>>> >the “ibm,loc-code” property with a value that represents the location
>>> >code for that hardware entity.
>>> >
>>> >Introduce an hcall to populate ibm,loc-code.
>>> >1) PCI passthru devices need to identify with its own ibm,loc-code
>>> >available on the host.
>>> >2) Emulated devices encode as following: qemu_:.
>>> >
>>> >Signed-off-by: Nikunj A Dadhania
>> [snip]
>>> >diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
>>> >index af71e8b..95157ac 100644
>>> >--- a/include/hw/ppc/spapr.h
>>> >+++ b/include/hw/ppc/spapr.h
>>> >@@ -310,7 +310,10 @@ typedef struct sPAPREnvironment {
>>> > #define KVMPPC_H_LOGICAL_MEMOP (KVMPPC_HCALL_BASE + 0x1)
>>> > /* Client Architecture support */
>>> > #define KVMPPC_H_CAS(KVMPPC_HCALL_BASE + 0x2)
>>> >-#define KVMPPC_HCALL_MAXKVMPPC_H_CAS
>>> >+#define KVMPPC_H_RTAS_UPDATE(KVMPPC_HCALL_BASE + 0x3)
>>> >+#define KVMPPC_H_REPORT_MC_ERR (KVMPPC_HCALL_BASE + 0x4)
>>> >+#define KVMPPC_H_GET_LOC_CODE (KVMPPC_HCALL_BASE + 0x5)
>>> >+#define KVMPPC_HCALL_MAXKVMPPC_H_GET_LOC_CODE
>>>
>>>
>>> Please add only relevant codes. And what happened to patches adding
>>> H_RTAS_UPDATE and H_REPORT_MC_ERR?
>>>
>>> Also (it is probably a very stupid question but still :) ), why are all
>>> these callbacks - hypercalls, not RTAS calls? The hypercalls are numbered in
>>> sPAPR and we kind of stealing numbers from that space while we are
>>> allocating RTAS tokens ourselves and have more freedom.
>>
>> Also, I thought the plan was to remove PCI device enumeration from
>> SLOF and move it to qemu (since we need to partially do that for
>> hotplug).
>
> For me it was a short term plan.
Sorry, I meant PCI device enumeration removal from SLOF was a long term
plan.
> IMHO, this cant be done in short time.
>
>> That removes the need for the hcall entirely.
>>
>> --
>> 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
Re: [Qemu-devel] [PATCH] tcg: optimise memory layout of TCGTemp
Am 29.03.2015 um 23:52 schrieb Richard Henderson: No decrease in boot time is good. We /know/ we're saving memory, after all. Well, I would not mind a decrease in boot time, too. The more it decreases, the better. :-) To be honest: in my version I only used 1 bit bitfield entries for boolean values, but 8 bit values (aligned on byte boundaries) for other values because as far as I know, most (all?) cpu architectures will need more time to extract some bits from a machine word than to extract a byte. I have no idea whether this makes a difference in performance as I did not run any runtime benchmark. Stefan
Re: [Qemu-devel] [RFC 00/10] MultiThread TCG.
to add some detail, Unfortunately Fred is away this week, so we won’t get this patch set to you ask quickly as I’d have liked. We have a ‘working’ implementation - where ‘working’ is limited to a couple of SMP cores, booting and running Dhrystone. The performance improvement we get is close to the 2x that you might expect (especially for such a simple test as Dhrystone). HOWEVER… We still have the ‘hacked’ version of tb_first (if you remember, we have made that into an array, addressed by current_cpu). This has two effects (at least): 1/ it’s probably un-necissary, but we DO need to protect cpu’s from each other when they invalidate tb_first (probably not when they write, as the current implementation only allows 1 thread to generate TB’s) 2/ it breaks GDB as current_cpu isn’t set in the IO thread, so that just set-faults and dies… We are in the process of working out how to fix this mess cleanly. So - Fred is unwilling to send the patch set as it stands, because frankly this part is totally broken. There is an independent patch set that needs splitting out which deals with just the atomic instruction issue - specifically for ARM (though I guess it’s applicable across the board)… So - in short - I HOPE to get the patch set onto the reflector sometime next week, and I’m sorry for the delay. When we do send it - it will be sent RFC. It is possible that we all agree that the atomic instruction fix is applicable, but the rest is almost certainly not. It’s there to show progress and the direction we’re going and give everybody a basis from which to discuss the issues…. not more. Cheers Mark. > On 27 Mar 2015, at 11:37, Frederic Konrad wrote: > > Hi, > > Yes a v2 will come soon. > Actually I'm trying to unbreak GDB stub and a strange segfault with smp > 2. > > Fred > > On 27/03/2015 11:08, Alex Bennée wrote: >> fred.kon...@greensocs.com writes: >> >>> From: KONRAD Frederic >>> >>> Hi everybody, >>> >>> This is the start of our work on TCG multithread. >> It's been a while since the first RFC are we likely to see a v2 of the >> patch series any time soon? >> >>> We send it for comment to be sure we are taking the right direction. >>> We already discussed the first patch but we keep it for simplicity. >>> >>> We choice to keep a common tbs array for all VCPU but protect it with the >>> tb_lock from TBContext. Then for each PageDesc we have a tb list per VCPU. >>> >>> Known issues: >>> * Some random deadlock. >>> * qemu_pause_cond is broken we can't quit QEMU. >>> * tb_flush is broken we must make sure no VCPU are executing code. >>> >>> Jan Kiszka (1): >>> Drop global lock during TCG code execution >>> >>> KONRAD Frederic (9): >>> target-arm: protect cpu_exclusive_*. >>> use a different translation block list for each cpu. >>> replace spinlock by QemuMutex. >>> remove unused spinlock. >>> extract TBContext from TCGContext. >>> protect TBContext with tb_lock. >>> tcg: remove tcg_halt_cond global variable. >>> cpu: remove exit_request global. >>> tcg: switch on multithread. >>> >>> cpu-exec.c| 47 +++ >>> cpus.c| 122 +++ >>> cputlb.c | 5 ++ >>> exec.c| 25 ++ >>> include/exec/exec-all.h | 4 +- >>> include/exec/spinlock.h | 49 --- >>> include/qom/cpu.h | 1 + >>> linux-user/main.c | 6 +- >>> scripts/checkpatch.pl | 9 +- >>> softmmu_template.h| 6 ++ >>> target-arm/cpu.c | 14 >>> target-arm/cpu.h | 3 + >>> target-arm/helper.h | 3 + >>> target-arm/op_helper.c| 10 +++ >>> target-arm/translate.c| 6 ++ >>> target-i386/mem_helper.c | 16 +++- >>> target-i386/misc_helper.c | 27 +- >>> tcg/i386/tcg-target.c | 8 ++ >>> tcg/tcg.h | 3 +- >>> translate-all.c | 208 >>> +++--- >>> vl.c | 6 ++ >>> 21 files changed, 335 insertions(+), 243 deletions(-) >>> delete mode 100644 include/exec/spinlock.h > +44 (0)20 7100 3485 x 210 +33 (0)5 33 52 01 77x 210 +33 (0)603762104 mark.burton
