ATA/BlockDevice adapter

[RaffoPazzo]

I know, someone will quickly reply You're the hell overcomplicating easy stuff. That's not the point.

The design of my OS, defines a set of interfaces as described by HW datasheet and platform specifcation.
For example there is the package hw.generic.ata, containing the interface Ata declaring operations such as readSectorChs or identifyDrive. The fact that this interface is contained in the hw.generic.ata package is dictated by the existance of the ATA specification.
The fact, similarly, that the IBM/PC does contain ATA controllers is modeled by interfaces in the plat.pc package, containing for example header files defining ATA's registers I/O address.
Ext2 classes are, instead, contained in the kernel.fs.ext2 package.
It exists also the kernel.io.block package containing classes and interfaces for block device support, i.e. the BlockDevice interface declaring operations like readBlock and writeBlock.

Now, the idea is that supporting Ext2 filesystem on ATA devices is just a matter of writing the Adapter between hw.generic.ata.Ata and kernel.io.block.BlockDevice, so to let the Ata interface be seen as a BlockDevice, which in turn is required by the Ext2 filesystem's classes to operate.

The question is:

Since there will be so much adapters in my kernel (not only those regarding drives/filesystems), should it exist a dedicated package containing all the adapters something like kernel.adapter or should they be placed together with the class being adapted?

Back to the example, should it exist kernel.adapter.AtaBlockDevice or should it exist hw.generic.ata.BlockDeviceAdapter?

Right now, as a first instance, i have chosen to let it exists hw.generic.ata.BlockDeviceAdapter, since the existance of kernel.adapter is prone to contain the hell of chaos.

[NickJohnson]

What language are you using to do all this, and what exactly is the implementation of a "package"?

[RaffoPazzo]

What language are you using to do all this, and what exactly is the implementation of a "package"?

C (according to C99 syntax).
A package is implemented by a related folder containing all the files needed to implement all the classifiers contained by the package itsel. If a package does not contain only interfaces (so if there is something to be compiled) then its content is processed to be compiled in order to produce a relocatable object file.

[NickJohnson]

Okay, so by "interface" you effectively mean header file with prototypes?

Since you're using C, this seems like a job for structures filled with function pointers.

[RaffoPazzo]

Okay, so by "interface" you effectively mean header file with prototypes?

Since you're using C, this seems like a job for structures filled with function pointers.

Right, this is an overview of the Ata interface i was talking about in the first post.

Code: Select all

typedef struct hw_generic_ata_Ata hw_generic_ata_Ata;
struct hw_generic_ata_Ata{

  hw_generic_ata_DiagnosticCode (*executeDriveDiagnostic)(
    hw_generic_ata_Ata* this);

  // TODO: Consider returning a string
  void (*getFirmwareRevision)(hw_generic_ata_Ata* this, char* str);

  // TODO: Consider returning a string
  void (*getModelNumber)(hw_generic_ata_Ata* this, char* str);

  // TODO: Consider returning a string
  void (*getSerialNumber)(hw_generic_ata_Ata* this, char* str);

  // TODO: Consider returning the data struct
  bool (*identifyDrive)(
    struct hw_generic_ata_Ata* this,
    struct hw_generic_ata_IdentifyDriveData* identifyDriveData);

  bool (*isDmaSupported)( struct hw_generic_ata_Ata* this );
  bool (*isLbaSupported)( struct hw_generic_ata_Ata* this );

  // TODO: Use a Buffer-like object instead of uint8*
  void (*readSectorChs)( hw_generic_ata_Ata* this,
    struct hw_generic_ata_Chs* sectorAddress, uint16* data); 

  void (*readSectorLba)( hw_generic_ata_Ata* this, uint32 lba, uint16* data); 
};

[NickJohnson]

It seems like instead of a separate adapter for each driver, you could just have each driver that implements an interface contain that interface structure within it's own. For example, struct hw_generic_ata_Ata could contain a pointer to a struct kernel_io_block_BlockDevice, which could be passed around when only the block device interface is needed.

[RaffoPazzo]

It seems like instead of a separate adapter for each driver, you could just have each driver that implements an interface contain that interface structure within it's own. For example, struct hw_generic_ata_Ata could contain a pointer to a struct kernel_io_block_BlockDevice, which could be passed around when only the block device interface is needed.

Already thought about that and discarded because this would mean to pollute the hw_generic_ata_Ata interface by something not defined in the ATA specification. Actually, providing hw_generic_ata_BlockDeviceAdpater results in polluting the hw_generic_ata package, bad too...

[Owen]

I'd use a similar design and nomenclature as Microsoft do with the Windows Driver Model, because its well understood, works and reasonable.

The driver is divided into two parts:

• The port driver (i.e. the "ATA Port driver") which is responsible for handling the general ATA portions of the interface, and translating from block device operations to said interface
• The miniport driver (i.e. the "ICH7 UDMA ATA Driver") which is responsible for talking to the actual hardware

Note that in this distinction, the interface between the port and miniport drivers need not correspond to the actual hardware. It should be simplified so that the miniport driver can be implemented with the minimum amount of code.



As an aside: Your interfaces appear synchronous; this is very poor design for a driver interface (where most devices are asynchronous anyway)

[NickJohnson]

Already thought about that and discarded because this would mean to pollute the hw_generic_ata_Ata interface by something not defined in the ATA specification. Actually, providing hw_generic_ata_BlockDeviceAdpater results in polluting the hw_generic_ata package, bad too...

I understand the idea of having the interface contain all of the functions in the spec, but having it so something is in the interface if and only if it is in the spec seems like overkill, especially because the previous suggestion would be an easy and effective fix. You also might run into trouble with your system if you try and write a driver for something with multiple conflicting specs or poor documentation.

[eddyb]

Why does it look like you're trying to do object-oriented interface in... C?
Have you considered taking advantage of C++'s classes (and other features)? Inheritance, virtual inheritance, templates, all that good stuff.
(Just pointing out that the complication is in the implementation, the idea is pretty good )

[RaffoPazzo]

As an aside: Your interfaces appear synchronous; this is very poor design for a driver interface (where most devices are asynchronous anyway)

Maybe I'm misunderstanding (or undersizing) something but being synchronus or asynchronus is a matter of threading and so, it is a matter of implementations not of interfaces.

Shouldn't interface reflect a "concept" e.g. BlockDevice and ATA being just an implementation detail at a different level?

Your clients wouldn't care much if they speak to an ATA device or SCSI device as long as they can get their blocks in and out.

Again, maybe I misunderstood something but right this is what an interface is ment for. My question is where this interfaces should lie in and, following the NickJohnson's suggestion, if it is more appropriate having an Adapter or having the ATA interface extending the BlockDevice interface.

I understand the idea of having the interface contain all of the functions in the spec, but having it so something is in the interface if and only if it is in the spec seems like overkill, especially because the previous suggestion would be an easy and effective fix. You also might run into trouble with your system if you try and write a driver for something with multiple conflicting specs or poor documentation.

I'm still in a prototyping phase whose result should be a set specifications and technical notes to use for the real designing and coding. If your right than this kind of issues should appear sooner the end of this phase.

Code in hand, having the hw_generic_ata_BlockDeviceAdapter is quite comfortable.

Why does it look like you're trying to do object-oriented interface in... C?
Have you considered taking advantage of C++'s classes (and other features)? Inheritance, virtual inheritance, templates, all that good stuff.
(Just pointing out that the complication is in the implementation, the idea is pretty good )

Simply because it is right what i'm doing
Already considered using C++ and discarded because of too many hidden stuff I wouldn't like facing with (e.g. vtable, operators, name mangling, exception handling performances and templates as well, this last one in C++ are not so good as they seems). My opinion is quite simple: C is basically portable assembly. Using it for object-oriented development is just a matter of writing code properly, which is easily aviodable using the proper tools.

[eddyb]

Why does it look like you're trying to do object-oriented interface in... C?
Have you considered taking advantage of C++'s classes (and other features)? Inheritance, virtual inheritance, templates, all that good stuff.
(Just pointing out that the complication is in the implementation, the idea is pretty good )

Simply because it is right what i'm doing
Already considered using C++ and discarded because of too many hidden stuff I wouldn't like facing with (e.g. vtable, operators, name mangling, exception handling performances and templates as well, this last one in C++ are not so good as they seems). My opinion is quite simple: C is basically portable assembly. Using it for object-oriented development is just a matter of writing code properly, which is easily aviodable using the proper tools.

I'm not even going to let trolls get this one...
Vtable is not your problem. Operators, not your problem. Name mangling, not your problem. Exception handling, not your problem (can be disabled). Templates rule and don't require any support code (just use them, they're like super-powerful macros).
You only need a few special functions and you're done (see C++ on the wiki).
And yes, writing messy C code can be avoided using the proper tools, which why I mentioned C++. Please don't make me start ranting about GNU C.

[RaffoPazzo]

I'm not even going to let trolls get this one...
Vtable is not your problem. Operators, not your problem. Name mangling, not your problem. Exception handling, not your problem (can be disabled). Templates rule and don't require any support code (just use them, they're like super-powerful macros).
You only need a few special functions and you're done (see cppsupport.cc from pedigree. this is with highlighting and without debugging stuff).
And yes, writing messy C code can be avoided using the proper tools, which why I mentioned C++. Please don't make me start ranting about GNU C.

You asked me if I considered using C++ and I just replied you that I had and what was my conclusions. If you think that C++ is applicable for your OS development then use it, no one cares.
Don't try to teach people what they already deeply know and don't point people you don't know using terms like trolls, they could teach you things which you would never imagine the existance. Moreover, no one asked your impressions about GNU C.

You have totally ingored the only question discussed in this topic. Your information contribute is still null. Be productive.

[eddyb]

I'm not even going to let trolls get this one...
Vtable is not your problem. Operators, not your problem. Name mangling, not your problem. Exception handling, not your problem (can be disabled). Templates rule and don't require any support code (just use them, they're like super-powerful macros).
You only need a few special functions and you're done (see C++ on the wiki).
And yes, writing messy C code can be avoided using the proper tools, which why I mentioned C++. Please don't make me start ranting about GNU C.

You asked me if I considered using C++ and I just replied you that I had and what was my conclusions. If you think that C++ is applicable for your OS development then use it, no one cares.
Don't try to teach people what they already deeply know and don't point people you don't know using terms like trolls, they could teach you things which you would never imagine the existance. Moreover, no one asked your impressions about GNU C.

You have totally ingored the only question discussed in this topic. Your information contribute is still null. Be productive.

I wasn't talking about you when I said "trolls". And I was trying to make your life easier. Also, what from what I said you deeply knew? Go ahead and use C, and you'll enter the big category of people implementing complex architectures in C. That could've been very nice in C++. And that annoy me because they look awful (your code example reminds me of linux drivers).
And your question can be easily answered if you consider C++: just make hw.generic.ata.Ata inherit hw.generic.Disk/hw.generic.Media/whatever, and write the ext2 driver on top of hw.generic.Disk/hw.generic.Media/whatever (there's a flaw here, though: ext2 is suppose to work with partitions, not disks. so you need about 20 more structures - in C - to cover every possibility).
I spent 5min total answering this, hoping I could help someone. I'm going to unbookmark forum.osdev.org.

EDIT: you need some support code for virtual inheritance, I just remembered. But that when you run into the Diamond problem.

[gerryg400]

Code: Select all

typedef struct hw_generic_ata_Ata hw_generic_ata_Ata;
struct hw_generic_ata_Ata{

  hw_generic_ata_DiagnosticCode (*executeDriveDiagnostic)(
    hw_generic_ata_Ata* this);

  // TODO: Consider returning a string
  void (*getFirmwareRevision)(hw_generic_ata_Ata* this, char* str);

  // TODO: Consider returning a string
  void (*getModelNumber)(hw_generic_ata_Ata* this, char* str);

  // TODO: Consider returning a string
  void (*getSerialNumber)(hw_generic_ata_Ata* this, char* str);

  // TODO: Consider returning the data struct
  bool (*identifyDrive)(
    struct hw_generic_ata_Ata* this,
    struct hw_generic_ata_IdentifyDriveData* identifyDriveData);

  bool (*isDmaSupported)( struct hw_generic_ata_Ata* this );
  bool (*isLbaSupported)( struct hw_generic_ata_Ata* this );

  // TODO: Use a Buffer-like object instead of uint8*
  void (*readSectorChs)( hw_generic_ata_Ata* this,
    struct hw_generic_ata_Chs* sectorAddress, uint16* data); 

  void (*readSectorLba)( hw_generic_ata_Ata* this, uint32 lba, uint16* data); 
};

I'm very confused by this approach. Why would a function like "isDmaSupported" need to be exported ? Surely only the driver itself needs to know whether DMA is supported.

And the difference between readSectorChs and readSectorLba would surely be buried within the driver and the exported function would be simply a generic readSector.