[vbox-dev] Interesting patch for vbox upstream

2017-08-01 Thread Hans de Goede 

Hi,

You may want to do something similar to this for vbox upstream:

https://github.com/jwrdegoede/vboxguest/commit/8a79961c661e3fba88c0e50447b2344e0c811d1a

Note in my tree this applies on top of:
https://github.com/jwrdegoede/vboxguest/commit/8c232c972f7b2dfbfc863a1aaeb0b44fdfcf82d0

Which I don't think you want, but it would be good to use the same
principle to differentiate between the kLevel_TrustedUsers and kLevel_AllUsers 
cases.

Regards,

Hans

___
vbox-dev mailing list
vbox-dev@virtualbox.org
https://www.virtualbox.org/mailman/listinfo/vbox-dev

[vbox-dev] [PATCH 2/2] vgdrvLinuxIOCtl: Always return standard Linux errno values

2017-08-01 Thread Hans de Goede 
Returning VERR_* style vbox-runtime codes as positive values is an ugly
hack and unlike any other ioctl call on Linux, breaking standard
expectations of ioctls.

All Guest Additions user-space code calls the ioctl through the
vbglR3DoIOCtl wrapper, which calls on RTErrConvertFromErrno on negative
return values. The one exception is the src/VBox/GuestHost/OpenGL/util
code, which gets fixed in this commit to deal with standard errno errors.

All Guest Additions user-space code has been audited to check that any
error codes requiring special handling survive being translated to an
errno and back again by using RTErrConvertToErrno + RTErrConvertFromErrno.

This means that we can return all errors as standard -errno values using
RTErrConvertToErrno and vbglR3DoIOCtl will convert them back to VERR_*
style vbox-runtime codes with any errors requiring special handling
surviving as is.

This commit removes the positive VERR_* style vbox-runtime codes
and makes vgdrvLinuxIOCtl always return standard Linux errno values.

Signed-off-by: Hans de Goede 
---
 src/VBox/Additions/common/VBoxGuest/VBoxGuest-linux.c | 7 ++-
 src/VBox/Additions/common/VBoxGuestLib/VBoxGuestR3Lib.cpp | 4 +++-
 src/VBox/GuestHost/OpenGL/util/vboxhgcm.c | 2 ++
 src/VBox/GuestHost/OpenGL/util/vboxhgsmi.c| 2 ++
 4 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/src/VBox/Additions/common/VBoxGuest/VBoxGuest-linux.c 
b/src/VBox/Additions/common/VBoxGuest/VBoxGuest-linux.c
index 1ba3ba6..d48eb9e 100644
--- a/src/VBox/Additions/common/VBoxGuest/VBoxGuest-linux.c
+++ b/src/VBox/Additions/common/VBoxGuest/VBoxGuest-linux.c
@@ -818,7 +818,12 @@ static int vgdrvLinuxIOCtl(struct inode *pInode, struct 
file *pFilp, unsigned in
 else
 {
 Log(("vgdrvLinuxIOCtl: pFilp=%p uCmd=%#x ulArg=%p failed, 
rc=%d\n", pFilp, uCmd, (void *)ulArg, rc));
-rc = -rc; Assert(rc > 0); /* Positive returns == negated VBox 
error status codes. */
+/* Userspace code relies on VERR_* style vbox-runtime codes,
+ * vbglR3DoIOCtl will convert them back to VERR_* style
+ * vbox-runtime codes. This means that any VERR_* codes which
+ * require special handling must survive the to and from errno
+ * conversion as is. */
+rc = -RTErrConvertToErrno(rc);
 }
 }
 else
diff --git a/src/VBox/Additions/common/VBoxGuestLib/VBoxGuestR3Lib.cpp 
b/src/VBox/Additions/common/VBoxGuestLib/VBoxGuestR3Lib.cpp
index 28da83d..61b1c00 100644
--- a/src/VBox/Additions/common/VBoxGuestLib/VBoxGuestR3Lib.cpp
+++ b/src/VBox/Additions/common/VBoxGuestLib/VBoxGuestR3Lib.cpp
@@ -433,7 +433,9 @@ int vbglR3DoIOCtl(unsigned iFunction, void *pvData, size_t 
cbData)
 if (RT_LIKELY(rc == 0))
 return VINF_SUCCESS;
 
-/* Positive values are negated VBox error status codes. */
+/* Older kernel guest drivers use to return (negated) VBox error status
+ * codes as positive values, for now we still handle this case in case
+ * we end up running with an older guest driver (20170730) */
 if (rc > 0)
 rc = -rc;
 else
diff --git a/src/VBox/GuestHost/OpenGL/util/vboxhgcm.c 
b/src/VBox/GuestHost/OpenGL/util/vboxhgcm.c
index fae2f22..813f3ec 100644
--- a/src/VBox/GuestHost/OpenGL/util/vboxhgcm.c
+++ b/src/VBox/GuestHost/OpenGL/util/vboxhgcm.c
@@ -650,6 +650,8 @@ static int crVBoxHGCMCall(CRConnection *conn, void *pvData, 
unsigned cbData)
 return VINF_SUCCESS;
 }
 #  ifdef RT_OS_LINUX
+if (rc < 0)
+rc = -RTErrConvertFromErrno(errno);
 if (rc >= 0) /* positive values are negated VBox error status codes. */
 {
 crWarning("vboxCall failed with VBox status code %d\n", -rc);
diff --git a/src/VBox/GuestHost/OpenGL/util/vboxhgsmi.c 
b/src/VBox/GuestHost/OpenGL/util/vboxhgsmi.c
index 7032381..d9c4818 100644
--- a/src/VBox/GuestHost/OpenGL/util/vboxhgsmi.c
+++ b/src/VBox/GuestHost/OpenGL/util/vboxhgsmi.c
@@ -277,6 +277,8 @@ static int crVBoxHGCMCall(void *pvData, unsigned cbData)
 return VINF_SUCCESS;
 }
 #  ifdef RT_OS_LINUX
+if (rc < 0)
+rc = -RTErrConvertFromErrno(errno);
 if (rc >= 0) /* positive values are negated VBox error status codes. */
 {
 crWarning("vboxCall failed with VBox status code %d\n", -rc);
-- 
2.9.4

___
vbox-dev mailing list
vbox-dev@virtualbox.org
https://www.virtualbox.org/mailman/listinfo/vbox-dev

[vbox-dev] [PATCH 1/2] RTErrConvertTo/FromErrno: Add translation for some extra codes

2017-08-01 Thread Hans de Goede 
Some VERR_ codes require special error handling in userspace, make
sure that these survive being converted to an errno and back again.

Specifically this commit adds support for the following VERR_ codes:

VERR_NOT_FOUND
VERR_HGCM_SERVICE_NOT_FOUND
VERR_BUFFER_OVERFLOW
VERR_OUT_OF_RANGE

To RTErrConvertToErrno and RTErrConvertFromErrno.

Signed-off-by: Hans de Goede 
---
 src/VBox/Runtime/common/err/RTErrConvertFromErrno.cpp | 9 +
 src/VBox/Runtime/common/err/RTErrConvertToErrno.cpp   | 9 +
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/src/VBox/Runtime/common/err/RTErrConvertFromErrno.cpp 
b/src/VBox/Runtime/common/err/RTErrConvertFromErrno.cpp
index a79cda5..d0cb45b 100644
--- a/src/VBox/Runtime/common/err/RTErrConvertFromErrno.cpp
+++ b/src/VBox/Runtime/common/err/RTErrConvertFromErrno.cpp
@@ -34,6 +34,7 @@
 #include 
 #include 
 #include 
+#include 
 
 
 RTDECL(int)  RTErrConvertFromErrno(unsigned uNativeCode)
@@ -153,7 +154,7 @@ RTDECL(int)  RTErrConvertFromErrno(unsigned uNativeCode)
 case EDOM:  return VERR_INVALID_PARAMETER;  /** @todo fix 
duplicate error */
 #endif
 #ifdef ERANGE
-case ERANGE:return VERR_INVALID_PARAMETER;  /** @todo fix 
duplicate error */
+case ERANGE:return VERR_OUT_OF_RANGE;
 #endif
 #ifdef EDEADLK
 case EDEADLK:   return VERR_DEADLOCK;
@@ -175,10 +176,10 @@ RTDECL(int)  RTErrConvertFromErrno(unsigned uNativeCode)
 #endif
 //41??
 #ifdef ENOMSG
-//case ENOMSG   42  /* No message of desired type */
+case ENOMSG:return VERR_NOT_FOUND;
 #endif
 #ifdef EIDRM
-//case EIDRM43  /* Identifier removed */
+case EIDRM: return VERR_HGCM_SERVICE_NOT_FOUND;
 #endif
 #ifdef ECHRNG
 //case ECHRNG   44  /* Channel number out of range */
@@ -272,7 +273,7 @@ RTDECL(int)  RTErrConvertFromErrno(unsigned uNativeCode)
 //case EBADMSG  74  /* Not a data message */
 #endif
 #ifdef EOVERFLOW
-case EOVERFLOW: return VERR_TOO_MUCH_DATA;   /** @todo fix 
duplicate error */
+case EOVERFLOW: return VERR_BUFFER_OVERFLOW;
 #endif
 #ifdef ENOTUNIQ
 case ENOTUNIQ:  return VERR_NET_NOT_UNIQUE_NAME;
diff --git a/src/VBox/Runtime/common/err/RTErrConvertToErrno.cpp 
b/src/VBox/Runtime/common/err/RTErrConvertToErrno.cpp
index 6e5823c..d32e7b5 100644
--- a/src/VBox/Runtime/common/err/RTErrConvertToErrno.cpp
+++ b/src/VBox/Runtime/common/err/RTErrConvertToErrno.cpp
@@ -34,6 +34,7 @@
 #include 
 #include 
 #include 
+#include 
 
 
 RTDECL(int) RTErrConvertToErrno(int iErr)
@@ -155,7 +156,7 @@ RTDECL(int) RTErrConvertToErrno(int iErr)
 //case VERR_INVALID_PARAMETER:return EDOM;
 #endif
 #ifdef ERANGE
-//case VERR_INVALID_PARAMETER:return ERANGE;
+case VERR_OUT_OF_RANGE: return ERANGE;
 #endif
 #ifdef EDEADLK
 case VERR_DEADLOCK: return EDEADLK;
@@ -178,10 +179,10 @@ RTDECL(int) RTErrConvertToErrno(int iErr)
 #endif
 //41??
 #ifdef ENOMSG
-//case ENOMSG   42  /* No message of desired type */
+case VERR_NOT_FOUND:return ENOMSG;
 #endif
 #ifdef EIDRM
-//case EIDRM43  /* Identifier removed */
+case VERR_HGCM_SERVICE_NOT_FOUND:   return EIDRM;
 #endif
 #ifdef ECHRNG
 //case ECHRNG   44  /* Channel number out of range */
@@ -275,7 +276,7 @@ RTDECL(int) RTErrConvertToErrno(int iErr)
 //case EBADMSG  74  /* Not a data message */
 #endif
 #ifdef EOVERFLOW
-//case VERR_TOO_MUCH_DATA:return EOVERFLOW;
+case VERR_BUFFER_OVERFLOW:  return EOVERFLOW;
 #endif
 #ifdef ENOTUNIQ
 case VERR_NET_NOT_UNIQUE_NAME:  return ENOTUNIQ;
-- 
2.9.4

___
vbox-dev mailing list
vbox-dev@virtualbox.org
https://www.virtualbox.org/mailman/listinfo/vbox-dev

Re: [vbox-dev] New kernel error message when running 4.13 as guest

2017-08-01 Thread Michal Necasek 

   Hi Hans,

 It's a really good question why that message is printed at all. As Frank 
pointed out, the apic_check_deadline_errata() function only goes by the CPU 
model and does not check whether the feature is available.

 It is generally bad form to assume specific CPU features based solely on the 
CPU model, and Intel pretty strongly advises against it. I would think that 
apic_check_deadline_errata() should check for X86_FEATURE_TSC_DEADLINE_TIMER 
first, because if it's not there, there is nothing to do. That would get rid of 
the message on VirtualBox and would save a couple of cycles on older CPUs that 
don't have the APIC deadline timer (and therefore don't need to have their 
model figured out).

 We will probably expose the microcode revision to the guest at some point, but 
as I mentioned, that's something which could have unpredictable side effects.


Regards,
  Michal

- Original Message -
From: hdego...@redhat.com
To: michal.neca...@oracle.com, vbox-dev@virtualbox.org
Sent: Wednesday, July 26, 2017 10:17:26 PM GMT +01:00 Amsterdam / Berlin / Bern 
/ Rome / Stockholm / Vienna
Subject: Re: [vbox-dev] New kernel error message when running 4.13 as guest

Hi Michal,

On 26-07-17 17:18, Michal Necasek wrote:
> 
> Hi Hans,
> 
>   The message is harmless. It's triggered because VirtualBox currently does 
> not expose the host CPU microcode revision to guest operating systems. I 
> don't think we want to do that without fully understanding what sorts of 
> assumptions operating systems might make when they see the actual revision.
> 
>   At any rate, VirtualBox currently doesn't support TSC deadline timers so 
> the message is simply noise.

Thank you for your reaction. I understand that the message is just noise, but 
it would
still be nice to do something about it, Linux distributions have gone through 
great
efforts to offer a smooth boot experience without scary text-messages and an 
error
like this breaks that part of the user-experience.

I wonder why the kernel prints the message at all if vbox does not support
TSC deadline timers, could it be that somehow the capability bit for that
gets leaked from the host to the guest even though it is not supported ?

Regards,

Hans

> - Original Message -
> From: hdego...@redhat.com
> To: vbox-dev@virtualbox.org
> Sent: Monday, July 24, 2017 9:49:56 AM GMT +01:00 Amsterdam / Berlin / Bern / 
> Rome / Stockholm / Vienna
> Subject: [vbox-dev] New kernel error message when running 4.13 as guest
> 
> Hi all,
> 
> I just upgraded a vbox guest to 4.13, the good news is
> everything works fine, but one thing which I noticed, which
> would be nice to fix is this new error message:
> 
> [0.00] [Firmware Bug]: TSC_DEADLINE disabled due to Errata; please 
> update microcode to version: 0xb2 (or later)
> 
> This is on a Skylake host with no special settings done
> to the guest (I selected the Fedora 64 bit template
> when installing).
> 
> Regards,
> 
> Hans
> ___
> vbox-dev mailing list
> vbox-dev@virtualbox.org
> https://www.virtualbox.org/mailman/listinfo/vbox-dev
> ___
> vbox-dev mailing list
> vbox-dev@virtualbox.org
> https://www.virtualbox.org/mailman/listinfo/vbox-dev
> 
___
vbox-dev mailing list
vbox-dev@virtualbox.org
https://www.virtualbox.org/mailman/listinfo/vbox-dev