It isn't really part of the documentation. Furthermore, entries there
are in not determined order, which breaks build reproducibility.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Similar to the EHCI driver - save/restore relevant BAR and command
register, re-configure DbC on resume and stop/start timer.
On resume trigger sending anything that was queued in the meantime.
Save full BAR value, instead of just the address part, to ease restoring
on resume.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
Re-use rmrr= parameter handling code to handle common device reserved
memory.
Move MAX_USER_RMRR_PAGES limit enforcement to apply only to
user-configured ranges, but not those from internal callers.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
That's possible, because the capability was designed specifically to
allow separate driver handle it, in parallel to unmodified xhci driver
(separate set of registers, pretending the port is "disconnected" for
the main xhci driver etc). It works with Linux dom0, although requires
an awful hack - re-enabling bus mastering behind dom0's backs.
Linux driver does similar thing - see
drivers/usb/early/xhci-dbc.c:xdbc_handle_events().
When controller sharing is enabled in kconfig (option marked as
experimental), dom0 is allowed to use the controller even if Xen uses it
for debug console. Additionally, option `dbgp=xhci,share=` is available
to either prevent even dom0 from using it (`no` value), or allow any
domain using it (`any` value).
In any case, to avoid Linux messing with the DbC, mark this MMIO area as
read-only. This might cause issues for Linux's driver (if it tries to
write something on the same page - like anoter xcap), but makes Xen's
use safe. In practice, as of Linux 5.18, it seems to work without
issues.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
When cable is unplugged, dbc_ensure_running() correctly detects this
situation (DBC_CTRL_DCR flag is clear), and prevent sending data
immediately to the device. It gets only queued in work ring buffers.
When cable is plugged in again, subsequent dbc_flush() will send the
buffered data.
But there is a corner case, where no subsequent data was buffered in the
work buffer, but a TRB was still pending. Ring the doorbell to let the
controller re-send them. For console output it is rare corner case (TRB
is pending for a very short time), but for console input it is very
normal case (there is always one pending TRB for input).
Extract doorbell ringing into separate function to avoid duplication.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Add another work ring buffer for received data, and point IN TRB at it.
Ensure there is always at least one pending IN TRB, so the controller
has a way to send incoming data to the driver.
Note that both "success" and "short packet" completion codes are okay -
in fact it will be "short packet" most of the time, as the TRB length is
about maximum size, not required size.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
The important part is to include those buffers in IOMMU page table
relevant for the USB controller. Otherwise, DbC will stop working as
soon as IOMMU is enabled, regardless of to which domain device assigned
(be it xen or dom0).
If the device is passed through to dom0 or other domain (see later
patches), that domain will effectively have access to those buffers too.
It does give such domain yet another way to DoS the system (as is the
case when having PCI device assigned already), but also possibly steal
the console ring content. Thus, such domain should be a trusted one.
In any case, prevent anything else being placed on those pages by adding
artificial padding.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Register common device reserved memory similar to how ivmd= parameter is
handled.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Add API similar to rmrr= and ivmd= arguments, but in a common code. This
will allow drivers to register reserved memory regardless of the IOMMU
vendor.
The direct reason for this API is xhci-dbc console driver (aka xue),
that needs to use DMA. But future change may unify command line
arguments for user-supplied reserved memory, and it may be useful for
other drivers in the future too.
This commit just introduces an API, subsequent patches will plug it in
appropriate places. The reserved memory ranges needs to be saved
locally, because at the point when they are collected, Xen doesn't know
yet which IOMMU driver will be used.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
This allows configuring EHCI and XHCI consoles separately,
simultaneously, such that e.g. one can be used for the console and the
other by the debugger.
This changes string_param() to custom_param() in both ehci and xhci
drivers. Both drivers parse only values applicable to them.
Suggested-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Handle parameters similar to dbgp=ehci.
Implement this by not resettting dbc->sbdf again in dbc_init_xhc(), but
using a value found there if non-zero. Additionally, add xue->xhc_num to
select n-th controller.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reset ports, to force host system to re-enumerate devices. Otheriwse it
will require the cable to be re-plugged, or will wait in the
"configuring" state indefinitely.
Trick and code copied from Linux:
drivers/usb/early/xhci-dbc.c:xdbc_start()->xdbc_reset_debug_port()
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
The ehci<n> number was parsed but ignored.
Fixes: 322ecbe4ac ("console: add EHCI debug port based serial console")
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Intel LPSS has INTERRUPT_LINE set to 0xff by default, that is declared
by the PCI Local Bus Specification Revision 3.0 (from 2004) as
"unknown"/"no connection". Fallback to poll mode in this case.
The 0xff handling is x86-specific, the surrounding code is guarded with
CONFIG_X86 anyway.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
This is purely based on the spec:
- Intel 500 Series PCH: 635218-006
- Intel 600 Series PCH: 691222-001, 648364-003
This is tested only on TGL-LP added initially, but according to the
spec, they should behave the same.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
This adds support for serial console as found in a laptop with TGL-LP
(StarBook MkV). Since the device is on the bus 0, it needs to be enabled
via "com1=...,amt", not just "...,pci".
Device specification is in Intel docs 631119-007 and 631120-001.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
An interesting corner case occurs when the byte sequence making up endbr64
ends up on a non-instruction boundary. Such embedded instructions mark legal
indirect branch targets as far as the CPU is concerned, which aren't legal as
far as the logic is concerned.
When CET-IBT is active, check for embedded byte sequences. Example failures
look like:
check-endbr.sh xen-syms Fail: Found 2 embedded endbr64 instructions
0xffff82d040325677: test_endbr64 at /local/xen.git/xen/arch/x86/x86_64/entry.S:28
0xffff82d040352da6: init_done at /local/xen.git/xen/arch/x86/setup.c:675
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
set_xcr0() and set_msr_xss() use cached value to avoid setting the
register to the same value over and over. But suspend/resume implicitly
reset the registers and since percpu areas are not deallocated on
suspend anymore, the cache gets stale.
Reset the cache on resume, to ensure the next write will really hit the
hardware. Choose value 0, as it will never be a legitimate write to
those registers - and so, will force write (and cache update).
Note the cache is used io get_xcr0() and get_msr_xss() too, but:
- set_xcr0() is called few lines below in xstate_init(), so it will
update the cache with appropriate value
- get_msr_xss() is not used anywhere - and thus not before any
set_msr_xss() that will fill the cache
Fixes: aca2a985a5 "xen: don't free percpu areas during suspend"
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Release-Acked-by: Ian Jackson <iwj@xenproject.org>
Besides standard UART setup, this device needs enabling
(vendor-specific) "Enhanced Control Bits" - otherwise disabling hardware
control flow (MCR[2]) is ignored. Add appropriate quirk to the
ns16550_setup_preirq(), similar to the handle_dw_usr_busy_quirk(). The
new function act on Exar 2-, 4-, and 8- port cards only. I have tested
the functionality on 2-port card but based on the Linux driver, the same
applies to other models too.
Additionally, Exar card supports fractional divisor (DLD[3:0] register,
at 0x02). This part is not supported here yet, and seems to not
be required for working 115200bps at the very least.
The specification for the 2-port card is available at:
https://www.maxlinear.com/product/interface/uarts/pcie-uarts/xr17v352
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
If fifo size is already set via uart_params, do not force it to 16 - which
may not match the actual hardware. Specifically Exar cards have fifo of
256 bytes.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Using RING_GET_RESPONSE() on a shared ring is easy to use incorrectly
(i.e., by not considering that the other end may alter the data in the
shared ring while it is being inspected). Safe usage of a response
generally requires taking a local copy.
Provide a RING_COPY_RESPONSE() macro to use instead of
RING_GET_RESPONSE() and an open-coded memcpy(). This takes care of
ensuring that the copy is done correctly regardless of any possible
compiler optimizations.
Use a volatile source to prevent the compiler from reordering or
omitting the copy.
This generalizes similar RING_COPY_REQUEST() macro added in 3f20b8def0.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
When device is removed, backend domain (which may be a driver domain) is
responsible for removing backend entries from xenstore. But in case of
driver domain, it has no access to remove all of them - specifically the
directory named after frontend-id remains. This may accumulate enough to
exceed xenstore quote of the driver domain, breaking further devices.
Fix this by calling libxl__xs_path_cleanup() on the backend path from
libxl__device_destroy() in the toolstack domain too. Note
libxl__device_destroy() is called when the driver domain already removed
what it can (see device_destroy_be_watch_cb()->device_hotplug_done()).
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Wei Liu <wl@xen.org>
Fix the resume path to load the shadow stack pointer from saved_ssp (not
saved_rsp), to match what suspend path does.
Fixes: 633ecc4a7c ("x86/S3: Save and restore Shadow Stack configuration")
Backport: 4.14
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
In file included from /usr/include/string.h:495,
from libxl_internal.h:38,
from libxl_utils.c:20:
In function 'strncpy',
inlined from 'libxl__prepare_sockaddr_un' at libxl_utils.c:1262:5:
/usr/include/bits/string_fortified.h:106:10: error: '__builtin_strncpy' specified bound 108 equals destination size [-Werror=stringop-truncation]
106 | return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Ian Jackson <ian.jackson@eu.citrix.com>
It seems xlu_pci_parse_bdf has a state machine that is too complex for
gcc to understand. The build fails with:
libxlu_pci.c: In function 'xlu_pci_parse_bdf':
libxlu_pci.c:32:18: error: 'func' may be used uninitialized in this function [-Werror=maybe-uninitialized]
32 | pcidev->func = func;
| ~~~~~~~~~~~~~^~~~~~
libxlu_pci.c:51:29: note: 'func' was declared here
51 | unsigned dom, bus, dev, func, vslot = 0;
| ^~~~
libxlu_pci.c:31:17: error: 'dev' may be used uninitialized in this function [-Werror=maybe-uninitialized]
31 | pcidev->dev = dev;
| ~~~~~~~~~~~~^~~~~
libxlu_pci.c:51:24: note: 'dev' was declared here
51 | unsigned dom, bus, dev, func, vslot = 0;
| ^~~
libxlu_pci.c:30:17: error: 'bus' may be used uninitialized in this function [-Werror=maybe-uninitialized]
30 | pcidev->bus = bus;
| ~~~~~~~~~~~~^~~~~
libxlu_pci.c:51:19: note: 'bus' was declared here
51 | unsigned dom, bus, dev, func, vslot = 0;
| ^~~
libxlu_pci.c:29:20: error: 'dom' may be used uninitialized in this function [-Werror=maybe-uninitialized]
29 | pcidev->domain = domain;
| ~~~~~~~~~~~~~~~^~~~~~~~
libxlu_pci.c:51:14: note: 'dom' was declared here
51 | unsigned dom, bus, dev, func, vslot = 0;
| ^~~
cc1: all warnings being treated as errors
Workaround it by setting the initial value to invalid value (0xffffffff)
and then assert on each value being set. This way we mute the gcc
warning, while still detecting bugs in the parse code.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Ian Jackson <ian.jackson@eu.citrix.com>
Qemu supports only 4 emulated IDE disks, when given more (or with higher
indexes), it will fail to start. Since the disks can still be accessible
using PV interface, just ignore emulated path and log a warning, instead
of rejecting the configuration altogether.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Device model stubdomains (both Mini-OS + qemu-trad and linux + qemu-xen)
are always started with at least 3 consoles: log, save, and restore.
Until xenconsoled learns how to handle multiple consoles, this is needed
for save/restore support.
For Mini-OS stubdoms, this is a bug. In practice, it works in most
cases because there is something else that triggers qemu in dom0 too:
vfb/vkb added if vnc/sdl/spice is enabled.
Additionally, Linux-based stubdomain waits for all the backends to
initialize during boot. Lack of some console backends results in
stubdomain startup timeout.
This is a temporary patch until xenconsoled will be improved.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
[Updated commit message with Marek's explanation from mailing list.]
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Access to QMP of QEMU in Linux stubdomain is possible over vchan
connection. Handle the actual vchan connection in a separate process
(vchan-socket-proxy). This simplified integration with QMP (already
quite complex), but also allows preliminary filtering of (potentially
malicious) QMP input.
Since only one client can be connected to vchan server at the same time
and it is not enforced by the libxenvchan itself, additional client-side
locking is needed. It is implicitly implemented by vchan-socket-proxy,
as it handle only one connection at a time. Note that qemu supports only
one simultaneous client on a control socket anyway (but in UNIX socket
case, it enforce it server-side), so it doesn't add any extra
limitation.
libxl qmp client code already has locking to handle concurrent access
attempts to the same qemu qmp interface.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Squash in changes of regenerated autotools files.
Kill the vchan-socket-proxy so we don't leak the daemonized processes.
libxl__stubdomain_is_linux_running() works against the guest_domid, but
the xenstore path is beneath the stubdomain. This leads to the use of
libxl_is_stubdom in addition to libxl__stubdomain_is_linux_running() so
that the stubdomain calls kill for the qmp-proxy.
Also call libxl__qmp_cleanup() to remove the unix sockets used by
vchan-socket-proxy. vchan-socket-proxy only creates qmp-libxl-$domid,
and libxl__qmp_cleanup removes that as well as qmp-libxenstat-$domid.
However, it tolerates ENOENT, and a stray qmp-libxenstat-$domid should
not exist.
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Add a simple proxy for tunneling socket connection over vchan. This is
based on existing vchan-node* applications, but extended with socket
support. vchan-socket-proxy serves both as a client and as a server,
depending on parameters. It can be used to transparently communicate
with an application in another domian that normally expose UNIX socket
interface. Specifically, it's written to communicate with qemu running
within stubdom.
Server mode listens for vchan connections and when one is opened,
connects to a pointed UNIX socket. Client mode listens on UNIX
socket and when someone connects, opens a vchan connection. Only
a single connection at a time is supported.
Additionally, socket can be provided as a number - in which case it's
interpreted as already open FD (in case of UNIX listening socket -
listen() needs to be already called). Or "-" meaning stdin/stdout - in
which case it is reduced to vchan-node2 functionality.
Example usage:
1. (in dom0) vchan-socket-proxy --mode=client <DOMID>
/local/domain/<DOMID>/data/vchan/1234 /run/qemu.(DOMID)
2. (in DOMID) vchan-socket-proxy --mode=server 0
/local/domain/<DOMID>/data/vchan/1234 /run/qemu.(DOMID)
This will listen on /run/qemu.(DOMID) in dom0 and whenever connection is
made, it will connect to DOMID, where server process will connect to
/run/qemu.(DOMID) there. When client disconnects, vchan connection is
terminated and server vchan-socket-proxy process also disconnects from
qemu.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Rely on a wrapper script in stubdomain to attach relevant consoles to
qemu. The save console (1) must be attached to fdset/1. When
performing a restore, $STUBDOM_RESTORE_INCOMING_ARG must be replaced on
the qemu command line by "fd:$FD", where $FD is an open file descriptor
number to the restore console (2).
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Address TODO in dm_state_save_to_fdset: Only remove savefile for
non-stubdom.
Use $STUBDOM_RESTORE_INCOMING_ARG instead of fd:3 and update commit
message.
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Let the server know when the client is connected. Otherwise server will
notice only when client send some data.
This change does not break existing clients, as libvchan user should
handle spurious notifications anyway (for example acknowledge of remote
side reading the data).
Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Replace spaces with tabs to match the file's whitespace.
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
This allows using arguments with spaces, like -append, without
nominating any special "separator" character.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
Write arguments in dm-argv directory instead of overloading mini-os's
dmargs string.
Make libxl__write_stub_dmargs vary behaviour based on the
is_linux_stubdom flag.
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Do not prohibit anymore using stubdomain with qemu-xen.
To help distingushing MiniOS and Linux stubdomain, add helper inline
functions libxl__stubdomain_is_linux() and
libxl__stubdomain_is_linux_running(). Those should be used where really
the difference is about MiniOS/Linux, not qemu-xen/qemu-xen-traditional.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
When qemu is running in stubdomain, any attempt to initialize vnc/sdl
there will crash it (on failed attempt to load a keymap from a file). If
vfb is present, all those cases are skipped. But since
b053f0c4c9 "libxl: do not start dom0 qemu
for stubdomain when not needed" it is possible to create a stubdomain
without vfb and contrary to the comment -vnc none do trigger VNC
initialization code (just skips exposing it externally).
Change the implicit SDL avoiding method to -nographics option, used when
none of SDL or VNC is enabled.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Before dfcccc6631 "efi: use directmap to access runtime services table"
all usages of efi_rs pointer were guarded by efi_rs_enter(), which
implicitly refused to operate with efi=no-rs (by checking if
efi_l4_pgtable is NULL - which is the case for efi=no-rs). The said
commit (re)moved that call as unneeded for just reading content of
efi_rs structure - to avoid unnecessary page tables switch. But it
neglected to check if efi_rs access is legal.
Fix this by adding explicit check for runtime service being enabled in
the cases that do not use efi_rs_enter().
Reported-by: Roman Shaposhnik <roman@zededa.com>
Fixes: dfcccc6631 "efi: use directmap to access runtime services table"
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
Do not require switching page tables to access (static) information in
the runtime services table itself, use directmap for this. This allows
exiting early from XEN_EFI_query_capsule_capabilities,
XEN_EFI_update_capsule and XEN_EFI_query_variable_info (in case of not
supported call) without all the impact of page table switch.
Suggested-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
Some UEFI implementations are not happy about lack of
SetVirtualAddressMap() call. Likely abuse the address map change
notification to do things beyond the necessary ConvertPointer() calls.
Specifically, wihtout the SetVirtualAddressMap() call, some access
EfiBootServices{Code,Data}, or even totally unmapped areas. Example
crash of GetVariable() call on Thinkpad W540:
Xen call trace:
[<0000000000000080>] 0000000000000080
[<8c2b0398e0000daa>] 8c2b0398e0000daa
Pagetable walk from ffffffff858483a1:
L4[0x1ff] = 0000000000000000 ffffffffffffffff
****************************************
Panic on CPU 0:
FATAL PAGE FAULT
[error_code=0002]
Faulting linear address: ffffffff858483a1
****************************************
Fix this by calling SetVirtualAddressMap() runtime service, giving it
1:1 map for areas marked as needed during runtime. The address space in
which EFI runtime services are called is unchanged, but UEFI view of it
may be.
Since it's fairly late in Xen 4.13 development cycle, disable it
by default and hide behind EXPERT.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
Remove unused (#ifdef-ed out) code. Reviving it in its current shape
won't fly because:
- SetVirtualAddressMap() needs to be called with 1:1 mapping, which
isn't the case at this time
- it uses directmap, which may go away soon
- it uses directmap, which is mapped with NX, breaking EfiRuntimeServicesCode
No functional change.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
Stubdomain do not have it's own config file - its configuration is
derived from target domains. Do not try to manipulate it when attaching
PCI device.
This bug prevented starting HVM with stubdomain and PCI passthrough
device attached.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
When qemu is running in stubdomain, handling "pci-ins" command will fail
if pcifront is not initialized already. Fix this by sending such command
only after confirming that pciback/front is running.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
HVM domains use IOMMU and device model assistance for communicating with
PCI devices, xen-pcifront/pciback isn't directly needed by HVM domain.
But pciback serve also second function - it reset the device when it is
deassigned from the guest and for this reason pciback needs to be used
with HVM domain too.
When HVM domain has device model in stubdomain, attaching xen-pciback to
the target domain itself may prevent attaching xen-pciback to the
(PV) stubdomain, effectively breaking PCI passthrough.
Fix this by attaching pciback only to one domain: if PV stubdomain is in
use, let it be stubdomain (the commit prevents attaching device to target
HVM in this case); otherwise, attach it to the target domain.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
When libxl__device_pci_add() is called, stubdomain is already running,
even when still constructing the target domain. Previously, do_pci_add()
was called with 'starting' hardcoded to false, but now do_pci_add() shares
'starting' flag in pci_add_state for both target domain and stubdomain.
Fix this by resetting (local) 'starting' to false in pci_add_dm_done()
(previously part of do_pci_add()) when handling stubdomain, regardless
of pas->starting value.
Fixes: 11db56f9a6 (libxl_pci: Use libxl__ao_device with libxl__device_pci_add)
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
match_watch_by_token() when returns an error, sets also exception within
python. This is generally the right thing to do, but when
xspy_read_watch() handle EAGAIN error internally, the exception needs to
be cleared. Otherwise it will fail like this:
xen.lowlevel.xs.Error: (11, 'Resource temporarily unavailable')
The above exception was the direct cause of the following exception:
Traceback (most recent call last):
(...)
result = self.handle.read_watch()
SystemError: <method 'read_watch' of 'xen.lowlevel.xs.xs' objects> returned a result with an error set
Fixes f6e1023412 "python: Extract registered watch search logic from xspy_read_watch()"
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Wei Liu <wl@xen.org>
Specifically:
xen/lowlevel/xc/xc.c: In function ‘pyxc_domain_create’:
xen/lowlevel/xc/xc.c:147:24: error: comparison of integer expressions of different signedness: ‘int’ and ‘long unsigned int’ [-Werror=sign-compare]
147 | for ( i = 0; i < sizeof(xen_domain_handle_t); i++ )
| ^
xen/lowlevel/xc/xc.c: In function ‘pyxc_domain_sethandle’:
xen/lowlevel/xc/xc.c:312:20: error: comparison of integer expressions of different signedness: ‘int’ and ‘long unsigned int’ [-Werror=sign-compare]
312 | for ( i = 0; i < sizeof(xen_domain_handle_t); i++ )
| ^
xen/lowlevel/xc/xc.c: In function ‘pyxc_domain_getinfo’:
xen/lowlevel/xc/xc.c:391:24: error: comparison of integer expressions of different signedness: ‘int’ and ‘long unsigned int’ [-Werror=sign-compare]
391 | for ( j = 0; j < sizeof(xen_domain_handle_t); j++ )
| ^
xen/lowlevel/xc/xc.c: In function ‘pyxc_get_device_group’:
xen/lowlevel/xc/xc.c:677:20: error: comparison of integer expressions of different signedness: ‘int’ and ‘uint32_t’ {aka ‘unsigned int’} [-Werror=sign-compare]
677 | for ( i = 0; i < num_sdevs; i++ )
| ^
xen/lowlevel/xc/xc.c: In function ‘pyxc_physinfo’:
xen/lowlevel/xc/xc.c:988:20: error: comparison of integer expressions of different signedness: ‘int’ and ‘long unsigned int’ [-Werror=sign-compare]
988 | for ( i = 0; i < sizeof(pinfo.hw_cap)/4; i++ )
| ^
xen/lowlevel/xc/xc.c:994:20: error: comparison of integer expressions of different signedness: ‘int’ and ‘long unsigned int’ [-Werror=sign-compare]
994 | for ( i = 0; i < ARRAY_SIZE(virtcaps_bits); i++ )
| ^
xen/lowlevel/xc/xc.c:998:24: error: comparison of integer expressions of different signedness: ‘int’ and ‘long unsigned int’ [-Werror=sign-compare]
998 | for ( i = 0; i < ARRAY_SIZE(virtcaps_bits); i++ )
| ^
xen/lowlevel/xs/xs.c: In function ‘xspy_ls’:
xen/lowlevel/xs/xs.c:191:23: error: comparison of integer expressions of different signedness: ‘int’ and ‘unsigned int’ [-Werror=sign-compare]
191 | for (i = 0; i < xsval_n; i++)
| ^
xen/lowlevel/xs/xs.c: In function ‘xspy_get_permissions’:
xen/lowlevel/xs/xs.c:297:23: error: comparison of integer expressions of different signedness: ‘int’ and ‘unsigned int’ [-Werror=sign-compare]
297 | for (i = 0; i < perms_n; i++) {
| ^
cc1: all warnings being treated as errors
Use size_t for loop iterators where it's compared with sizeof() or
similar construct.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
On some machines (for example Thinkpad P52), UEFI GOP reports
framebuffer located above 4GB (0x4000000000 on that machine). This
address does not fit in {xen,dom0}_vga_console_info.u.vesa_lfb.lfb_base
field, which is 32bit. The overflow here cause all kind of memory
corruption when anything tries to write something on the screen,
starting with zeroing the whole framebuffer in vesa_init().
Fix this similar to how it's done in Linux: add ext_lfb_base field at
the end of the structure, to hold upper 32bits of the address. Since the
field is added at the end of the structure, it will work with older
Linux versions too (other than using possibly truncated address - no
worse than without this change). Thanks to ABI containing size of the
structure (start_info.console.dom0.info_size), Linux can detect when
this field is present and use it appropriately then.
Since this change public interface and use __XEN_INTERFACE_VERSION__,
bump __XEN_LATEST_INTERFACE_VERSION__.
Note: if/when backporting this change to Xen <= 4.12, #if in xen.h needs
to be extended with " || defined(__XEN__)".
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Commit f089fddd94 "xen: report PV capability in sysctl and use it in
toolstack" changed meaning of virt_caps bit 1 - previously it was
"directio", but was changed to "pv" and "directio" was moved to bit 2.
Adjust python wrapper to use #defines for the bits values, and add
reporting of both "pv_directio" and "hvm_directio".
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
vlfb_info is an alias for vga_console_info.u.vesa_lfb, so this change is
purely cosmetic. But using the same name helps reading the code.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
The limit 1900x1200 do not match real world devices (1900 looks like a
typo, should be 1920). But in practice the limits are arbitrary and do
not serve any real purpose. As discussed in "Increase framebuffer size
to todays standards" thread, drop them completely.
This fixes graphic console on device with 3840x2160 native resolution.
Suggested-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
When bitmap_fill(..., 0) is called, do not try to write anything. Before
this patch, it tried to write almost LONG_MAX, surely overwriting
something.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
This will prevent race when client waits for server with xs_watch - all
entries should appear at once.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Use physinfo.max_cpu_id instead of physinfo.nr_cpus to get max CPU id.
This fixes for example 'xenpm get-cpufreq-para' with smt=off, which
otherwise would miss half of the cores.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Since udev is no longer used to call hotplug scripts (neither in dom0
nor driver domain), this scripts is no longer referenced anywhere. libxl
(xl devd or else) has own cleanup code.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
gdb 8.0 fixed bounds checking for 'g' packet (commit
9dc193c3be85aafa60ceff57d3b0430af607b4ce "Check for truncated
registers in process_g_packet"). This revealed that gdbsx did
not properly formatted 'g' packet - segment registers and eflags are
expected to be 32-bit fields in the response (according to
gdb/features/i386/64bit-core.xml in gdb sources). Specific error is:
Truncated register 26 in remote 'g' packet
instead of silently truncating part of register.
Additionally, it looks like segment registers of 64bit guests were never
reported correctly, because of type mismatch.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Add --replace-escape option to xenconsoled, which replaces ESC with
'.' in console output written to log file. This makes it slightly safer
to do tail -f on a console output of untrusted guest.
The pty output is unaffected by this option.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
[ wei: move variables into a narrower scope ]
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Older gcc does not support #pragma GCC diagnostics, so use alternative
approach - change variable type to uint32_t (this code handle 32-bit
requests only anyway), which apparently also avoid gcc complaining about
this (otherwise correct) code.
Fixes 437e00fea0 "tools/kdd: mute spurious
gcc warning"
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
Acked-by: Tim Deegan <tim@xen.org>
gcc-8 complains:
kdd.c:698:13: error: 'memcpy' offset [-204, -717] is out of the bounds [0, 216] of object 'ctrl' with type 'kdd_ctrl' {aka 'union <anonymous>'} [-Werror=array-bounds]
memcpy(buf, ((uint8_t *)&ctrl.c32) + offset, len);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
kdd.c: In function 'kdd_select_callback':
kdd.c:642:14: note: 'ctrl' declared here
kdd_ctrl ctrl;
^~~~
But this is impossible - 'offset' is unsigned and correctly validated
few lines before.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Release-Acked-by: Juergen Gross <jgross@suse.com>
gcc-8 complains:
vhd-util-read.c: In function 'vhd_util_read':
vhd-util-read.c:50:24: error: '%lu' directive output may be truncated writing between 1 and 20 bytes into a region of size 15 [-Werror=format-truncation=]
snprintf(nbuf, nsize, "%" PRIu64, num);
^~~
vhd-util-read.c:50:25: note: format string is defined here
snprintf(nbuf, nsize, "%" PRIu64, num);
vhd-util-read.c:50:24: note: directive argument in the range [0, 18446744073709551614]
snprintf(nbuf, nsize, "%" PRIu64, num);
^~~
vhd-util-read.c:50:2: note: 'snprintf' output between 2 and 21 bytes into a destination of size 15
snprintf(nbuf, nsize, "%" PRIu64, num);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
vhd-util-read.c:43:24: error: '%#lx' directive output may be truncated writing between 1 and 18 bytes into a region of size 15 [-Werror=format-truncation=]
snprintf(nbuf, nsize, "%#" PRIx64 , num);
^~~~
vhd-util-read.c:43:25: note: format string is defined here
snprintf(nbuf, nsize, "%#" PRIx64 , num);
vhd-util-read.c:43:24: note: directive argument in the range [0, 18446744073709551614]
snprintf(nbuf, nsize, "%#" PRIx64 , num);
^~~~
vhd-util-read.c:43:2: note: 'snprintf' output between 2 and 19 bytes into a destination of size 15
snprintf(nbuf, nsize, "%#" PRIx64 , num);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Make the buffer larger.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Release-Acked-by: Juergen Gross <jgross@suse.com>
gcc-8 complains:
tapdisk-vbd.c: In function 'tapdisk_vbd_resume_ring':
tapdisk-vbd.c:1671:53: error: 'snprintf' output may be truncated before the last format character [-Werror=format-truncation=]
snprintf(params.name, sizeof(params.name) - 1, "%s", message);
^
tapdisk-vbd.c:1671:3: note: 'snprintf' output between 1 and 256 bytes into a destination of size 255
snprintf(params.name, sizeof(params.name) - 1, "%s", message);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
The "- 1" in buffer size should be actually applied to message, to leave
place for terminating '\0', not the other way around (truncate '\0' even
if it would fit).
In function 'tapdisk_control_open_image',
inlined from 'tapdisk_control_handle_request' at tapdisk-control.c:660:10:
tapdisk-control.c:465:2: error: 'strncpy' specified bound 256 equals destination size [-Werror=stringop-truncation]
strncpy(params.name, vbd->name, BLKTAP2_MAX_MESSAGE_LEN);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In function 'tapdisk_control_create_socket',
inlined from 'tapdisk_control_open' at tapdisk-control.c:836:9:
tapdisk-control.c:793:2: error: 'strncpy' specified bound 108 equals destination size [-Werror=stringop-truncation]
strncpy(saddr.sun_path, td_control.path, sizeof(saddr.sun_path));
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
block-qcow.c: In function 'qcow_create':
block-qcow.c:1216:5: error: 'strncpy' specified bound 4096 equals destination size [-Werror=stringop-truncation]
strncpy(backing_filename, backing_file,
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
sizeof(backing_filename));
~~~~~~~~~~~~~~~~~~~~~~~~~
I those cases, reduce size of copied string and make sure final '\0' is
added.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Release-Acked-by: Juergen Gross <jgross@suse.com>
gcc-8 complains:
gx_main.c: In function 'prepare_stop_reply':
gx_main.c:385:9: error: 'strncpy' output truncated before terminating nul copying 6 bytes from a string of the same length [-Werror=stringop-truncation]
strncpy(buf, "watch:", 6);
^~~~~~~~~~~~~~~~~~~~~~~~~
Since terminating '\0' isn't needed here at all, switch to memcpy.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Release-Acked-by: Juergen Gross <jgross@suse.com>
gcc-8 complains:
xen-lowmemd.c: In function 'handle_low_mem':
xen-lowmemd.c:80:55: error: '%s' directive output may be truncated writing up to 511 bytes into a region of size 489 [-Werror=format-truncation=]
snprintf(error, BUFSZ,"Failed to write target %s to xenstore", data);
^~ ~~~~
xen-lowmemd.c:80:9: note: 'snprintf' output between 36 and 547 bytes into a destination of size 512
snprintf(error, BUFSZ,"Failed to write target %s to xenstore", data);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In practice it wouldn't happen, because 'data' contains string
representation of 64-bit unsigned number (20 characters at most).
But place a limit to mute gcc warning.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Release-Acked-by: Juergen Gross <jgross@suse.com>
gcc-8 warns about possible truncation of trailing '\0'.
Final character is overridden by '\0' anyway, so don't bother to copy
it.
This fixes compile failure:
xc_pm.c: In function 'xc_set_cpufreq_gov':
xc_pm.c:308:5: error: 'strncpy' specified bound 16 equals destination size [-Werror=stringop-truncation]
strncpy(scaling_governor, govname, CPUFREQ_NAME_LEN);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Release-Acked-by: Juergen Gross <jgross@suse.com>
Similar functionality to libxl_domain_suspend(), but do not save domains
state to any file. Only suspend the domain and keep it in suspended
shutdown state (do not destroy it). Such domain can be later woken up
with libxl_domain_resume. The main reason for this functionality is to
suspend the host while some domains are running, potentially holding PCI
devices. This will give a chance to a driver in such a domain to
properly suspend the device.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Signed-off-by: Marcus of Wetware Labs <marcus@wetwa.re>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Backend domain may be independently destroyed - there is no
synchronization of libxl structures (including /libxl tree) elsewhere.
Backend might also remove the device info from its backend xenstore
subtree on its own.
We have various cases (not comprehensive list):
- both frontend and backend operational: after setting
be/state=XenbusStateClosing backend wait for frontend confirmation
and respond with be/state=XenbusStateClosed; then libxl in dom0
remove frontend entries and libxl in backend domain (which may be the
same) remove backend entries
- unresponsive backend/frontend: after a timeout, force=1 is used to remove
frontend entries, instead of just setting
be/state=XenbusStateClosing; then wait for be/state=XenbusStateClosed.
If that timeout too, remove both frontend and backend entries
- backend gone, with this patch: no place for setting/waiting on
be/state - go directly to removing frontend entries, without waiting
for be/state=XenbusStateClosed (this is the difference vs force=1)
Without this patch the end result is similar, both frontend and backend
entries are removed, but in case of backend gone:
- libxl waits for be/state=XenbusStateClosed (and obviously timeout)
- return value from the function signal an error, which for example
confuse libvirt - it thinks the device remove failed, so is still
there
If such situation is detected, do not fail the removal, but finish the
cleanup of the frontend side and return 0.
This is just workaround, the real fix should watch when the device
backend is removed (including backend domain destruction) and remove
frontend at that time. And report such event to higher layer code, so
for example libvirt could synchronize its state.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Do not setup vfb+vkb when no access method was configured. Then check if
qemu is really needed.
The only not configurable thing forcing qemu running in dom0 after this
change are consoles used to save/restore. But even in that case, there
is much smaller part of qemu exposed.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
If xc_gntshr_open failed the only thing to cleanup is free allocated
memory. So instead of calling libxenvchan_close (which assume
valid calculated buffers being mmaped already) free memory and return.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
This is result of parsing cpu_map.xml from libvirt.
The most important part is handling leaf 0x00000007, but while at it add
other bits too.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
GCC-7 have -Wimplicit-fallthrough enabled with -Wextra. Add appropriate
comment which both mute the warning and improve readibility.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
In Python3 'long' type have been merged into 'int', '1L' syntax is no
longer valid. Assign 'int' type to a 'long' variable in python3, so
'long(1)' will give correct result in both python2 and python3.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
In Python3, PyTypeObject looks slightly different, and also module
initialization is different. Instead of Py_InitModule, PyModule_Create
should be called on already defined PyModuleDef structure. And then
initialization function should return that module.
Additionally initialization function should be named PyInit_<name>,
instead of init<name>.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
In Python3 'int' and 'long' types are the same, there are no longer
separate PyInt_* functions. Provide convenient #defines to limit #if in
code.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
In Python2 PyBytes is the same as PyString, but in Python3 PyString is
gone and 'str' is really PyUnicode in C-API.
When handling arbitrary data, use PyBytes - which is the right thing to
do in Python3, and pose no API change in Python2. When handling
xenstore paths and transaction ids, which have well defined format, use
PyUnicode - to ease API usage - no need to prefix all xenstore paths
with 'b' when migrating scripts to Python3.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Fields not named here will be zero-initialized anyway, but using this
way will be much easier to support both Python2 and Python3.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Py_TYPE works on both Python2 and Python3, while internals of
PyObject_HEAD have changed.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
tp_getattr method of type object is deprecated already in Python2 and
gone in Python3. Default implementation does the same as this custom one.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Having DefaultDependencies=no means it can be started before / is
remounted read-write, which will result in various failures (to start
with opening the log).
Since "libxl: trigger attach events for devices attached before xl devd
startup" it is no longer important to start it as early as possible,
because it will process devices created before its startup.
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
When this daemon is started after creating backend device, that device
will not be configured.
Racy situation:
1. driver domain is started
2. frontend domain is started (just after kicking driver domain off)
3. device in frontend domain is connected to the backend (as specified
in frontend domain configuration)
4. xl devd is started in driver domain
End result is that backend device in driver domain is not configured
(like network interface is not enabled), so the device doesn't work.
Fix this by artifically triggering events for devices already present in
xenstore before xl devd is started. Do this only after xenstore watch is
already registered, and only for devices not already initialized (in
XenbusStateInitWait state).
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
The remote (hostile) process can set ring buffer indices to any value
at any time. If that happens, it is possible to get "buffer space"
(either for writing data, or ready for reading) negative or greater
than buffer size. This will end up with buffer overflow in the second
memcpy inside of do_send/do_recv.
Fix this by introducing new available bytes accessor functions
raw_get_data_ready and raw_get_buffer_space which are robust against
mad ring states, and only return sanitised values.
Proof sketch of correctness:
Now {rd,wr}_{cons,prod} are only ever used in the raw available bytes
functions, and in do_send and do_recv.
The raw available bytes functions do unsigned arithmetic on the
returned values. If the result is "negative" or too big it will be
>ring_size (since we used unsigned arithmetic). Otherwise the result
is a positive in-range value representing a reasonable ring state, in
which case we can safely convert it to int (as the rest of the code
expects).
do_send and do_recv immediately mask the ring index value with the
ring size. The result is always going to be plausible. If the ring
state has become mad, the worst case is that our behaviour is
inconsistent with the peer's ring pointer. I.e. we read or write to
arguably-incorrect parts of the ring - but always parts of the ring.
And of course if a peer misoperates the ring they can achieve this
effect anyway.
So the security problem is fixed.
This is XSA-86.
(The patch is essentially Ian Jackson's work, although parts of the
commit message are by Marek.)
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
