Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

R4.1: Adapt to new gui protocol version using grant refs #34

Merged
merged 1 commit into from
Mar 9, 2019

Conversation

HW42
Copy link
Contributor

@HW42 HW42 commented Feb 16, 2018

No description provided.

@HW42
Copy link
Contributor Author

HW42 commented Feb 16, 2018

Depends on QubesOS/qubes-gui-common#2 and requires QubesOS/qubes-gui-daemon#21 in dom0.

@HW42 HW42 changed the title Adapt to new gui protocol version using grant refs R4.1: Adapt to new gui protocol version using grant refs Feb 16, 2018
# Checks for header files.
AC_HEADER_STDC
AC_CHECK_HEADER([u2mfnlib.h])

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

xf86-video-dummy/src/dummy_driver.c Outdated Show resolved Hide resolved
gui-agent/vmside.c Outdated Show resolved Hide resolved
@HW42
Copy link
Contributor Author

HW42 commented May 15, 2018

With the new grant based memory sharing you can run easily in the default limits for allocated/mapped grants. Inside a domain there's /sys/module/xen_gntdev/parameters/limit and /sys/module/xen_gntalloc/parameters/limit. AFAIU they are only to avoid DoS by user space inside the domain so can be simply set to something very high (like here). Any opinion about the concrete value?

More tricky is the global limit controlled by gnttab_max_frames. Default is 32. I have run into this limit at least sometimes. For testing I arbitrarily set it to 128 and had no issues so far. But with more intense usage this might also be not enough.

@marmarek
Copy link
Member

What exactly happen (form user POV) when you run into this limit? Is closing some window enough to get the VM back to working state?

As for actual value, I'd say something allowing handling N windows of maximum supported size, where N, lets say 4.

@cfcs
Copy link

cfcs commented May 16, 2018

I think I recall @reynir also having problems with gnttab_max_frames (on the host side)? @reynir, do you remember the details?

@marmarek: would that mean it would be possible to run 8 windows at half size?

@marmarek
Copy link
Member

@cfcs old GUI protocol didn't used grant references, so that's unlikely. There was a similar problem in mini-os + mirage vchan implementation (hardcoded limit), but that's mostly unrelated to GUI.

As for count of windows - yes.

@cfcs
Copy link

cfcs commented May 18, 2018

@marmarek that was the problem I was thinking of, thank you. :)

Re: Window size: 4 seems a bit limiting, at least I frequently have more than 4 full-screen windows open. If the default is to be conservative, any chance this could be made a configuration option?

@marmarek
Copy link
Member

marmarek commented May 18, 2018 via email

@HW42
Copy link
Contributor Author

HW42 commented May 18, 2018

Fixed the domid != 0 case ;]

gui-domain-test

That's Xephyr (nested X server) running in a domain named gui-domain (with blue border from dom0). In gui-domain there's a qubes-guid running using the Xephyr server. This qubes-guid draws those green borders around the window from another domain. The terminal window itself doesn't get a border since there's no window manager running there which would need to draw the border.

cc @rootkovska

@cfcs
Copy link

cfcs commented May 18, 2018

@marmarek Ahh, I thought it was of the current resolution. That constant definitely works for me. :)


all: libxf86-qubes-common.so

libxf86-qubes-common.so: xf86-qubes-common.c include/xf86-qubes-common.h
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't this have fstack-protector-all, ro-relocations etc?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rpm already define them, it's just about using CFLAGS/LDFLAGS. I'll add it here too.

ret =
read(g->xserver_fd, ((char *) mfnbuf) + rcvd,
size - rcvd);
wd_msg_buf = alloca(wd_msg_len);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing error handling (was already missing in the old version).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants