I was also wondering if interrupts in OSv are handler in a way similar to Linux 
or FreeBSD?

Sent from my iPhone

> On May 5, 2017, at 08:49, Waldek Kozaczuk <jwkozac...@gmail.com> wrote:
> 
> 
> Nadav,
> 
> I agree that I should focus on getting VMBUS driver ported and making OSv 
> boot on hyper/V with ramdisk.
> 
> Also with yours and Avi's help I got most of the compilation errors fixed ( I 
> am down to under 150 lines of errors :-).
> 
> Anyway there are still some outstanding questions/issues I need help with:
> The biggest issue is related to setting up VMBUS interrupt handler which does 
> not seem to be compatible with how OSv drivers do it; I wonder if that part 
> of freebsd code needs to converted to some OSv constructs like interrupt 
> subclasses (processor::inter_processor_interrupt, 
> processor::gsi_edge_interrupt or processor::gsi_level_interrupt); am I on the 
> right track?; Here are the links to the relevant part of the code
> https://github.com/wkozaczuk/osv/blob/b97c056b464e28a4b8f1ff68c4b947109c08f20e/bsd/sys/dev/hyperv/vmbus/vmbus.cc#L128
> https://github.com/wkozaczuk/osv/blob/b97c056b464e28a4b8f1ff68c4b947109c08f20e/bsd/sys/dev/hyperv/vmbus/vmbus.cc#L875-L924
> interrupt handle part1 in assembly code 
> https://github.com/wkozaczuk/osv/blob/b97c056b464e28a4b8f1ff68c4b947109c08f20e/bsd/sys/dev/hyperv/vmbus/amd64/vmbus_vector.S#L34-L46
> interrupt handler part 2 - 
> https://github.com/wkozaczuk/osv/blob/b97c056b464e28a4b8f1ff68c4b947109c08f20e/bsd/sys/dev/hyperv/vmbus/vmbus.cc#L667-L689
> pause_sbt does not exist - I wonder what the equivalent in OSv is - 
> https://github.com/freebsd/freebsd/blob/7a5edbcf70144666ddbbd33f6890fe2ad265ff09/sys/kern/kern_synch.c#L301-L334
> what would be the equivalent of - callout_reset_sbt_curcpu 
> (https://github.com/freebsd/freebsd/blob/7a5edbcf70144666ddbbd33f6890fe2ad265ff09/sys/sys/callout.h#L107)
> Should I port taskqueue_create_fast() and taskqueue_start_threads_cpuset() 
> from freebsd add them to the set of similar functions in 
> bsd/sys/sys/taskqueue.h?
> lapic_ipi_free and lapic_ipi_alloc doe not exist in OSv but I think they are 
> used to allocate/free interrupt handlers in OSv so somehow in OSv it would 
> have to be done differently
> Most of the relevant code that has the issues I am describing above is in 
> these 3 files (all other files in vmbus subdirectory compile at least):
> https://github.com/wkozaczuk/osv/blob/hyper_v/bsd/sys/dev/hyperv/vmbus/vmbus.cc
> https://github.com/wkozaczuk/osv/blob/hyper_v/bsd/sys/dev/hyperv/vmbus/vmbus_chan.cc
> https://github.com/wkozaczuk/osv/blob/hyper_v/bsd/sys/dev/hyperv/vmbus/amd64/vmbus_vector.S
> 
> Thanks in advance for your help,
> Waldek
> 
> 
>> On Thursday, April 27, 2017 at 5:07:22 AM UTC-4, Nadav Har'El wrote:
>> 
>>> On Thu, Apr 27, 2017 at 11:37 AM, Avi Kivity <a...@scylladb.com> wrote:
>>> 
>>> 
>>> On 04/27/2017 07:37 AM, Waldek Kozaczuk wrote:
>>> 
>>>> Here are other issues where I am seeking advice:
>>>> SYSCTL in FreeBSD provides configuration management mechanism for kernel 
>>>> and device drivers (https://www.freebsd.org/cgi/man.cgi?sysctl(8)). As I 
>>>> understand OSv does not support similar mechanism (and possibly will never 
>>>> do). Does it mean that all sysctl related code should be disabled in the 
>>>> Hyper/V drivers that I am bringing in from FreeBSD?
>>> 
>>> Yes, it can be ignored, at least now.
>> 
>> If you're importing code and don't want to bother porting pieces of it yet, 
>> please keep those parts #ifdef'ed out, to make it easier to re-enabling them 
>> in the future.
>> 
>>> 
>>> 
>>>> The vmbus driver uses 4 atomic_* functions that are not present in OSv; 
>>>> should they be brought in from BSD 
>>>> (https://github.com/freebsd/freebsd/blob/master/sys/amd64/include/atomic.h);
>>>>  it looks like some of these functions are intended to be used by kernel 
>>>> modules - does it have any implications to OSv?
>>>> atomic_swap_long
>>>> atomic_testandclear_int
>>>> atomic_testandclear_long
>>>> atomic_testandset_int
>>> 
>>> You can just bring them over (or implement them with std::atomic).
>> 
>>  In bsd/x64/machine/atomic.h we already have  many of the BSD 
>> atomic_functions, you can add more there (and can probably copy them from 
>> BSD, I assume that's what we did with the stuff already there). It's easier 
>> to copy than re-implement because then you won't need to think hard about 
>> what are the specific guarantees that these functions are supposed to make.
>> 
>> And like Avi already said, there's also C++'s std::atomic which you can use 
>> in new code, but I think it will be easier for you not to have to modify 
>> that code.
>> 
>>> 
>>>> In some places the code uses DELAY() macro (example 
>>>> https://github.com/freebsd/freebsd/blob/becaf5f7f2e608c98d9c470f1ae004b24882f6d5fe3e20e5/sys/dev/hyperv/vmbus/vmbus_chan.c#L687)
>>>>  that I believe makes it wait/sleep on the current thread 
>>>> (https://www.freebsd.org/cgi/man.cgi?query=DELAY&apropos=0&sektion=9). How 
>>>> should it be converted to OSv? 
>>> 
>>> There is sched::thread::sleep().
>> 
>> According to delay(9) (see http://www.unix.com/man-page/freebsd/9/delay/),  
>> delay() is supposed to do a busy loop for the given number of microseconds. 
>> So DELAY(1000) (as in that code) should busy loop until a whole millisecond 
>> has passed. I agree with Avi that when such a long amount of time is 
>> concerned, it makes sense to just use sched::thread::sleep() or even 
>> usleep() which already takes the correct units - and let other threads work. 
>> For shorter periods, you can easily implement DELAY, as a busy loop which 
>> continuously calls gettimeofday() and stops looping when enough microseconds 
>> have passed (or it would be even nicer to implement the same thing using 
>> C++'s std::chrono mechanisms).
>> 
>>> 
>>>> In some places the code uses __compiler_membar() macro 
>>>> (https://github.com/freebsd/freebsd/blob/becaf5f7f2e608c98d9c470f1ae004b5fe3e20e5/sys/sys/cdefs.h#L108).
>>>>  How should it be converted to OSv?
>>> 
>>> std::atomic_signal_fence(std::memory_order_seq_cst)
>> 
>> We also have <osv/barrier.hh> with the barrier() function, but I agree that 
>> std::atomic_signal_fence looks more standard.
>>  
>>> 
>>> 
>>>> In some places the code uses critical_enter()/citical_exit() 
>>>> (https://www.freebsd.org/cgi/man.cgi?query=critical_enter&sektion=9) to 
>>>> prevent thread from migrating to other CPU. How should it be converted to 
>>>> OSv? 
>>> 
>>> WITH_LOCK(preempt_lock) { .... }
>> 
>> If *really* the only concern is migration to other CPUs, we also have 
>> WITH_LOCK(migration_lock).
>> But looking at the critical(9) manual page, it seems indeed you should 
>> indeed use preempt_lock because the manual has the text "These functions are 
>> used to prevent preemption".
>>  
>>> 
>>> 
>>>> In some places the code uses VM_GUEST_VM macro and vm_guest global 
>>>> variable (example 
>>>> https://github.com/freebsd/freebsd/blob/becaf5f7f2e608c98d9c470f1ae004b5fe3e20e5/sys/dev/hyperv/vmbus/hyperv.c#L147-L148)
>>>>  to determine if code is running on HyperV? What would be the best way to 
>>>> convert it to OSv? Or maybe it does not apply as in OSv hypervisor is 
>>>> detected during boot time and corresponding code would not be executed 
>>>> anyway?
>> OSv also has some code which can tell a curious application on which 
>> hypervisor is running (see http://www.unix.com/man-page/freebsd/9/delay/), 
>> but I assume that most of the HyperV-specific code would only get run when 
>> HyperV is detected, so that code wouldn't need to check if hyperv is 
>> available. I'm not familiar with that code to give a better answer.
>>  
>>> The struct ifqueue (sys/net/ifq.h) has following changes which affect 
>>> porting netvsc driver:
>>> new field ifq_drv_maxlen - 
>>> https://github.com/freebsd/freebsd/blob/becaf5f7f2e608c98d9c470f1ae004b5fe3e20e5/sys/dev/hyperv/netvsc/if_hn.c#L1348
>>> removed field altq_flags which is used by macro IFQ_SET_READY 
>>> (https://github.com/freebsd/freebsd/blob/8b6efb8965fa7f80abdc363663ed6dafa0c70ecd/sys/net/ifq.h#L228)
>>> Shall we simply comment out this part of the code as nothing else in OSv 
>>> current code uses it?
>>> 
>>> 
>>> 
>>> Here are the things I have changed so far:
>>> Fixed conversion errors by using static_cast<>
>>> I noticed that in other places (type *) construct was used to achieve the 
>>> same thing as static_cast<type *>(); which one is preferred? 
>>> Replaced pause() with bsd_pause() (OSv version of the same functionality I 
>>> think - is this correct?)
>>> Replaced CSUM_* new constants with old ones (for example CSUM_IP_TCP with 
>>> CSUM_TCP) because of the changes in 
>>> https://github.com/freebsd/freebsd/blob/master/sys/sys/mbuf.h 
>>> Due to the changes of struct mbuf layout in FreeBSD 
>>> (https://github.com/freebsd/freebsd/commit/08b6ceecbb05f0b7bd21e29feee1624747a5b172)
>>>  I changed all relevant places in netvsc like in this example:
>>> (m)->m_pkthdr.len to (m)-M_dat.MH.MH__pkthdr.len
>>> Is this the right fix?
>>> Due to the difference of field if_snd in struct ifnet between OSv and 
>>> FreeBSD (look at 
>>> https://github.com/cloudius-systems/osv/commit/19f24526cdd9d937b8d33d25ed7c6437dd5ea89f)
>>>  which makes macros like IFQ_DRV_DEQUEUE not-compilable, I had to replace 
>>> IFQ_DRV_DEQUEUE macro with IF_DEQUEUE and IFQ_DRV_PREPEND with IF_PREPEND
>>> I did it based on the reading of 
>>> https://www.freebsd.org/cgi/man.cgi?query=altq&sektion=9 which makes me 
>>> think that driver managed queue are not supported in OSv
>>> Please see my changes here - 
>>> https://github.com/wkozaczuk/osv/commit/e486143830fcd2020a267cba7f1d3a38df8c34d6#diff-4f9f901a3850cc1241e39ad0cf0d2cb0R4119
>>> Added boolean_t which was missing in OSv
>>> Replaced rdmsr() and wrmsr() with processor::rdmsr() and processor::wrmsr()
>>> Disabled PCI Pass Through support in netvsc driver which use PCIB related 
>>> macros like PCIB_MAP_MSI and functions pcib_ not present in OSv at this time
>>> For example this header is not present in OSv - 
>>> https://github.com/freebsd/freebsd/blob/master/sys/dev/pci/pcib_private.h
>>> Read more about CPI Pass Through here - 
>>> https://www.slideshare.net/iXsystems/pci-passthrough-freebsd-vm-on-hyperv-meetbsd-california-2016
>>> Temporarily commented out usage of mtx_lock(&Giant)/mtx_unlock(&Giant) in 
>>> vmbus.cc
>>> Look for example here - 
>>> https://github.com/freebsd/freebsd/blob/8b6efb8965fa7f80abdc363663ed6dafa0c70ecd/sys/dev/hyperv/vmbus/vmbus.c#L520-L522
>>> What would be the best way to achieve similar locking functionality in OSv? 
>>> Should corresponding logic use some kind of global lock but only limited to 
>>> hyperv vmbus? 
>>> Added ift_counter enum and if_inc_counter() function to if_var.h
>>> Added bsd/sys/net/rndis.h
>>> Added bsd/sys/sys/timetc.h
>>> Added bsd/sys/sys/buf_ring.h
>>> There are other devices that I am not sure if they need to be ported to OSv:
>>> kbd - synthetic keyboard (most likely not needed)
>>> pcib - synthetic PCI bridge driver (https://reviews.freebsd.org/D8332)
>>> kvp - key-value pair driver
>>> heartbeat
>>> timesync - I was wondering if this one needs to be ported to possibly 
>>> synchronize clock with the hypervisor; excuse my ignorance in this subject 
>>> but maybe OSv uses some generic mechanism to synchronize clock that works 
>>> on all hypervisors?
>>> I did not see any console driver. Is it the case that there is some 
>>> standard mechanism to use console that works on all hypervisors?
>>> 
>>> I am also thinking of changing my original plan. Instead of porting all 
>>> drivers first I will first port vmbus only and then work on implementing 
>>> hyperv::vmbus and hyperv::hyperv_driver classes to make OSv at least boot 
>>> on Hyper/V and print something to the console.
>>> 
>>> Also if anybody wants to collaborate I created the branch here - 
>>> https://github.com/wkozaczuk/osv/tree/hyper_v. It is pretty much in flux 
>>> and most hyperv files do not compile yet. I would definitely need help port 
>>> storvsc and netvsc drivers.
>>> 
>>> Also I am apologize for such a long email but I wanted to list all my 
>>> specific questions to get a specific help/feedback.
>>> 
>>> Cheers and thanks in advance,
>>> Waldek
>>> -- 
>>> You received this message because you are subscribed to the Google Groups 
>>> "OSv Development" group.
>>> To unsubscribe from this group and stop receiving emails from it, send an 
>>> email to osv-dev+u...@googlegroups.com.
>>> For more options, visit https://groups.google.com/d/optout.
>> 
>>> -- 
>>> You received this message because you are subscribed to the Google Groups 
>>> "OSv Development" group.
>>> To unsubscribe from this group and stop receiving emails from it, send an 
>>> email to osv-dev+u...@googlegroups.com.
>>> For more options, visit https://groups.google.com/d/optout.
>> 

-- 
You received this message because you are subscribed to the Google Groups "OSv 
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to osv-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to