xen-troops / displ_be Goto Github PK
View Code? Open in Web Editor NEWLicense: GNU General Public License v2.0
License: GNU General Public License v2.0
It is sometimes useful not to redirect logs, but request logs to be collected such as DEBUG level was set
If segmentation fault occurs backend doesn't go to closed state. On next restart it sees that frontend is in connected state and tries to create same ring buffer. The solution could be: update current frontend state on backend restart and do not call frontend state changed callback.
Scenario:
Fix could be for the BE to read cons at start and set prod = cons
Dear Oleksandr,
Be so kind to end up with PRs to the related repositories to get this functionality integrated.
src/displayBackend/wayland/Display.cpp:void Display::registryHandler(wl_registry *registry, uint32_t id, const std::string& interface, uint32_t version)
When registry is being parsed multiple instances of classes are created with new which may fail. These need to be reviewed if proper error handling is implemented.
Reported by @iusyk
Method getEDID does not change the state of the instance, so, it is reasonable to make it const.
There are 2 issues with the ctor of class CtrlRingBuffer
src/displayBackend/wayland/WaylandZCopy.hpp:
Classes derived from WaylandZCopy have destructors which may result in undefined behavior:
Deleting a derived class object using a pointer to a base class that has a non-virtual destructor results in undefined behavior. To correct this situation, the base class should be defined with a virtual destructor.
Reported by @lorc
There is the following code in function 'int main(int argc, char argv[])'
for(size_t i = 0; i < db1->getSize() / 4; i++)
{
(reinterpret_cast<uint32_t>(db1->getBuffer()))[i] = 0xFFAACC;
}
No sense to use the reinterpret_cast here, the following code allows to avoid of reinterpret_cast usage.
uint32_t *uiBegin= static_cast<uint32_t * >(db1->getBuffer());
uint32_t *uiEnd= uiBegin + size/sizeof(uint32_t);
std::for_each(uiBegin, uiEnd, [](uint32_t &v){ v= 0xFFAACC;});
according to the code of base class , FrontendHandlerBase does not expect any exceptions, but, potentially, DisplayFrontendHandler::onBind may throw the exception if
Because backend overrides default segmentation fault handler, crash dumb is not generated.
To keep both handlers following solution could be applied:
http://www.alexonlinux.com/how-to-handle-sigsegv-but-also-generate-core-dump
Memcpy impact on PV DRM performance should be measured following this discussion:
https://lists.xenproject.org/archives/html/xen-devel/2016-11/msg01298.html
We've agreed that for this special case the scenario should be as following:
The dtor has the call mConnector->getName(), where mConnector is the pointer that is not checked for the nullptr (see the issue in DisplayCommandHandler::DisplayCommandHandler
DisplayCommandHandler::~DisplayCommandHandler()
{
LOG(mLog, DEBUG) << "Delete command handler, connector name: "
<< mConnector->getName();
mConnector.reset();
}
wrap dtor code in if(mConnector){}
Base class DisplayItf::FrameBuffer has the method getHandle, which is used in some ways
This approach brings the following problems
1.Using reinterpret_cast to get the desirable value
2.Deduction of the type from uintptr_t
3.Not safe code with unpredictable behavior (in the code it is possoble to deduct any type from uintptr_t)
For the purpose of data buffers handling, the real type of FrameBuffer depends on the DisplayMode (DRM or Wayland)
The following class hierachy has been developed to work with the both modes:
Base class DisplayItf::FrameBuffer has the following declaration of the method getHandle
DisplayItf::FrameBuffer
...
/**
Children
a)Drm::FrameBuffer
uintptr_t getHandle() - method is used to return the frame id(integer), so no issue here, because this value is used to save and return frame ID
b)Wayland::WlBuffer (and his children)
/**
where mWlBuffer is the pointer at wl_buffer*
The sample of the usage
wl_surface_attach(mWlSurface, reinterpret_cast<wl_buffer*>(frameBuffer->getHandle()), 0, 0);
Method is used just to cast pointer to the int to be casted back to the original (wl_buffer*) value !!!
The main problem is the method getHandle, this method should be removed from the base class, because it does not have any reasonable sense - it maybe ID, pointer, dummy value.
Remove getHandle from DisplayItf::FrameBuffer.
Instead of getHandle method:
1)Add the virtual uint32_t FrameBuffer::getID()const {...} to the DRM FrameBuffer
2)Add wl_buffer* Wayland::WlBuffer::getWLBuffer()const {...} to WlBuffer
3)To support the new types, use dynamic_cast to deduct the correct type
4) Fix the following function (Surface::draw), because code code is not safe (see comments marked IU)
mBuffer = dynamic_cast<WlBuffer*>(frameBuffer.get());
if (nullptr == mBuffer)
{
return ; // or inform about the error
}
mBuffer->setSurface(this);
wl_surface_damage(mWlSurface, 0, 0, frameBuffer->getWidth(), frameBuffer->toWLBuffer());
wl_surface_attach(mWlSurface, frameBuffer->getWLBuffer(), 0, 0);
5)Drm::Connector::init
change the code
auto fbId = frameBuffer->getHandle();
at
auto fBuffer= dynamic_castDRM::FrameBuffer*frameBuffer.get())
if(nullptr == fBuffer) {
throw Exception("Buffer type has to be DRM::FrameBuffer*")
}
auto fbId = fBuffer->getID();
6)DRM::Connector::pageFlip
auto fbId = frameBuffer->getHandle();
at
auto fBuffer= dynamic_castDRM::FrameBuffer*frameBuffer.get())
if(nullptr == fBuffer) {
throw Exception("Buffer type has to be DRM::FrameBuffer*")
}
auto fbId = fBuffer->getID();
Event handler OnDevice is registered in wl_drm_listener
The code of the handler make the instance(in heap) of type DisplayWayland.
The class DisplayWayland inherits the Drm::Display, the ctor of which can throw the exception (type Exception)
In general - event handler must not throw the exception because there are not any guarantees that C code can handle it
see code
void WaylandZCopy::onDevice(const string& name)
{
lock_guard lock(mMutex);
LOG(mLog, DEBUG) << "onDevice name: " << name;
mDrmDevice.reset(new Drm::DisplayWayland(name));
authenticate();
}
Display::Display ()
{
...
if (mDrmFd < 0)
{
throw Exception("Cannot open DRM device: " + mName, errno);
}
...
if (drmGetCap(mDrmFd, DRM_CAP_DUMB_BUFFER, &hasDumb) < 0 || !hasDumb)
{
throw Exception("Drm device does not support dumb buffers", errno);
}
}
Wrap code inside OnDevice in block try{}catch(const Exception&){}
class Registry has the method void* bind(const wl_interface interface)
Some cases to use are below ;
SharedMemory::init
mWlSharedMemory = static_cast<wl_shm>(bind(&wl_shm_interface));
WaylandKms::init()
mWlKms = static_cast<wl_kms*>(bind(&wl_kms_interface));
...
There are 2 prposals
benefits - it allows to encapsulate the deduction of the returned type
of course.some changes are required in other classes to adopt the behavior like
wl_shell is considered as deprecated across the entire Wayland community, with new clients being designed to use the new xdg_shell protocol.
It still possible to work with ws_shell using EXTRA_OEMESON:append = " -Ddeprecated-wl-shell=true"
but quite possible that this workaround will be not available in further versions of weston
src/displayBackend/drm/Display.hpp has number of "explicit" key word usages, which are not needed.
This needs to be reviewed and fixed.
Reported by @iusyk
xl destroy 1
[ 87.949763] xenbr0: port 2(vif1.0) entered disabled state
[ 88.015029] xenbr0: port 2(vif1.0) entered disabled state
[ 88.022057] device vif1.0 left promiscuous mode
[ 88.026714] audit: type=1700 audit(87.575:4): dev=vif1.0 prom=0 old_prom=256 auid=4294967295 uid=0 gid=0 ses=4294967295
[ 88.037574] xenbr0: port 2(vif1.0) entered disabled state
root@m3ulcb-xen-dom0:/xen-scripts#/xen-scripts#
root@m3ulcb-xen-dom0:
root@m3ulcb-xen-dom0:/xen-scripts# xl list/xen-scripts#
Name ID Mem VCPUs State Time(s)
Domain-0 0 512 4 r----- 18.8
(null) 1 0 4 --p--d 5.8
root@m3ulcb-xen-dom0:
root@m3ulcb-xen-dom0:~/xen-scripts# xenstore-ls -f
/tool = ""
/tool/xenstored = ""
/local = ""
/local/domain = ""
/local/domain/0 = ""
/local/domain/0/control = ""
/local/domain/0/control/feature-poweroff = "1"
/local/domain/0/control/feature-reboot = "1"
/local/domain/0/domid = "0"
/local/domain/0/name = "Domain-0"
/local/domain/0/backend = ""
/local/domain/0/backend/vdispl = ""
/local/domain/0/backend/vdispl/1 = ""
/local/domain/0/backend/vdispl/1/0 = ""
/local/domain/0/backend/vdispl/1/0/versions = "1"
/local/domain/0/backend/vdispl/1/0/frontend-id = "1"
/local/domain/0/backend/vdispl/1/0/frontend = "/local/domain/1/device/vdispl/0"
/local/domain/0/backend/vdispl/1/0/state = "4"
/vm = ""
/libxl = ""
The function stop() is called from dtor of DevInputBase
DevInputBase::~DevInputBase()
{
stop();
....
}
Where stop delegates the call to PollFd::stop
mPollFd->stop();
But function PollFd::stop may throw the exception in the case of unsuccess execution of the write operation
see the code:
void PollFd::stop()
{
uint8_t data = 0xFF;
if (write(mPipeFds[PipeType::WRITE], &data, sizeof(data)) < 0)
{
throw Exception("Error writing pipe", errno);
}
}
The behavior may be the source of application crash
Wrap the call of stop() in try{}catch(const Exception&){} block
Extend exception with errcode field. And return it if req failed.
Reported by @rshym
I use your displ_be for graphical output from PV and PVH domains. I use it to run some domU with full desktop environment. Both in Wayland mode and with DRM. Great software, thanks!
I have not yet succeeded in displaying the virtual console. And the output of the boot messages even less.
Is that even possible?
The class ConnectorBase has some private methods that do not use the class members or functions. All these methods should be declared as static or moved in .cpp file, because they are not a part of the class interface
There are some methods that:
The following methods should be removed or modified
release() - the sense of this method is to release the instance state, because of this, instance of class is not usable until user calls method 'init'.
If it is necessary to change the state of the class instance in one transaction, it is better to use
a) pattern state - implement the State class and use the method setState
b) replace the method release with reset one, where reset should have the list of the parameters to activate a new class state
This method MUST be replaced.
isInitialized - MUST be removed because the sense if this method, to be sure, that method init has been called after release
This method is the source of the following problems
a) isInitialized MUST be called in all methods of the class, besides, the exception MUST be thrown if isInitialized is false
b) all calls of then class methods MUST be inside of block try{}catch{}
c) because of 'a' and 'b', the source code grows and class is more complicated
Redesign the class to use the State pattern or method reset.
Method DevInputBase::init()
There is the following code:
if (ioctl(mFd, EVIOCGRAB, reinterpret_cast<void*>(1)))
{
throw XenBackend::Exception("Grabbed by another process", EBUSY);
}
ioctl(mFd, EVIOCGRAB, reinterpret_cast<void*>(0));
It is proposed the following to avoid reinterpret_cast
template T& as_lvalue( T&& t ) { return t; }
ioctl(mFd, EVIOCGRAB,, &as_lvalue(int{1})
or
ioctl(mFd, EVIOCGRAB,, &as_lvalue(int{0})
Currently in case unsupported pixel format, wayland returns error on polling fd which treated as general wayland error. Backend is closed in this case. For proper handling, backend shell store supported pixel format and perform check on buffer creating.
here is common improvement - use the switch , because req.operation may have the invalid value
(this->*sCmdTable.at(req.operation))(req, rsp);
class DisplayCommandHandler::
The ctor DisplayCommandHandler::DisplayCommandHandler takes the followinf input parameters
-DisplayPtr display,
-ConnectorPtr connector,
-BuffersStoragePtr buffersStorage,
-EventRingBufferPtr eventBuffer
all of the parameters are the pointers (rather shared_ptr)
The problems
After running Xorg and xclock, the screen does not output the clock, it's keeps graying out.
Thanks !
The Xorg runing log:
/# Xorg :0
X.Org X Server 1.20.11
X Protocol Version 11, Revision 0
Build Operating System: linux Ubuntu
Current Operating System: Linux vm1-desktop 5.11.0-34-generic #3620.04.1-Ubuntu SMP Fri Aug 27 08:06:32 UTC 2021 x86_6420.04.2 (For technical support please see http://www.ubuntu.com/support)
Kernel command line: root=/dev/xvda5
Build Date: 06 July 2021 10:17:51AM
xorg-server 2:1.20.11-1ubuntu1
Current version of pixman: 0.38.4
Before reporting problems, check http://wiki.x.org
to make sure that you have the latest version.
Markers: (--) probed, (**) from config file, (==) default setting,
(++) from command line, (!!) notice, (II) informational,
(WW) warning, (EE) error, (NI) not implemented, (??) unknown.
(==) Log file: "/var/log/Xorg.0.log", Time: Sun Sep 26 13:41:45 2021
(==) Using system config directory "/usr/share/X11/xorg.conf.d"
MESA-LOADER: failed to retrieve device information
MESA-LOADER: failed to open xendrm-du: /usr/lib/dri/xendrm-du_dri.so: cannot open shared object file: No such file or directory (search paths /usr/lib/x86_64-linux-gnu/dri:$${ORIGIN}/dri:/usr/lib/dri)
failed to load driver: xendrm-du
(II) modeset(0): Initializing kms color map for depth 24, 8 bpc.
Implement Display Manager which will handle surfaces from display backend and other applications.
Problem: compositor has to be verified for the nullptr, otherwise, in the case of nullptr the behavior is unexpected
Description
We have the following sequence
Surface::Surface (wl_compositor* compositor)
->wl_compositor_create_surface(compositor)
->wl_proxy_marshal_constructor((struct wl_proxy *) wl_compositor,...)
->wl_proxy_marshal_array_constructor()
->wl_proxy_marshal_array_constructor(struct wl_proxy *proxy,...)
->wl_proxy_marshal_array_constructor_versioned(struct wl_proxy *proxy,...)
The wl_proxy_marshal_array_constructor_versioned starts with the following code
"
pthread_mutex_lock(&proxy->display->mutex);
message = &proxy->object.interface->methods[opcode];
....
"
proxy is the composer in Surface ctor, so, in the case of NULL - behavior undefined
It is clear, the instance of the class can be created just via the factory (because of private ctor)
but, at least assert has to be implemented, to be sure that value is correct
Proposed improvement
add assert (compositor != nullptr) or if (compositor == nullptr) {throw std::invalid_parameter("compositor can not be nullptr.");}
add explicit to the declaration of Surface ctor.
Currently DRM device path is hardcoded.
It should be put to command line or config file.
DisplayBackend::DisplayBackend(DisplayPtr display,
const string& deviceName) :
BackendBase("DisplBackend", deviceName),
mDisplay(display)
{
// mDisplay maybe not initilized
// reasonable to add
if(mDisplay) {
mDisplay->start();
}
else {
// inform about the error (throw an exception) or change the object state at invalid
}
}
For multi touch device onUp event is only reported for the very first id in series, e.g. for 2 fingers:
onUp id=0 and no id=1.
Workaround:
diff --git a/src/displayBackend/wayland/SeatTouch.cpp b/src/displayBackend/wayland/SeatTouch.cpp
index 01965d2be68a..c47580440867 100644
--- a/src/displayBackend/wayland/SeatTouch.cpp
+++ b/src/displayBackend/wayland/SeatTouch.cpp
@@ -117,7 +117,7 @@ void SeatTouch::onUp(uint32_t serial, uint32_t time, int32_t id)
}
}
mCurrentCallback = mCallbacks.end();
+// mCurrentCallback = mCallbacks.end();
}
displ_be -c ../displ_be.cfg
ivi_controller not available
[Warning] The ilm_control_context is already destroyed
01.01.70 00:03:27.549 | Display | WRN - Can't initialize ILM: failed. ILM capability will be disabled.
01.01.70 00:03:27.557 | DisplBackend | INF - Create new frontend: Dom(1/0)
01.01.70 00:03:27.558 | DisplFrontend | INF - Dom(1/0) Set backend state to: [Initializing]
01.01.70 00:03:27.560 | DisplFrontend | INF - Dom(1/0) Frontend state changed to: [Initializing]
01.01.70 00:03:27.560 | DisplFrontend | INF - Dom(1/0) Set backend state to: [InitWait]
01.01.70 00:03:27.576 | DisplFrontend | INF - Dom(1/0) Frontend state changed to: [Initialized]
01.01.70 00:03:27.577 | DisplFrontend | INF - Dom(1/0) Add ring buffer, ref: 2476, port: 20
01.01.70 00:03:27.579 | DisplFrontend | INF - Dom(1/0) Add ring buffer, ref: 2475, port: 19
01.01.70 00:03:27.580 | DisplFrontend | INF - Dom(1/0) Set backend state to: [Connected]
01.01.70 00:03:27.599 | DisplFrontend | INF - Dom(1/0) Frontend state changed to: [Connected]
^C01.01.70 00:11:15.003 | DisplFrontend | INF - Dom(1/0) Set backend state to: [Closing]
01.01.70 00:11:15.079 | Main | ERR - Segmentation fault!
displ_be(_Z19segmentationHandleri+0x74)[0x4298b4]
linux-vdso.so.1(__kernel_rt_sigreturn+0x0)[0xffffb73866c0]
displ_be(_ZN21DisplayCommandHandler13sendFlipEventEm+0x224)[0x436ce4]
displ_be(_ZN7Wayland7Surface12frameHandlerEv+0x13c)[0x45ea8c]
/usr/lib/libffi.so.6(ffi_call_SYSV+0x64)[0xffffb6d8de14]
/usr/lib/libffi.so.6(ffi_call+0x9c)[0xffffb6d8e73c]
/usr/lib/libwayland-client.so.0(+0x8c08)[0xffffb7282c08]
The CPU usage of the kmscube process running in DomU exceeds 50%, but its CPU usage in Dom0 does not exceed 2%.
Remark:
Back-end command: displ_be -m DRM -z -v *:Debug
Displ_be disables zero copy (under zero copy, the front-end error: [drm:xen_drm_drv_dumb_create [drm_xen_front]] ERROR Failed to cre ate dumb buffer: -25. Back-end error: gnttab: error: ioctl DMABUF_EXP_FROM_REFS_V2 ioctlapproate failed: Inappro device )
Thanks !
A declarative, efficient, and flexible JavaScript library for building user interfaces.
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google ❤️ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.