Markus Armbruster [Thu, 23 Aug 2018 16:40:24 +0000 (18:40 +0200)]
json: Support %% in JSON strings when interpolating
The previous commit makes JSON strings containing '%' awkward to
express in templates: you'd have to mask the '%' with an Unicode
escape \u0025. No template currently contains such JSON strings.
Support the printf conversion specification %% in JSON strings as a
convenience anyway, because it's trivially easy to do.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20180823164025.12553-58-armbru@redhat.com>
Markus Armbruster [Thu, 23 Aug 2018 16:40:23 +0000 (18:40 +0200)]
json: Improve safety of qobject_from_jsonf_nofail() & friends
The JSON parser optionally supports interpolation. This is used to
build QObjects by parsing string templates. The templates are C
literals, so parse errors (such as invalid interpolation
specifications) are actually programming errors. Consequently, the
functions providing parsing with interpolation
(qobject_from_jsonf_nofail(), qobject_from_vjsonf_nofail(),
qdict_from_jsonf_nofail(), qdict_from_vjsonf_nofail()) pass
&error_abort to the parser.
However, there's another, more dangerous kind of programming error:
since we use va_arg() to get the value to interpolate, behavior is
undefined when the variable argument isn't consistent with the
interpolation specification.
The same problem exists with printf()-like functions, and the solution
is to have the compiler check consistency. This is what
GCC_FMT_ATTR() is about.
To enable this type checking for interpolation as well, we carefully
chose our interpolation specifications to match printf conversion
specifications, and decorate functions parsing templates with
GCC_FMT_ATTR().
Note that this only protects against undefined behavior due to type
errors. It can't protect against use of invalid interpolation
specifications that happen to be valid printf conversion
specifications.
However, there's still a gaping hole in the type checking: GCC
recognizes '%' as start of printf conversion specification anywhere in
the template, but the parser recognizes it only outside JSON strings.
For instance, if someone were to pass a "{ '%s': %d }" template, GCC
would require a char * and an int argument, but the parser would
va_arg() only an int argument, resulting in undefined behavior.
Avoid undefined behavior by catching the programming error at run
time: have the parser recognize and reject '%' in JSON strings.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20180823164025.12553-57-armbru@redhat.com>
Markus Armbruster [Thu, 23 Aug 2018 16:40:22 +0000 (18:40 +0200)]
json: Keep interpolation state in JSONParserContext
The recursive descent parser passes along a pointer to
JSONParserContext. It additionally passes a pointer to interpolation
state (a va_alist *) as needed to reach its consumer
parse_interpolation().
Stuffing the latter pointer into JSONParserContext saves us the
trouble of passing it along, so do that.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20180823164025.12553-56-armbru@redhat.com>
Markus Armbruster [Thu, 23 Aug 2018 16:40:21 +0000 (18:40 +0200)]
tests/drive_del-test: Fix harmless JSON interpolation bug
test_after_failed_device_add() does this:
response = qmp("{'execute': 'device_add',"
" 'arguments': {"
" 'driver': 'virtio-blk-%s',"
" 'drive': 'drive0'"
"}}", qvirtio_get_dev_type());
Wrong. An interpolation specification must be a JSON token, it
doesn't work within JSON string tokens. The code above doesn't use
the value of qvirtio_get_dev_type(), and sends arguments
{"driver": "virtio-blk-%s", "drive": "drive0"}}
The command fails because there is no driver named "virtio-blk-%".
Harmless, since the test wants the command to fail. Screwed up in
commit
2f84a92ec63.
Fix the obvious way. The command now fails because the drive is
empty, like it did before commit
2f84a92ec63.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20180823164025.12553-55-armbru@redhat.com>
Markus Armbruster [Thu, 23 Aug 2018 16:40:20 +0000 (18:40 +0200)]
json: Clean up headers
The JSON parser has three public headers, json-lexer.h, json-parser.h,
json-streamer.h. They all contain stuff that is of no interest
outside qobject/json-*.c.
Collect the public interface in include/qapi/qmp/json-parser.h, and
everything else in qobject/json-parser-int.h.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20180823164025.12553-54-armbru@redhat.com>
Markus Armbruster [Thu, 23 Aug 2018 16:40:19 +0000 (18:40 +0200)]
qobject: Drop superfluous includes of qemu-common.h
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20180823164025.12553-53-armbru@redhat.com>
Markus Armbruster [Thu, 23 Aug 2018 16:40:18 +0000 (18:40 +0200)]
json: Make JSONToken opaque outside json-parser.c
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20180823164025.12553-52-armbru@redhat.com>
Markus Armbruster [Thu, 23 Aug 2018 16:40:17 +0000 (18:40 +0200)]
json: Unbox tokens queue in JSONMessageParser
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20180823164025.12553-51-armbru@redhat.com>
Markus Armbruster [Thu, 23 Aug 2018 16:40:16 +0000 (18:40 +0200)]
json: Streamline json_message_process_token()
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20180823164025.12553-50-armbru@redhat.com>
Markus Armbruster [Thu, 23 Aug 2018 16:40:15 +0000 (18:40 +0200)]
json: Enforce token count and size limits more tightly
Token count and size limits exist to guard against excessive heap
usage. We check them only after we created the token on the heap.
That's assigning a cowboy to the barn to lasso the horse after it has
bolted. Close the barn door instead: check before we create the
token.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20180823164025.12553-49-armbru@redhat.com>
Markus Armbruster [Thu, 23 Aug 2018 16:40:14 +0000 (18:40 +0200)]
qjson: Have qobject_from_json() & friends reject empty and blank
The last case where qobject_from_json() & friends return null without
setting an error is empty or blank input. Callers:
* block.c's parse_json_protocol() reports "Could not parse the JSON
options". It's marked as a work-around, because it also covered
actual bugs, but they got fixed in the previous few commits.
* qobject_input_visitor_new_str() reports "JSON parse error". Also
marked as work-around. The recent fixes have made this unreachable,
because it currently gets called only for input starting with '{'.
* check-qjson.c's empty_input() and blank_input() demonstrate the
behavior.
* The other callers are not affected since they only pass input with
exactly one JSON value or, in the case of negative tests, one error.
Fail with "Expecting a JSON value" instead of returning null, and
simplify callers.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20180823164025.12553-48-armbru@redhat.com>
Markus Armbruster [Thu, 23 Aug 2018 16:40:13 +0000 (18:40 +0200)]
json: Assert json_parser_parse() consumes all tokens on success
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20180823164025.12553-47-armbru@redhat.com>
Markus Armbruster [Thu, 23 Aug 2018 16:40:12 +0000 (18:40 +0200)]
json: Fix streamer not to ignore trailing unterminated structures
json_message_process_token() accumulates tokens until it got the
sequence of tokens that comprise a single JSON value (it counts curly
braces and square brackets to decide). It feeds those token sequences
to json_parser_parse(). If a non-empty sequence of tokens remains at
the end of the parse, it's silently ignored. check-qjson.c cases
unterminated_array(), unterminated_array_comma(), unterminated_dict(),
unterminated_dict_comma() demonstrate this bug.
Fix as follows. Introduce a JSON_END_OF_INPUT token. When the
streamer receives it, it feeds the accumulated tokens to
json_parser_parse().
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20180823164025.12553-46-armbru@redhat.com>
Markus Armbruster [Thu, 23 Aug 2018 16:40:11 +0000 (18:40 +0200)]
json: Fix latent parser aborts at end of input
json-parser.c carefully reports end of input like this:
token = parser_context_pop_token(ctxt);
if (token == NULL) {
parse_error(ctxt, NULL, "premature EOI");
goto out;
}
Except parser_context_pop_token() can't return null, it fails its
assertion instead. Same for parser_context_peek_token(). Broken in
commit
65c0f1e9558, and faithfully preserved in commit
95385fe9ace.
Only a latent bug, because the streamer throws away any input that
could trigger it.
Drop the assertions, so we can fix the streamer in the next commit.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20180823164025.12553-45-armbru@redhat.com>
Markus Armbruster [Thu, 23 Aug 2018 16:40:10 +0000 (18:40 +0200)]
qjson: Fix qobject_from_json() & friends for multiple values
qobject_from_json() & friends use the consume_json() callback to
receive either a value or an error from the parser.
When they are fed a string that contains more than either one JSON
value or one JSON syntax error, consume_json() gets called multiple
times.
When the last call receives a value, qobject_from_json() returns that
value. Any other values are leaked.
When any call receives an error, qobject_from_json() sets the first
error received. Any other errors are thrown away.
When values follow errors, qobject_from_json() returns both a value
and sets an error. That's bad. Impact:
* block.c's parse_json_protocol() ignores and leaks the value. It's
used to to parse pseudo-filenames starting with "json:". The
pseudo-filenames can come from the user or from image meta-data such
as a QCOW2 image's backing file name.
* vl.c's parse_display_qapi() ignores and leaks the error. It's used
to parse the argument of command line option -display.
* vl.c's main() case QEMU_OPTION_blockdev ignores the error and leaves
it in @err. main() will then pass a pointer to a non-null Error *
to net_init_clients(), which is forbidden. It can lead to assertion
failure or other misbehavior.
* check-qjson.c's multiple_values() demonstrates the badness.
* The other callers are not affected since they only pass strings with
exactly one JSON value or, in the case of negative tests, one
error.
The impact on the _nofail() functions is relatively harmless. They
abort when any call receives an error. Else they return the last
value, and leak the others, if any.
Fix consume_json() as follows. On the first call, save value and
error as before. On subsequent calls, if any, don't save them. If
the first call saved a value, the next call, if any, replaces the
value by an "Expecting at most one JSON value" error. Take care not
to leak values or errors that aren't saved.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20180823164025.12553-44-armbru@redhat.com>
Markus Armbruster [Thu, 23 Aug 2018 16:40:09 +0000 (18:40 +0200)]
json: Improve names of lexer states related to numbers
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20180823164025.12553-43-armbru@redhat.com>
Markus Armbruster [Thu, 23 Aug 2018 16:40:08 +0000 (18:40 +0200)]
json: Replace %I64d, %I64u by %PRId64, %PRIu64
Support for %I64d got added in commit
2c0d4b36e7f "json: fix PRId64 on
Win32". We had to hard-code I64d because we used the lexer's finite
state machine to check interpolations. No more, so clean this up.
Additional conversion specifications would be easy enough to implement
when needed.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20180823164025.12553-42-armbru@redhat.com>
Markus Armbruster [Thu, 23 Aug 2018 16:40:07 +0000 (18:40 +0200)]
json: Leave rejecting invalid interpolation to parser
Both lexer and parser reject invalid interpolation specifications.
The parser's check is useless.
The lexer ends the token right after the first bad character. This
tends to lead to suboptimal error reporting. For instance, input
[ %04d ]
produces the tokens
JSON_LSQUARE [
JSON_ERROR %0
JSON_INTEGER 4
JSON_KEYWORD d
JSON_RSQUARE ]
The parser then yields an error, an object and two more errors:
error: Invalid JSON syntax
object: 4
error: JSON parse error, invalid keyword
error: JSON parse error, expecting value
Dumb down the lexer to accept [A-Za-z0-9]*. The parser's check is now
used. Emit a proper error there.
The lexer now produces
JSON_LSQUARE [
JSON_INTERP %04d
JSON_RSQUARE ]
and the parser reports just
JSON parse error, invalid interpolation '%04d'
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20180823164025.12553-41-armbru@redhat.com>
Markus Armbruster [Thu, 23 Aug 2018 16:40:06 +0000 (18:40 +0200)]
json: Pass lexical errors and limit violations to callback
The callback to consume JSON values takes QObject *json, Error *err.
If both are null, the callback is supposed to make up an error by
itself. This sucks.
qjson.c's consume_json() neglects to do so, which makes
qobject_from_json() null instead of failing. I consider that a bug.
The culprit is json_message_process_token(): it passes two null
pointers when it runs into a lexical error or a limit violation. Fix
it to pass a proper Error object then. Update the callbacks:
* monitor.c's handle_qmp_command(): the code to make up an error is
now dead, drop it.
* qga/main.c's process_event(): lumps the "both null" case together
with the "not a JSON object" case. The former is now gone. The
error message "Invalid JSON syntax" is misleading for the latter.
Improve it to "Input must be a JSON object".
* qobject/qjson.c's consume_json(): no update; check-qjson
demonstrates qobject_from_json() now sets an error on lexical
errors, but still doesn't on some other errors.
* tests/libqtest.c's qmp_response(): the Error object is now reliable,
so use it to improve the error message.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20180823164025.12553-40-armbru@redhat.com>
Markus Armbruster [Thu, 23 Aug 2018 16:40:05 +0000 (18:40 +0200)]
json: Treat unwanted interpolation as lexical error
The JSON parser optionally supports interpolation. The lexer
recognizes interpolation tokens unconditionally. The parser rejects
them when interpolation is disabled, in parse_interpolation().
However, it neglects to set an error then, which can make
json_parser_parse() fail without setting an error.
Move the check for unwanted interpolation from the parser's
parse_interpolation() into the lexer's finite state machine. When
interpolation is disabled, '%' is now handled like any other
unexpected character.
The next commit will improve how such lexical errors are handled.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20180823164025.12553-39-armbru@redhat.com>
Markus Armbruster [Thu, 23 Aug 2018 16:40:04 +0000 (18:40 +0200)]
json: Rename token JSON_ESCAPE & friends to JSON_INTERP
The JSON parser optionally supports interpolation. The code calls it
"escape". Awkward, because it uses the same term for escape sequences
within strings. The latter usage is consistent with RFC 8259 "The
JavaScript Object Notation (JSON) Data Interchange Format" and ISO C.
Call the former "interpolation" instead.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20180823164025.12553-38-armbru@redhat.com>
Markus Armbruster [Thu, 23 Aug 2018 16:40:03 +0000 (18:40 +0200)]
json: Don't create JSON_ERROR tokens that won't be used
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20180823164025.12553-37-armbru@redhat.com>
Markus Armbruster [Thu, 23 Aug 2018 16:40:02 +0000 (18:40 +0200)]
json: Don't pass null @tokens to json_parser_parse()
json_parser_parse() normally returns the QObject on success. Except
it returns null when its @tokens argument is null.
Its only caller json_message_process_token() passes null @tokens when
emitting a lexical error. The call is a rather opaque way to say json
= NULL then.
Simplify matters by lifting the assignment to json out of the emit
path: initialize json to null, set it to the value of
json_parser_parse() when there's no lexical error. Drop the special
case from json_parser_parse().
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20180823164025.12553-36-armbru@redhat.com>
Markus Armbruster [Thu, 23 Aug 2018 16:40:01 +0000 (18:40 +0200)]
json: Redesign the callback to consume JSON values
The classical way to structure parser and lexer is to have the client
call the parser to get an abstract syntax tree, the parser call the
lexer to get the next token, and the lexer call some function to get
input characters.
Another way to structure them would be to have the client feed
characters to the lexer, the lexer feed tokens to the parser, and the
parser feed abstract syntax trees to some callback provided by the
client. This way is more easily integrated into an event loop that
dispatches input characters as they arrive.
Our JSON parser is kind of between the two. The lexer feeds tokens to
a "streamer" instead of a real parser. The streamer accumulates
tokens until it got the sequence of tokens that comprise a single JSON
value (it counts curly braces and square brackets to decide). It
feeds those token sequences to a callback provided by the client. The
callback passes each token sequence to the parser, and gets back an
abstract syntax tree.
I figure it was done that way to make a straightforward recursive
descent parser possible. "Get next token" becomes "pop the first
token off the token sequence". Drawback: we need to store a complete
token sequence. Each token eats 13 + input characters + malloc
overhead bytes.
Observations:
1. This is not the only way to use recursive descent. If we replaced
"get next token" by a coroutine yield, we could do without a
streamer.
2. The lexer reports errors by passing a JSON_ERROR token to the
streamer. This communicates the offending input characters and
their location, but no more.
3. The streamer reports errors by passing a null token sequence to the
callback. The (already poor) lexical error information is thrown
away.
4. Having the callback receive a token sequence duplicates the code to
convert token sequence to abstract syntax tree in every callback.
5. Known bug: the streamer silently drops incomplete token sequences.
This commit rectifies 4. by lifting the call of the parser from the
callbacks into the streamer. Later commits will address 3. and 5.
The lifting removes a bug from qjson.c's parse_json(): it passed a
pointer to a non-null Error * in certain cases, as demonstrated by
check-qjson.c.
json_parser_parse() is now unused. It's a stupid wrapper around
json_parser_parse_err(). Drop it, and rename json_parser_parse_err()
to json_parser_parse().
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20180823164025.12553-35-armbru@redhat.com>
Markus Armbruster [Thu, 23 Aug 2018 16:40:00 +0000 (18:40 +0200)]
json: Have lexer call streamer directly
json_lexer_init() takes the function to process a token as an
argument. It's always json_message_process_token(). Makes the code
harder to understand for no actual gain. Drop the indirection.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20180823164025.12553-34-armbru@redhat.com>
Marc-André Lureau [Thu, 23 Aug 2018 16:39:59 +0000 (18:39 +0200)]
json-parser: simplify and avoid JSONParserContext allocation
parser_context_new/free() are only used from json_parser_parse(). We
can fold the code there and avoid an allocation altogether.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <
20180719184111.5129-9-marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <
20180823164025.12553-33-armbru@redhat.com>
Marc-André Lureau [Thu, 23 Aug 2018 16:39:58 +0000 (18:39 +0200)]
json: remove useless return value from lexer/parser
The lexer always returns 0 when char feeding. Furthermore, none of the
caller care about the return value.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <
20180326150916.9602-10-marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <
20180823164025.12553-32-armbru@redhat.com>
Markus Armbruster [Thu, 23 Aug 2018 16:39:57 +0000 (18:39 +0200)]
check-qjson: Fix and enable utf8_string()'s disabled part
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20180823164025.12553-31-armbru@redhat.com>
Markus Armbruster [Thu, 23 Aug 2018 16:39:56 +0000 (18:39 +0200)]
json: Fix \uXXXX for surrogate pairs
The JSON parser treats each half of a surrogate pair as unpaired
surrogate. Fix it to recognize surrogate pairs.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20180823164025.12553-30-armbru@redhat.com>
Markus Armbruster [Thu, 23 Aug 2018 16:39:55 +0000 (18:39 +0200)]
json: Reject invalid \uXXXX, fix \u0000
The JSON parser translates invalid \uXXXX to garbage instead of
rejecting it, and swallows \u0000.
Fix by using mod_utf8_encode() instead of flawed wchar_to_utf8().
Valid surrogate pairs are now differently broken: they're rejected
instead of translated to garbage. The next commit will fix them.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20180823164025.12553-29-armbru@redhat.com>
Markus Armbruster [Thu, 23 Aug 2018 16:39:54 +0000 (18:39 +0200)]
json: Simplify parse_string()
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20180823164025.12553-28-armbru@redhat.com>
Markus Armbruster [Thu, 23 Aug 2018 16:39:53 +0000 (18:39 +0200)]
json: Leave rejecting invalid escape sequences to parser
Both lexer and parser reject invalid escape sequences in strings. The
parser's check is useless.
The lexer ends the token right after the first non-well-formed byte.
This tends to lead to suboptimal error reporting. For instance, input
{"abc\@ijk": 1}
produces the tokens
JSON_LCURLY {
JSON_ERROR "abc\@
JSON_KEYWORD ijk
JSON_ERROR ": 1}\n
The parser then reports three errors
Invalid JSON syntax
JSON parse error, invalid keyword 'ijk'
Invalid JSON syntax
before it recovers at the newline.
Drop the lexer's escape sequence checking, and make it accept the same
characters after backslash it accepts elsewhere in strings. It now
produces
JSON_LCURLY {
JSON_STRING "abc\@ijk"
JSON_COLON :
JSON_INTEGER 1
JSON_RCURLY
and the parser reports just
JSON parse error, invalid escape sequence in string
While there, fix parse_string()'s inaccurate function comment.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20180823164025.12553-27-armbru@redhat.com>
Markus Armbruster [Thu, 23 Aug 2018 16:39:52 +0000 (18:39 +0200)]
json: Accept overlong \xC0\x80 as U+0000 ("modified UTF-8")
Since the JSON grammer doesn't accept U+0000 anywhere, this merely
exchanges one kind of parse error for another. It's purely for
consistency with qobject_to_json(), which accepts \xC0\x80 (see commit
e2ec3f97680).
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20180823164025.12553-26-armbru@redhat.com>
Markus Armbruster [Thu, 23 Aug 2018 16:39:51 +0000 (18:39 +0200)]
json: Leave rejecting invalid UTF-8 to parser
Both the lexer and the parser (attempt to) validate UTF-8 in JSON
strings.
The lexer rejects bytes that can't occur in valid UTF-8: \xC0..\xC1,
\xF5..\xFF. This rejects some, but not all invalid UTF-8. It also
rejects ASCII control characters \x00..\x1F, in accordance with RFC
8259 (see recent commit "json: Reject unescaped control characters").
When the lexer rejects, it ends the token right after the first bad
byte. Good when the bad byte is a newline. Not so good when it's
something like an overlong sequence in the middle of a string. For
instance, input
{"abc\xC0\xAFijk": 1}\n
produces the tokens
JSON_LCURLY {
JSON_ERROR "abc\xC0
JSON_ERROR \xAF
JSON_KEYWORD ijk
JSON_ERROR ": 1}\n
The parser then reports four errors
Invalid JSON syntax
Invalid JSON syntax
JSON parse error, invalid keyword 'ijk'
Invalid JSON syntax
before it recovers at the newline.
The commit before previous made the parser reject invalid UTF-8
sequences. Since then, anything the lexer rejects, the parser would
reject as well. Thus, the lexer's rejecting is unnecessary for
correctness, and harmful for error reporting.
However, we want to keep rejecting ASCII control characters in the
lexer, because that produces the behavior we want for unclosed
strings.
We also need to keep rejecting \xFF in the lexer, because we
documented that as a way to reset the JSON parser
(docs/interop/qmp-spec.txt section 2.6 QGA Synchronization), which
means we can't change how we recover from this error now. I wish we
hadn't done that.
I think we should treat \xFE the same as \xFF.
Change the lexer to accept \xC0..\xC1 and \xF5..\xFD. It now rejects
only \x00..\x1F and \xFE..\xFF. Error reporting for invalid UTF-8 in
strings is much improved, except for \xFE and \xFF. For the example
above, the lexer now produces
JSON_LCURLY {
JSON_STRING "abc\xC0\xAFijk"
JSON_COLON :
JSON_INTEGER 1
JSON_RCURLY
and the parser reports just
JSON parse error, invalid UTF-8 sequence in string
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20180823164025.12553-25-armbru@redhat.com>
Markus Armbruster [Thu, 23 Aug 2018 16:39:50 +0000 (18:39 +0200)]
json: Report first rather than last parse error
Quiz time! When a parser reports multiple errors, but the user gets
to see just one, which one is (on average) the least useful one?
Yes, you're right, it's the last one! You're clearly familiar with
compilers.
Which one does QEMU report?
Right again, the last one! You're clearly familiar with QEMU.
Reproducer: feeding
{"abc\xC2ijk": 1}\n
to QMP produces
{"error": {"class": "GenericError", "desc": "JSON parse error, key is not a string in object"}}
Report the first error instead. The reproducer now produces
{"error": {"class": "GenericError", "desc": "JSON parse error, invalid UTF-8 sequence in string"}}
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20180823164025.12553-24-armbru@redhat.com>
Markus Armbruster [Thu, 23 Aug 2018 16:39:49 +0000 (18:39 +0200)]
json: Reject invalid UTF-8 sequences
We reject bytes that can't occur in valid UTF-8 (\xC0..\xC1,
\xF5..\xFF in the lexer. That's insufficient; there's plenty of
invalid UTF-8 not containing these bytes, as demonstrated by
check-qjson:
* Malformed sequences
- Unexpected continuation bytes
- Missing continuation bytes after start bytes other than
\xC0..\xC1, \xF5..\xFD.
* Overlong sequences with start bytes other than \xC0..\xC1,
\xF5..\xFD.
* Invalid code points
Fixing this in the lexer would be bothersome. Fixing it in the parser
is straightforward, so do that.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20180823164025.12553-23-armbru@redhat.com>
Markus Armbruster [Thu, 23 Aug 2018 16:39:48 +0000 (18:39 +0200)]
check-qjson: Document we expect invalid UTF-8 to be rejected
The JSON parser rejects some invalid sequences, but accepts others
without correcting the problem.
We should either reject all invalid sequences, or minimize overlong
sequences and replace all other invalid sequences by a suitable
replacement character. A common choice for replacement is U+FFFD.
I'm going to implement the former. Update the comments in
utf8_string() to expect this.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20180823164025.12553-22-armbru@redhat.com>
Markus Armbruster [Thu, 23 Aug 2018 16:39:47 +0000 (18:39 +0200)]
json: Tighten and simplify qstring_from_escaped_str()'s loop
Simplify loop control, and assert that the string ends with the
appropriate quote (the lexer ensures it does).
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20180823164025.12553-21-armbru@redhat.com>
Markus Armbruster [Thu, 23 Aug 2018 16:39:46 +0000 (18:39 +0200)]
json: Revamp lexer documentation
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20180823164025.12553-20-armbru@redhat.com>
Markus Armbruster [Thu, 23 Aug 2018 16:39:45 +0000 (18:39 +0200)]
json: Reject unescaped control characters
Fix the lexer to reject unescaped control characters in JSON strings,
in accordance with RFC 8259 "The JavaScript Object Notation (JSON)
Data Interchange Format".
Bonus: we now recover more nicely from unclosed strings. E.g.
{"one: 1}\n{"two": 2}
now recovers cleanly after the newline, where before the lexer
remained confused until the next unpaired double quote or lexical
error.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20180823164025.12553-19-armbru@redhat.com>
Markus Armbruster [Thu, 23 Aug 2018 16:39:44 +0000 (18:39 +0200)]
json: Fix lexer to include the bad character in JSON_ERROR token
json_lexer[] maps (lexer state, input character) to the new lexer
state. The input character is consumed unless the new state is
terminal and the input character doesn't belong to this token,
i.e. the state transition uses look-ahead. When this is the case,
input character '\0' would result in the same state transition.
TERMINAL_NEEDED_LOOKAHEAD() exploits this.
Except this is wrong for transitions to IN_ERROR. There, the
offending input character is in fact consumed: case IN_ERROR returns.
It isn't added to the JSON_ERROR token, though.
Fix that by making TERMINAL_NEEDED_LOOKAHEAD() return false for
transitions to IN_ERROR.
There's a slight complication. json_lexer_flush() passes input
character '\0' to flush an incomplete token. If this results in
JSON_ERROR, we'd now add the '\0' to the token. Suppress that.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20180823164025.12553-18-armbru@redhat.com>
Markus Armbruster [Thu, 23 Aug 2018 16:39:43 +0000 (18:39 +0200)]
check-qjson: Cover interpolation more thoroughly
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20180823164025.12553-17-armbru@redhat.com>
Markus Armbruster [Thu, 23 Aug 2018 16:39:42 +0000 (18:39 +0200)]
check-qjson qmp-test: Cover control characters more thoroughly
RFC 8259 "The JavaScript Object Notation (JSON) Data Interchange
Format" requires control characters in strings to be escaped.
Demonstrate the JSON parser accepts U+0001 .. U+001F unescaped.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20180823164025.12553-16-armbru@redhat.com>
Markus Armbruster [Thu, 23 Aug 2018 16:39:41 +0000 (18:39 +0200)]
check-qjson: Fix utf8_string() to test all invalid sequences
Some of utf8_string()'s test_cases[] contain multiple invalid
sequences. Testing that qobject_from_json() fails only tests we
reject at least one invalid sequence. That's incomplete.
Additionally test each non-space sequence in isolation.
This demonstrates that the JSON parser accepts invalid sequences
starting with \xC2..\xF4. Add a FIXME comment.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20180823164025.12553-15-armbru@redhat.com>
Markus Armbruster [Thu, 23 Aug 2018 16:39:40 +0000 (18:39 +0200)]
check-qjson: Simplify utf8_string()
The previous commit made utf8_string()'s test_cases[].utf8_in
superfluous: we can use .json_in instead. Except for the case testing
U+0000. \x00 doesn't work in C strings, so it tests \\u0000 instead.
But testing \\uXXXX is escaped_string()'s job. It's covered there.
Test U+0001 here, and drop .utf8_in.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20180823164025.12553-14-armbru@redhat.com>
Markus Armbruster [Thu, 23 Aug 2018 16:39:39 +0000 (18:39 +0200)]
check-qjson: Cover UTF-8 in single quoted strings
utf8_string() tests only double quoted strings. Cover single quoted
strings, too: store the strings to test without quotes, then wrap them
in either kind of quote.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20180823164025.12553-13-armbru@redhat.com>
Markus Armbruster [Thu, 23 Aug 2018 16:39:38 +0000 (18:39 +0200)]
check-qjson: Consolidate partly redundant string tests
simple_string() and single_quote_string() have become redundant with
escaped_string(), except for embedded single and double quotes.
Replace them by a test that covers just that.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20180823164025.12553-12-armbru@redhat.com>
Markus Armbruster [Thu, 23 Aug 2018 16:39:37 +0000 (18:39 +0200)]
check-qjson: Cover escaped characters more thoroughly, part 2
Cover escaped single quote, surrogates, invalid escapes, and
noncharacters. This demonstrates that valid surrogate pairs are
misinterpreted, and invalid surrogates and noncharacters aren't
rejected.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20180823164025.12553-11-armbru@redhat.com>
Markus Armbruster [Thu, 23 Aug 2018 16:39:36 +0000 (18:39 +0200)]
check-qjson: Streamline escaped_string()'s test strings
Merge a few closely related test strings, and drop a few redundant
ones.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20180823164025.12553-10-armbru@redhat.com>
Markus Armbruster [Thu, 23 Aug 2018 16:39:35 +0000 (18:39 +0200)]
check-qjson: Cover escaped characters more thoroughly, part 1
escaped_string() first tests double quoted strings, then repeats a few
tests with single quotes. Repeat all of them: store the strings to
test without quotes, and wrap them in either kind of quote for
testing.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20180823164025.12553-9-armbru@redhat.com>
Markus Armbruster [Thu, 23 Aug 2018 16:39:34 +0000 (18:39 +0200)]
test-qga: Clean up how we test QGA synchronization
To permit recovering from arbitrary JSON parse errors, the JSON parser
resets itself on lexical errors. We recommend sending a 0xff byte for
that purpose, and test-qga covers this usage since commit
5229564b832.
That commit had to add an ugly hack to qmp_fd_vsend() to make capable
of sending this byte (it's designed to send only valid JSON).
The previous commit added a way to send arbitrary text. Put that to
use for this purpose, and drop the hack from qmp_fd_vsend().
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20180823164025.12553-8-armbru@redhat.com>
Markus Armbruster [Thu, 23 Aug 2018 16:39:33 +0000 (18:39 +0200)]
qmp-test: Cover syntax and lexical errors
qmp-test neglects to cover QMP input that isn't valid JSON. libqtest
doesn't let us send such input. Add qtest_qmp_send_raw() for this
purpose, and put it to use in qmp-test.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20180823164025.12553-7-armbru@redhat.com>
[Commit message typo fixed]
Markus Armbruster [Thu, 23 Aug 2018 16:39:32 +0000 (18:39 +0200)]
qmp-cmd-test: Split off qmp-test
qmp-test is for QMP protocol tests. Commit
e4a426e75ef added generic,
basic tests of query commands to it. Move them to their own test
program qmp-cmd-test, to keep qmp-test focused on the protocol.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20180823164025.12553-6-armbru@redhat.com>
Markus Armbruster [Thu, 23 Aug 2018 16:39:31 +0000 (18:39 +0200)]
check-qjson: Cover whitespace more thoroughly
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20180823164025.12553-5-armbru@redhat.com>
Markus Armbruster [Thu, 23 Aug 2018 16:39:30 +0000 (18:39 +0200)]
check-qjson: Cover blank and lexically erroneous input
qobject_from_json() can return null without setting an error on
lexical errors. I call that a bug. Add test coverage to demonstrate
it.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20180823164025.12553-4-armbru@redhat.com>
Markus Armbruster [Thu, 23 Aug 2018 16:39:29 +0000 (18:39 +0200)]
check-qjson: Cover multiple JSON objects in same string
qobject_from_json() & friends misbehave when the JSON text has more
than one JSON value. Add test coverage to demonstrate the bugs.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20180823164025.12553-3-armbru@redhat.com>
Markus Armbruster [Thu, 23 Aug 2018 16:39:28 +0000 (18:39 +0200)]
docs/interop/qmp-spec: How to force known good parser state
Section "QGA Synchronization" specifies that sending "a raw 0xFF
sentinel byte" makes the server "reset its state and discard all
pending data prior to the sentinel." What actually happens there is a
lexical error, which will produce one or more error responses.
Moreover, it's not specific to QGA.
Create new section "Forcing the JSON parser into known-good state" to
document the technique properly. Rewrite section "QGA
Synchronization" to document just the other direction, i.e. command
guest-sync-delimited.
Section "Protocol Specification" mentions "synchronization bytes
(documented below)". Delete that.
While there, fix it not to claim '"Server" is QEMU itself', but
'"Server" is either QEMU or the QEMU Guest Agent'.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20180823164025.12553-2-armbru@redhat.com>
Peter Maydell [Fri, 24 Aug 2018 13:46:31 +0000 (14:46 +0100)]
Merge remote-tracking branch 'remotes/juanquintela/tags/check/
20180822' into staging
check/next for
20180822
# gpg: Signature made Wed 22 Aug 2018 09:03:40 BST
# gpg: using RSA key
F487EF185872D723
# gpg: Good signature from "Juan Quintela <quintela@redhat.com>"
# gpg: aka "Juan Quintela <quintela@trasno.org>"
# Primary key fingerprint: 1899 FF8E DEBF 58CC EE03 4B82 F487 EF18 5872 D723
* remotes/juanquintela/tags/check/
20180822:
check: Only test tpm devices when they are compiled in
check: Only test usb-ehci when it is compiled in
check: Only test usb-uhci devices when they are compiled in
check: Only test usb-ohci when it is compiled in
check: Only test nvme when it is compiled in
check: Only test pvpanic when it is compiled in
check: Only test wdt_ib700 when it is compiled in
check: Only test sdhci when it is compiled in
check: Only test i82801b11 when it is compiled in
check: Only test ioh3420 when it is compiled in
check: Only test ipack when it is compiled in
check: Only test hda when it is compiled in
check: Only test ac97 when it is compiled in
check: Only test es1370 when it is compiled in
check: Only test rtl8139 when it is compiled in
check: Only test pcnet when it is compiled in
check: Only test eepro100 when it is compiled in
check: Only test ne2000 when it is compiled in
check: Only test vmxnet3 when it is compiled in
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Peter Maydell [Fri, 24 Aug 2018 12:29:07 +0000 (13:29 +0100)]
Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-
20180824-1' into staging
target-arm queue:
* Fix rounding errors in scaling float-to-int and int-to-float operations
* Connect virtualization-related IRQs and memory regions of GICv2
in boards that use Cortex-A7 or Cortex-A15
* Support taking exceptions to AArch32 Hyp mode
* Clear CPSR.IL and CPSR.J on 32-bit exception entry
(a minor bug fix that won't affect non-buggy guest code)
* mps2-an505: Implement various missing devices:
dual timer, watchdogs, counters in the FPGAIO registers,
some missing ID/control registers, TrustZone Master Security
Controllers, PL081 DMA controllers, PL022 SPI controllers
* correct ID register values for mps2-an385, -an511, -an505
* fix some hardcoded tabs in untouched backwaters of the
target/arm codebase
* raspi: Refactor framebuffer property handling code and implement
support for the virtual framebuffer/viewport
# gpg: Signature made Fri 24 Aug 2018 13:19:04 BST
# gpg: using RSA key
3C2525ED14360CDE
# gpg: Good signature from "Peter Maydell <peter.maydell@linaro.org>"
# gpg: aka "Peter Maydell <pmaydell@gmail.com>"
# gpg: aka "Peter Maydell <pmaydell@chiark.greenend.org.uk>"
# Primary key fingerprint: E1A5 C593 CD41 9DE2 8E83 15CF 3C25 25ED 1436 0CDE
* remotes/pmaydell/tags/pull-target-arm-
20180824-1: (52 commits)
hw/arm/mps2: Fix ID register errors on AN511 and AN385
hw/display/bcm2835_fb: Validate bcm2835_fb_mbox_push() config
hw/display/bcm2835_fb: Validate config settings
hw/display/bcm2835_fb: Fix handling of virtual framebuffer
hw/display/bcm2835_fb: Abstract out calculation of pitch, size
hw/display/bcm2835_fb: Reset resolution, etc correctly
hw/display/bcm2835_fb: Drop unused size and pitch fields
hw/misc/bcm2835_property: Track fb settings using BCM2835FBConfig
hw/misc/bcm2835_fb: Move config fields to their own struct
target/arm: Remove a handful of stray tabs
target/arm: Untabify iwmmxt_helper.c
target/arm: Untabify translate.c
hw/arm/mps2-tz: Fix MPS2 SCC config register values
hw/arm/mps2-tz: Instantiate SPI controllers
hw/ssi/pl022: Correct wrong DMACR and ICR handling
hw/ssi/pl022: Correct wrong value for PL022_INT_RT
hw/ssi/pl022: Use DeviceState::realize rather than SysBusDevice::init
hw/ssi/pl022: Don't directly call vmstate_register()
hw/ssi/pl022: Set up reset function in class init
hw/ssi/pl022: Allow use as embedded-struct device
...
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Peter Maydell [Fri, 24 Aug 2018 12:17:50 +0000 (13:17 +0100)]
hw/arm/mps2: Fix ID register errors on AN511 and AN385
Fix MPS2 SCC config register values for the mps2-an511
and mps2-an385 boards:
* the SCC_AID bits [23:20] specify the FPGA build target board revision,
and the SCC_CFG4 register specifies the actual board revision, so
these should have matching values. Claim to be board revision C,
consistently -- we had the revision in the wrong part of SCC_AID.
* SCC_ID bits [15:4] should be the board number in hex, not decimal
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id:
20180823175225.22612-1-peter.maydell@linaro.org
Peter Maydell [Fri, 24 Aug 2018 12:17:50 +0000 (13:17 +0100)]
hw/display/bcm2835_fb: Validate bcm2835_fb_mbox_push() config
Refactor bcm2835_fb_mbox_push() to work by calling
bcm2835_fb_validate_config() and bcm2835_fb_reconfigure(),
so that config set this way is also validated.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id:
20180814144436.679-9-peter.maydell@linaro.org
Peter Maydell [Fri, 24 Aug 2018 12:17:50 +0000 (13:17 +0100)]
hw/display/bcm2835_fb: Validate config settings
Validate the config settings that the guest tries to set.
The wiki page documentation is not really accurate here:
generally rather than failing requests to set bad parameters,
the hardware will just clip them to something sensible.
Validate the most important parameters: sizes and
the viewport offsets. This prevents the framebuffer
code from trying to read out-of-range memory.
In the property handling code, we validate the new parameters every
time we encounter a tag that sets them. This means we validate the
config multiple times if the request includes multiple config-setting
tags, but the code would require significant restructuring to do a
validation only once but still return the clipped settings for
get-parameter tags and the buffer allocation tag.
Validation of settings made via the older bcm2835_fb_mbox_push()
function will be done in the next commit.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id:
20180814144436.679-8-peter.maydell@linaro.org
Peter Maydell [Fri, 24 Aug 2018 12:17:49 +0000 (13:17 +0100)]
hw/display/bcm2835_fb: Fix handling of virtual framebuffer
The raspi framebuffir in bcm2835_fb supports the definition
of a virtual "viewport", which is smaller than the full
physical framebuffer size and at an adjustable offset within
it. Only the viewport area is sent to the screen. This allows
the guest to do things like double buffering, or scrolling
by adjusting the viewport origin. Currently QEMU doesn't
implement this at all.
Add support for this feature:
* the property mailbox code needs to distinguish the
virtual width/height from the physical width/height
* the framebuffer code needs to do something with the
virtual width/height/origin information
Note that the wiki documentation on the semantics of the
virtual and physical height and width has it the wrong way
around -- the virtual size is the size of the allocated
buffer, and the physical size is the size of the display,
so the virtual size is always the same as or larger than
the physical.
If the viewport size is set smaller than the physical
screen size, we ignore the viewport settings completely
and just display the physical screen area.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id:
20180814144436.679-7-peter.maydell@linaro.org
Peter Maydell [Fri, 24 Aug 2018 12:17:49 +0000 (13:17 +0100)]
hw/display/bcm2835_fb: Abstract out calculation of pitch, size
Abstract out the calculation of the pitch and size of the
framebuffer into functions that operate on the BCM2835FBConfig
struct -- these are about to get a little more complicated
when we add support for virtual and physical sizes differing.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id:
20180814144436.679-6-peter.maydell@linaro.org
Peter Maydell [Fri, 24 Aug 2018 12:17:49 +0000 (13:17 +0100)]
hw/display/bcm2835_fb: Reset resolution, etc correctly
The bcm2835_fb's initial resolution and other parameters are set
via QOM properties. We should reset to those initial values on
device reset, which means we need to save the QOM property
values somewhere that they are not overwritten by guest
changes to the framebuffer configuration.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id:
20180814144436.679-5-peter.maydell@linaro.org
Peter Maydell [Fri, 24 Aug 2018 12:17:49 +0000 (13:17 +0100)]
hw/display/bcm2835_fb: Drop unused size and pitch fields
The BCM2835FBState struct has a 'pitch' field which is a
cached copy of xres * (bpp >> 3), and a 'size' field which is
a cached copy of pitch * yres. However we don't actually do
anything with these fields; delete them. We retain the
now-unused slots in the VMState struct for migration
compatibility.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id:
20180814144436.679-4-peter.maydell@linaro.org
Peter Maydell [Fri, 24 Aug 2018 12:17:48 +0000 (13:17 +0100)]
hw/misc/bcm2835_property: Track fb settings using BCM2835FBConfig
Refactor the fb property setting code so that rather than
using a set of pointers to local variables to track
whether a config value has been updated in the current
mbox and if so what its new value is, we just copy
all the current settings of the fb at the start, and
then update that copy as we go along, before asking
the fb to switch to it at the end.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id:
20180814144436.679-3-peter.maydell@linaro.org
Peter Maydell [Fri, 24 Aug 2018 12:17:48 +0000 (13:17 +0100)]
hw/misc/bcm2835_fb: Move config fields to their own struct
The handling of framebuffer properties in the bcm2835_property code
is a bit clumsy, because for each of the many fb related properties
we try to track the value we're about to set and whether we're going
to be setting a value, and then we hand all the new values off
to the framebuffer via a function which takes them all as separate
arguments. It would be simpler if the property code could easily
copy all the framebuffer's current settings, update them with
the new specified values and then ask the framebuffer to switch
to the new set.
As the first part of this refactoring, pull all the fb config
settings fields in BCM2835FBState out into their own struct.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id:
20180814144436.679-2-peter.maydell@linaro.org
Peter Maydell [Fri, 24 Aug 2018 12:17:48 +0000 (13:17 +0100)]
target/arm: Remove a handful of stray tabs
Following the bulk conversion of the iwMMXt code, there are
just a handful of hard coded tabs in target/arm; fix them.
This is a whitespace-only patch.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-id:
20180821165215.29069-4-peter.maydell@linaro.org
Peter Maydell [Fri, 24 Aug 2018 12:17:48 +0000 (13:17 +0100)]
target/arm: Untabify iwmmxt_helper.c
Untabify the arm iwmmxt_helper.c. This affects only the iwMMXt code.
We've never touched that code in years, so it's not going to get
fixed up by our "change when touched" process, and a bulk change is
not going to be too disruptive.
This commit was produced using Emacs "untabify" (plus one
by-hand removal of a space to fix a checkpatch nit); it is
a whitespace-only change.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-id:
20180821165215.29069-3-peter.maydell@linaro.org
Peter Maydell [Fri, 24 Aug 2018 12:17:47 +0000 (13:17 +0100)]
target/arm: Untabify translate.c
Untabify the arm translate.c. This affects only some lines,
mostly comments, in the iwMMXt code. We've never touched
that code in years, so it's not going to get fixed up
by our "change when touched" process, and a bulk change
is not going to be too disruptive.
This commit was produced using Emacs "untabify"; it is
a whitespace-only change.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-id:
20180821165215.29069-2-peter.maydell@linaro.org
Peter Maydell [Fri, 24 Aug 2018 12:17:47 +0000 (13:17 +0100)]
hw/arm/mps2-tz: Fix MPS2 SCC config register values
Some of the config register values we were setting for the MPS2 SCC
weren't correct:
* the SCC_AID bits [23:20] specify the FPGA build target board revision,
and the SCC_CFG4 register specifies the actual board revision, so
these should have matching values. Claim to be board revision C,
consistently -- we had the revision in the wrong part of SCC_AID.
* SCC_ID bits [15:4] should be 0x505, not decimal 505
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-id:
20180820141116.9118-23-peter.maydell@linaro.org
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Peter Maydell [Fri, 24 Aug 2018 12:17:47 +0000 (13:17 +0100)]
hw/arm/mps2-tz: Instantiate SPI controllers
The SPI controllers in the MPS2 AN505 board are PL022s.
We have a model of the PL022, so create these devices.
We don't currently model the LCD controller that sits behind
one of the PL022s; the others are intended to control devices
that sit on the FPGA's general purpose SPI connector or
"shield" expansion connectors.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-id:
20180820141116.9118-22-peter.maydell@linaro.org
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Peter Maydell [Fri, 24 Aug 2018 12:17:46 +0000 (13:17 +0100)]
hw/ssi/pl022: Correct wrong DMACR and ICR handling
In the PL022, register offset 0x20 is the ICR, a write-only
interrupt-clear register. Register offset 0x24 is DMACR, the DMA
control register. We were incorrectly implementing (a stub version
of) DMACR at 0x20, and not implementing anything at 0x24. Fix this
bug.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-id:
20180820141116.9118-21-peter.maydell@linaro.org
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Peter Maydell [Fri, 24 Aug 2018 12:17:46 +0000 (13:17 +0100)]
hw/ssi/pl022: Correct wrong value for PL022_INT_RT
The PL022 interrupt registers have bits allocated as:
0: ROR (receive overrun)
1: RT (receive timeout)
2: RX (receive FIFO half full or less)
3: TX (transmit FIFO half full or less)
A cut and paste error meant we had the wrong value for
the PL022_INT_RT constant. This bug doesn't affect device
behaviour, because we don't implement the receive timeout
feature and so never set that interrupt bit.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-id:
20180820141116.9118-20-peter.maydell@linaro.org
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Peter Maydell [Fri, 24 Aug 2018 12:17:45 +0000 (13:17 +0100)]
hw/ssi/pl022: Use DeviceState::realize rather than SysBusDevice::init
Move from the legacy SysBusDevice::init method to using
DeviceState::realize.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-id:
20180820141116.9118-19-peter.maydell@linaro.org
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Peter Maydell [Fri, 24 Aug 2018 12:17:45 +0000 (13:17 +0100)]
hw/ssi/pl022: Don't directly call vmstate_register()
Use the DeviceState vmsd pointer rather than calling vmstate_register()
directly.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-id:
20180820141116.9118-18-peter.maydell@linaro.org
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Peter Maydell [Fri, 24 Aug 2018 12:17:45 +0000 (13:17 +0100)]
hw/ssi/pl022: Set up reset function in class init
Currently the PL022 calls pl022_reset() from its class init
function. Make it register a DeviceState reset method instead,
so that we reset the device on system reset.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-id:
20180820141116.9118-17-peter.maydell@linaro.org
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Peter Maydell [Fri, 24 Aug 2018 12:17:44 +0000 (13:17 +0100)]
hw/ssi/pl022: Allow use as embedded-struct device
Create a new include file for the pl022's device struct,
type macros, etc, so that it can be instantiated using
the "embedded struct" coding style.
While we're adding the new file to MAINTAINERS, add
also the .c file, which was missing an entry.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-id:
20180820141116.9118-16-peter.maydell@linaro.org
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Peter Maydell [Fri, 24 Aug 2018 12:17:44 +0000 (13:17 +0100)]
hw/arm/mps2-tz: Create PL081s and MSCs
The AN505 FPGA image includes four PL081 DMA controllers, each
of which is gated by a Master Security Controller that allows
the guest to prevent a non-secure DMA controller from accessing
memory that is used by secure guest code. Create and wire
up these devices.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-id:
20180820141116.9118-15-peter.maydell@linaro.org
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Peter Maydell [Fri, 24 Aug 2018 12:17:44 +0000 (13:17 +0100)]
hw/arm/iotkit: Wire up the lines for MSCs
The IoTKit doesn't have any MSCs itself but it does need
some wiring to connect the external signals from MSCs
in the outer board model up to the registers and the
NVIC IRQ line.
We also need to expose a MemoryRegion corresponding to
the AHB bus, so that MSCs in the outer board model can
use that as their downstream port. (In the FPGA this is
the "AHB Slave Expansion" ports shown in the block
diagram in the AN505 documentation.)
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-id:
20180820141116.9118-14-peter.maydell@linaro.org
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Peter Maydell [Fri, 24 Aug 2018 12:17:44 +0000 (13:17 +0100)]
hw/misc/iotkit-secctl: Wire up registers for controlling MSCs
The IoTKit does not have any Master Security Contollers itself,
but it does provide registers in the secure privilege control
block which allow control of MSCs in the external system.
Add support for these registers.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-id:
20180820141116.9118-13-peter.maydell@linaro.org
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Peter Maydell [Fri, 24 Aug 2018 12:17:43 +0000 (13:17 +0100)]
hw/misc/tz-msc: Model TrustZone Master Security Controller
Implement a model of the TrustZone Master Securtiy Controller,
as documented in the Arm CoreLink SIE-200 System IP for
Embedded TRM (DDI0571G):
https://developer.arm.com/products/architecture/m-profile/docs/ddi0571/g
The MSC is intended to sit in front of a device which can
be a bus master (eg a DMA controller) and programmably gate
its transactions. This allows a bus-mastering device to be
controlled by non-secure code but still restricted from
making accesses to addresses which are secure-only.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-id:
20180820141116.9118-12-peter.maydell@linaro.org
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Peter Maydell [Fri, 24 Aug 2018 12:17:43 +0000 (13:17 +0100)]
hw/misc/iotkit: Wire up the sysctl and sysinfo register blocks
Wire up the system control element's register banks
(sysctl and sysinfo).
This is the last of the previously completely unimplemented
components in the IoTKit.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id:
20180820141116.9118-11-peter.maydell@linaro.org
Peter Maydell [Fri, 24 Aug 2018 12:17:43 +0000 (13:17 +0100)]
hw/misc/iotkit-sysinfo: Implement IoTKit system information block
Implement the IoTKit system control element's system information
block; this is just a pair of read-only version/config registers,
plus the usual PID/CID ID registers.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id:
20180820141116.9118-10-peter.maydell@linaro.org
Peter Maydell [Fri, 24 Aug 2018 12:17:42 +0000 (13:17 +0100)]
hw/misc/iotkit-sysctl: Implement IoTKit system control element
The Arm IoTKit includes a system control element which
provides a block of read-only ID registers and a block
of read-write control registers. Implement a minimal
version of this.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id:
20180820141116.9118-9-peter.maydell@linaro.org
Peter Maydell [Fri, 24 Aug 2018 12:17:42 +0000 (13:17 +0100)]
hw/arm/iotkit: Wire up the S32KTIMER
The IoTKit has a CMSDK timer device that runs on the S32KCLK.
Create this and wire it up.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id:
20180820141116.9118-8-peter.maydell@linaro.org
Peter Maydell [Fri, 24 Aug 2018 12:17:42 +0000 (13:17 +0100)]
hw/arm/iotkit: Wire up the watchdogs
The IoTKit includes three different instances of the
CMSDK APB watchdog; create and wire them up.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id:
20180820141116.9118-7-peter.maydell@linaro.org
Peter Maydell [Fri, 24 Aug 2018 12:17:42 +0000 (13:17 +0100)]
hw/arm/mps2: Wire up dual-timer in mps2-an385 and mps2-an511
The MPS2 FPGA images for the Cortex-M3 (mps2-an385 and mps2-511)
both include a CMSDK dual-timer module. Wire this up.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id:
20180820141116.9118-6-peter.maydell@linaro.org
Peter Maydell [Fri, 24 Aug 2018 12:17:41 +0000 (13:17 +0100)]
hw/arm/iotkit: Wire up the dualtimer
Now we have a model of the CMSDK dual timer, we can wire it
up in the IoTKit.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id:
20180820141116.9118-5-peter.maydell@linaro.org
Peter Maydell [Fri, 24 Aug 2018 12:17:41 +0000 (13:17 +0100)]
hw/timer/cmsdk-apb-dualtimer: Implement CMSDK dual timer module
The Arm Cortex-M System Design Kit includes a "dual-input timer module"
which combines two programmable down-counters. Implement a model
of this device.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id:
20180820141116.9118-4-peter.maydell@linaro.org
Peter Maydell [Fri, 24 Aug 2018 12:17:40 +0000 (13:17 +0100)]
hw/misc/mps2-fpgaio: Implement PSCNTR and COUNTER
In the MPS2 FPGAIO, PSCNTR is a free-running downcounter with
a reload value configured via the PRESCALE register, and
COUNTER counts up by 1 every time PSCNTR reaches zero.
Implement these counters.
We can just increment the counters migration subsection's
version ID because we only added it in the previous commit,
so no released QEMU versions will be using it.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id:
20180820141116.9118-3-peter.maydell@linaro.org
Peter Maydell [Fri, 24 Aug 2018 12:17:40 +0000 (13:17 +0100)]
hw/misc/mps2-fpgaio: Implement 1Hz and 100Hz counters
The MPS2 FPGAIO block includes some simple free-running counters.
Implement these.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id:
20180820141116.9118-2-peter.maydell@linaro.org
Peter Maydell [Fri, 24 Aug 2018 12:17:39 +0000 (13:17 +0100)]
hw/arm/boot: AArch32 kernels should be started in Hyp mode if available
The kernel booting specification for an AArch32 kernel requires that
it is booted in Hyp mode if available; otherwise the kernel can't
enable KVM. We were incorrectly leaving the kernel in SVC mode.
If we're booting an AArch32 kernel in the Nonsecure state and Hyp
mode is available, start in it.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Reviewed-by: Luc Michel <luc.michel@greensocs.com>
Message-id:
20180820153020.21478-7-peter.maydell@linaro.org
Peter Maydell [Fri, 24 Aug 2018 12:17:38 +0000 (13:17 +0100)]
target/arm: Clear CPSR.IL and CPSR.J on 32-bit exception entry
On 32-bit exception entry, CPSR.J must always be set to 0
(see v7A Arm ARM DDI0406C.c B1.8.5). CPSR.IL must also
be cleared on 32-bit exception entry (see v8A Arm ARM
DDI0487C.a G1.10).
Clear these bits. (This fixes a bug which will never be noticed
by non-buggy guests.)
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Reviewed-by: Luc Michel <luc.michel@greensocs.com>
Message-id:
20180820153020.21478-6-peter.maydell@linaro.org
Peter Maydell [Fri, 24 Aug 2018 12:17:38 +0000 (13:17 +0100)]
target/arm: Implement support for taking exceptions to Hyp mode
Implement the necessary support code for taking exceptions
to Hyp mode in AArch32.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Reviewed-by: Luc Michel <luc.michel@greensocs.com>
Message-id:
20180820153020.21478-5-peter.maydell@linaro.org
Peter Maydell [Fri, 24 Aug 2018 12:17:37 +0000 (13:17 +0100)]
target/arm: Factor out code for taking an AArch32 exception
Factor out the code which changes the CPU state so as to
actually take an exception to AArch32. We're going to want
to use this for handling exception entry to Hyp mode.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Reviewed-by: Luc Michel <luc.michel@greensocs.com>
Message-id:
20180820153020.21478-4-peter.maydell@linaro.org
Peter Maydell [Fri, 24 Aug 2018 12:17:37 +0000 (13:17 +0100)]
target/arm: Implement AArch32 HCR and HCR2
The AArch32 HCR and HCR2 registers alias HCR_EL2
bits [31:0] and [63:32]; implement them.
Since HCR2 exists in ARMv8 but not ARMv7, we need new
regdef arrays for "we have EL3, not EL2, we're ARMv8"
and "we have EL2, we're ARMv8" to hold the definitions.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Reviewed-by: Luc Michel <luc.michel@greensocs.com>
Message-id:
20180820153020.21478-3-peter.maydell@linaro.org
Peter Maydell [Fri, 24 Aug 2018 12:17:36 +0000 (13:17 +0100)]
target/arm: Implement RAZ/WI HACTLR2
The v8 AArch32 HACTLR2 register maps to bits [63:32] of ACTLR_EL2.
We implement ACTLR_EL2 as RAZ/WI, so make HACTLR2 also RAZ/WI.
(We put the regdef next to ACTLR_EL2 as a reminder in case we
ever make ACTLR_EL2 something other than RAZ/WI).
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Reviewed-by: Luc Michel <luc.michel@greensocs.com>
Message-id:
20180820153020.21478-2-peter.maydell@linaro.org
Peter Maydell [Fri, 24 Aug 2018 12:17:35 +0000 (13:17 +0100)]
hw/arm/vexpress: Add "virtualization" property controlling presence of EL2
Add a "virtualization" property to the vexpress-a15 board,
controlling presence of EL2. As with EL3, we default to
enabling it, but the user can disable it if they have an
older guest which can't cope with it being present.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Luc Michel <luc.michel@greensocs.com>
Message-id:
20180821132811.17675-10-peter.maydell@linaro.org