Skip to content

usbvm#30

Merged
crogers1 merged 26 commits intoOpenXT:masterfrom
jandryuk:usbvm-5.10
Nov 9, 2023
Merged

usbvm#30
crogers1 merged 26 commits intoOpenXT:masterfrom
jandryuk:usbvm-5.10

Conversation

@jandryuk
Copy link
Contributor

This adds a second "stub-mode" to vusb-daemon, selected by command line argument. Regular mode is mostly the same as before with vusb-daemon on dbus and attaching pvUSB devices. Dom0 can still run this way without a usbvm. stub-mode is for running in usbvm.

Communication between the dom0 and usbvm vusb-daemon occurs over xenstore. In stub-mode, vusb-daemon does not connect to dbus. It writes its USB devices in xenstore under data/usb. stub-mode has a xenstore watch for device assignment requests, so it can bind the usb device to the backend driver.

When a usbvm is detected, uuid 00000000-0000-0000-0000-000000000003, The dom0 vusbd populates its device list from the xenstore data. Policy is still enforced in dom0. dom0 users a xenstore write to instruct the usbvm to bind USB devices, and then does the frontend-backend xenstore writes to set up the PV connection.

@jandryuk
Copy link
Contributor Author

Force pushed rebased on master to avoid the merge errors in src/main.c

@jandryuk
Copy link
Contributor Author

One thing to note is udev & sysfs properties aren't supported for usbvm/stub-mode. An RPC mechanism would be needed to have the dom0 policy piece query usbvm for the desired properties. dom0 could then make its policy decision based off the returned data.

jandryuk added 24 commits May 18, 2023 15:35
We have the xs_dom0path variable for this purpose.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Allow flexibility will running the backend from another domain.  The
code will key off UUID 00000000-0000-0000-0000-000000000003 for a USBVM
and use dom0 otherwise.  Timing might be weird, but usbvm should appear
before anything else.

If usbvm reboots, we try to pick up the new domid.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
We need to set permissions properly for usbvm to be able to access
the xenstore frontend and backend paths that dom0 creates.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Switch from the hardcoded 0 to the actual backend domid.  The
backend will fail to map the front end without this.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
The generic delete code will be useful from the xenstore device
detection.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
udev_find_more and its helpers don't actually need the device_t
structure for any of their actions.  Rework the functions to return the
type which can then be assigned to device_t directly.

libusb_find_more_about_nic() needs to take the vid and pid and return an
integer type for integration with the other usbvm code.  Make the
change.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Pass the type into device_add instead of setting it manually afterwards.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
We'll need our domid to properly set permissions since DOMID_SELF
doesn't seem to be automatically translated.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Move the freeing of a device_t into a helper function.  We also free
serial now which would otherwise leak memory.  We take advantage of
free(NULL) being allowable since serial isn't always set.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Read device information out of xenstore.

Do some basic input validation for device_t's read from xenstore.  If a
field is not set, then we bail on adding the device.  We also ensure
that the number values are semi-valid.

It is okay for serial to be NULL, so that is non-fatal.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
The USB VM will write these XenStore entries to pass USB device info
from USB VM to vusb-daemon in Dom0.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Remove the xenstore entry for a device when it is unplugged.  This is
how USB VM communicates device removal to Dom0's vusb-daemon.  It is
specific to USB VM.

If we're removing a xenstore device node, we should clean up the watch
at the same time.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
When USB VM removes a xenstore device entry, react to it by removing the
internal device representation.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
xenstore_init is a combination of one-time initialization and a runtime
updating of xs_backend_path.  Move the runtime changes into
xenstore_new_backend to keep xenstore_init for onetime initialization.

We'll be re-ordering xenstore_init to before usb_backend_domid is
available.

With the refactoring, we drop xenstore_init()'s now unused argument.
Also drop the incorrect comment.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Move the return of the xenstore fd from xsdev_watch_init to
xenstore_init.  The fd comes from the xenstore handle and is independent
of the watch.

Now, xsdev_watch_init can return the result of xs_watch for error
detection.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
We won't always be running dbus, so hide the conditional use of g_xcbus
inside a wrapper function.  This avoids the conditionals in the main
select loop.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Add a stub mode that disables the DBus rpc.  This mode just listens to
udev and xenstore watches and writes xenstore entries.  Look for the
explicit "stub-mode" command line argument to enable stub-mode.

vusb-manager in usbvm cannot talk dbus.  It's not connected or
initialized.  When a device is removed it will call down through
common_device_del to usbmanager_device_remove.  We want this for
xenstore but not udev based removal with a USB VM.  For dom0 backend, we
need this for udev removal, so we cannot easily condition the send by
the call path.  Instead, just make the dbus sends conditioned on a dbus
connection being available.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
get_usbinfo uses udev to generate a usbinfo_t structure.  We can just
grab the fields our of device_t and bypass udev for this.

I guess calling into udev would confirm the device is present, but dom0
cannot do that for a device in usbvm.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
vusb-daemon dom0 must notify the usbvm to perform the vusb_assign.  We
use xenstore for the notification.

vusb_assign now needs a local and remote version to handle the different
cases.

xasprintf is no long static since it's called from vusb_assign_remote.

We need a 0 on success, so we have to convert the bool true of
xs_write to 0.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
vusb_assign needs to run in usbvm, so call it when a xenstore watch
fires.  Check for an assign: prefix and handle those while logging
totally unexpected values.

watch_path is only set in dom0, so we can't access it in USB VM.  Move
its check after the watch_token check.  watch_token should only trigger
in dom0, and watch_path will be set in that case.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
This is a choice.  We revert to dom0 operations when the USB VM goes
away.  The other option would be to set usb_backend_domid to -1 and
prevent any assignment while USB VM is gone.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
In the USB VM, we don't have a policy database setup.  Therefore we
cannot auto assign from the udev code path.  Plus since the policy
database is not initialized, we segfault.

We hoist the policy assignment check out of udev_maybe_add_device so it
can be called directly from the udev or xenstore paths.  This seperates
the code better for toostack vs. USB VM.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
We could start up with devices already in xenstore.  While the xenstore
watch will fire, it'll be the top level directory which we don't
process.

Explicity iterate through the directory and grab any existing device
entries.  xs_directory gives us the node names and not the absolute
path, so we have to convert to absolute ourselves.

xsdev_fill should expanded out the node to the full path, so do that.
Also split xsdev_add out of xsdev_event_one and have xsdev_fill call
that directly.  There is no reason the fill function should possibly end
up in remove.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
When the usbvm shuts down, thre is no clean up of the devices.  Current
code expects the /local/domain/Z/data/usb/devX-Y watch to be seen, but
for a shutdown, libxl (?) will just delete /local/domain/Z, which is
delivered as the watch_path /local/domain/Y/data/usb firing.  Therefore
we need to react to /local/domain/Y/data/usb firing by clearing out
devices.

This is simplistic for now - usbvm means it holds all devices and they
should all be removed.  Future enhancement with need to only remove
devices associated with a particular usbvm.

Note, there is something wrong with the UI updating on device removal.
dbus queries of vusb-daemon returns an empty list of devices, but the UI
is still showing devices from the shutdown usbvm.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
jandryuk added 2 commits May 19, 2023 13:55
wait_for_state clears out all watches without looking at the paths or
tokens.  This means wait_for_state could discard watch events our main
loop is looking for - devices won't be populated or remove.  Other
badness could occur.

wait_for_state could be reworked into the main event loop, but that
would be difficult.  The RPC handling is synchronous, so we don't
currently have code to send the response at a future time which would be
needed if the main loop handled the watches.  Another complex idea would
be to make wait_for_state queue other watches for handling back in the
main loop.

A simpler approach is to use a second xenstore handle just for this
function.  That way its watches won't interfere with watches on the main
handle.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
This prints for the devX-Y node and all children, so 10 times per added
usb device.  That's too much, so make it debug so it won't be seen.

When a device is added, there is still an "xsdev_event_one adding device
X:Y" message.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
---
The multiple watch firings is unfortunate - we only need 1.  Maybe a
single xenstore node could contain all the data, but it is nicer to have
the key-value pairs in xenstore.

We could fill data one subtree, and have watches on a second.  When the
watch files, you have to go look in the other tree.  Splitting like that
isn't particularly appealing either.
@jandryuk
Copy link
Contributor Author

jandryuk commented Nov 9, 2023

Refpolicy fixed up. Moved defconfig to 6.1 folder... oh, I didn't regen that under 6.1. Let me re-do that real quick.

Edit: This comment belonged in the xenclient-oe PR.

@crogers1
Copy link
Contributor

crogers1 commented Nov 9, 2023

Merging soon.

@crogers1 crogers1 merged commit 70b6856 into OpenXT:master Nov 9, 2023
@jandryuk jandryuk deleted the usbvm-5.10 branch November 9, 2023 18:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants